Add tslint/prettier for linting/formatting.

[gurgeous]

This change adds tslint & prettier as dev dependencies and recommends the vscode extensions as well. Using tslint will increase the quality of our work with very little dev cost. Prettier will auto-format our js and ts files on save. This sidesteps all problems related to code formatting, since prettier will just take care of it for us.

There are no code changes in this PR. Once this is merged, we should go ahead and run tslint and prettier across the entire source tree as a single PR. Otherwise we'll suffer through piecemeal diffs for subsequent PRs.

If we agree that this is a step we'd like to take, please read the proposed .prettierrc below. You can see the full list of options here:

https://prettier.io/docs/en/options.html

Personally I don't care very much about the specific prettier settings. That's the great thing about adopting prettier - using it helps you stop caring about formatting. :)

[wingrunr21]

I was thinking tslint-microsoft-contrib vs tslint:recommended here

[gurgeous]

I have zero experience with ts or tslint. Fine with me :)

[wingrunr21]

Except for using tslint:recommended this all seems fine.

[gurgeous]

I looked into it and it appears that tslint-microsoft-contrib is incompatible with linting one file at a time. From https://marketplace.visualstudio.com/items?itemName=eg2.tslint, "Rules with type information are currently not supported by vscode-tslint, pls see issue /~https://github.com/Microsoft/vscode-tslint/issues/70."

Those rules only work if you lint the entire project at once with a grunt task...

Maybe we could start with this and switch to better rules as they become available? Better some tslint than no tslint :)

[wingrunr21]

I'm not following. The tslint-microsoft-contrib is just a set of pre-configured TSLint rules. Why would it be any more or less compatible with vscode-tslint than any other TSLint ruleset?

[gurgeous]

My understanding of these tools is not great so I could be wrong. The tslint-microsoft-contrib rules require running with --type-check --project, otherwise you get a bunch of errors like below. But vscode-tslint doesn't support those options, because that would require linting the whole project each time linting is run.

Warning: The 'no-use-before-declare' rule requires type information.
Warning: The 'no-cookies' rule requires type information.
Warning: The 'await-promise' rule requires type information.
Warning: The 'match-default-export-name' rule requires type information.
Warning: The 'no-floating-promises' rule requires type information.
Warning: The 'no-for-in-array' rule requires type information.
Warning: The 'no-unsafe-any' rule requires type information.
Warning: The 'promise-function-async' rule requires type information.
Warning: The 'restrict-plus-operands' rule requires type information.
Warning: The 'strict-boolean-expressions' rule requires type information.
Warning: The 'completed-docs' rule requires type information.
Warning: The 'no-unnecessary-qualifier' rule requires type information.
Warning: The 'no-unnecessary-type-assertion' rule requires type information.
Warning: The 'no-void-expression' rule requires type information.
Warning: The 'use-default-type-parameter' rule requires type information.

[wingrunr21]

Yep, got that. That just means we are going to need to support also having people being able to run TSLint over the whole project (possibly via a task that can be run within VSCode so you still get feedback in the problems section). vscode-tslint's explanation for not supporting this is valid, but if we are running TSLint we should be linting types too IMO

[gurgeous]

I think there's value in tslint regardless of the ruleset applied. If you think the tslint-microsoft-contrib rules are worth the bother I'm happy to give it a shot. Certainly we can turn it on and then just ignore the warnings due to lack of types.

We can add a task to run with --type-check etc. too. Separate PR, probably.

Let me try turning it on here and see how it feels. Maybe we can disable the invalid rules in the vscode-tslint config to avoid warning spam.

[wingrunr21]

Please switch it to tslint-microsoft-contrib

[gurgeous]

Sure. How about this? I disabled a few rules that were overly noisy. I think it's generating some useful stuff now. If you have some experience with tslint it would be great if you could help me customize the ruleset a bit...

[gurgeous]

Oh, and here's a sample of the errors produced:

$ tslint --project src --config tslint.json

255 Unsafe use of expression of type 'xxx'.
 142 expected variable-declaration: 'xxx' to have a typedef
 107 expected arrow-parameter: 'xxx' to have a typedef
  91 Identifier 'xxx' is never reassigned; use 'xxx' instead of 'xxx'.
  35 Forbidden 'xxx' keyword, use 'xxx' or 'xxx' instead
  30 expected parameter: 'xxx' to have a typedef
  30 Missing blank line before return
  26 Use a template literal instead of concatenating with a string literal.
  25 if statements must be braced
  25 expected call-signature: 'xxx' to have a typedef
  23 expected arrow-call-signature to have a typedef
  20 Type declaration of 'xxx' loses type-safety. Consider replacing it with a more precise type.
  18 variable name must be in lowerCamelCase or UPPER_CASE
  18 Operands of 'xxx' operation must either be both strings or both numbers, consider using template literals
  18 Import sources within a group must be alphabetized.
  17 functions that return promises must be async
  14 Expression has type `void`. Put it on its own line as a statement.
  13 Replace block comment with a single-line comment
  13 Promises must be handled appropriately
... (a hundred more) ...

[gurgeous]

BTW, this is the default config that yo code generates for new ts vscode extensions:

{
  "rules": {
    "no-string-throw": true,
    "no-unused-expression": true,
    "no-duplicate-variable": true,
    "curly": true,
    "class-name": true,
    "semicolon": [true, "always", "ignore-bound-class-methods"],
    "triple-equals": true
  },
  "defaultSeverity": "warning"
}

Might be worth considering. Let me know if you want me to fix these conflicts or make changes to get it merged.