Add support for TSLint

[ianschmitz]

Closes #5641.

Adds support for TSLint via fork-ts-checker-webpack-plugin.

TODO:

• Figure out where tslint.json should live
• Finalize rules
• React hooks rules - Add linting for React Hooks palantir/tslint-react#186
• Lint test files when running jest

[ianschmitz]

@Timer let me know how you feel about this. I refactored all the serviceWorker code to use async/await, which fixes all the TSLint errors.

[andyrichardson]

tslint-config-prettier and tslint-react would be nice defaults to have 👍

[ianschmitz]

@andyrichardson which rules were you thinking from tslint-react? A lot of the rules seemed to be style prefer nces which we try to avoid.

[andyrichardson]

Fair point - jsx-key is already flagged by React so I guess the only other useful non-style option is jsx-self-close.

[mohamedmansour]

This will expose tslint.json to the root, so technically we can add our own rules to the project, correct? The current way create-react-app does eslint, it exposes a package.json config called eslintConfig, we could do the same and name it tslintConfig.

[Timer]

@mohamedmansour just like ESLint, you will not be able to add your own rules. Added rules will strictly affect the editor output, not the build tools.

If you need to check linting on commit, I suggest you use a precommit hook.

[mohamedmansour]

This is unfortunate, why couldn't we change that model and allow the users to override tslint.json? The beautiful thing about TypeScript is that it is a superset of JavaScript, each team can decide what kind of strictness they need or want. Once create-react-app enforce this projects strictness for linting, then I cannot use the rules that I want to enforce in my own project.

For example, we are overriding many rules to fit our teams standard here https://github.com/OfficeDev/office-ui-fabric-react/blob/master/packages/tslint-rules/tslint.json

And in my current project, I just do the following in tslint.json:

{
  "extends": [
    "@uifabric/tslint-rules"
  ],
  "rules": {
    "jsx-no-lambda": false,
    "jsx-ban-props": false,
    "typedef": [false]
  }
}

Having a choice on how linting works is more preferable than the way this PR does it.

[samuelcastro]

@ianschmitz What's left to get this done? How can we help?

[ianschmitz]

@ianschmitz What's left to get this done? How can we help?

This would be awesome: palantir/tslint-react#186

[adriankremer]

it's nice that someone already works on this!!!

Today i also thought about mapping eslint to tslint ( tslint-eslint-rules ) or parsing typescript with eslint typescript-eslint-parser
which both would let us stay on one linting ruleset and also would have the advantages of keeping the linting configuration detached from the app.

Unfortunately mapping eslint to tslint does not work with "extends" so we would have to create another project (e.g. tslint-config-react-app) and duplicate the ruleset. And my favourite: parsing typescript with eslint does not work on the IDE because of the fileextension.

[adriankremer]

btw. will this PR cover IDE support?

[ianschmitz]

We should be able to follow the same pattern that we already have for ESLint to add a dummy tslint.json file to root of repo and extend our ruleset

[adriankremer]

yes thats the only way i think (too bad there is no tslintConfig standard for package.json). then it would be the best to create another package like tslint-config-react-app and reference this via fork-ts-checker-webpack-plugin and the tslint.json ?

@Timer already reserved it on npm :) https://www.npmjs.com/package/tslint-config-react-app

[ianschmitz]

Yep we have that in this PR already 🙂

[harrysolovay]

I created a solution to this problem. You can use your own TSLint instead of the built-in linting or fork-ts-checker by using the Rescripts framework and its use-tslint-config rescript (I just released it and would really love your feedback!) :)

[samuelcastro]

@harrysolovay Rescripts seems amazing, looking forward to test it, good job!

[harrysolovay]

@samuelcastro thank you so much! Please let me know your thoughts & any suggestions :)

[ianschmitz]

Based on the current roadmap that has been laid out over at microsoft/TypeScript#29288, it sounds like they will be standardizing on ESLint:

our editor team will be focusing on leveraging ESLint rather than duplicating work. For scenarios that ESLint currently doesn't cover (e.g. semantic linting or program-wide linting), we'll be working on sending contributions to bring ESLint's TypeScript support to parity with TSLint. As an initial testbed of how this works in practice, we'll be switching the TypeScript repository over to using ESLint, and sending any new rules upstream.

I will keep a close eye on this to see if this is the direction that we should follow as they make progress bringing ESLint up to snuff to support TypeScript.

[ianschmitz]

More information from ESLint team:

https://eslint.org/blog/2019/01/future-typescript-eslint

[ianschmitz]

Closing in favor of #6513