@gkalpak here we are! 😄 🎉

But actually.... eslint seems to not handling very well the large amount of files to check here, and it is so much slower than tslint in this instance... whilst this check takes tslint around 40 seconds on my machine, eslint takes around 4 minutes.... 😞
(and you can see I am ignoring all possible files except the ts ones)

This is rather unfortunate... but I guess it would still make sense to move away from the deprecates tslint.... what do you think?

@rafaelss95 Thank you so very much for your nice reviewing! it was really helpful and much appreciated! 😃

I replied to your comments feel free to have another look 🙂

(ps: again same as list time, for some reason I cannot re-request your review 😕)

Hi @dario-piotrowicz, creator of @typescript-eslint and @angular-eslint here 👋

Regarding:

But actually.... eslint seems to not handling very well the large amount of files to check here, and it is so much slower than tslint in this instance... whilst this check takes tslint around 40 seconds on my machine, eslint takes around 4 minutes.... 😞

I cover performance concerns in depth in the README here: https://github.com/angular-eslint/angular-eslint#eslint-configs-and-performance

I do regret caving to the pressure of not having an extra tsconfig file by default. I think we will change this in v13.

For now, though, let's work through that guide for this case (and even take it a bit further at the end):

Baseline - how you have it configured at the time of writing

❯ time yarn lint                                                                                                                                                                                
yarn run v1.22.10
$ yarn check-env && yarn docs-lint && ng lint && yarn example-lint && yarn tools-lint
$ yarn ~~check-env
$ node scripts/check-environment
$ yarn aio-check-local
$ node tools/ng-packages-installer check .
$ eslint --ignore-path="tools/transforms/.eslintignore" tools/transforms

Linting "site"...

All files pass linting.

$ eslint content/examples
$ eslint tools/firebase-test-utils && eslint tools/ng-packages-installer
✨  Done in 108.88s.
yarn lint  98.18s user 33.07s system 120% cpu 1:49.20 total

Following the performance guide - adding tsconfig.eslint.json and updating .eslintrc.json

CREATE tsconfig.eslint.json

{
  "extends": "./tsconfig.json",
  "include": ["**/*.ts"],
  "exclude": ["aio-builds-setup", "dist", "node_modules", "out-tsc", "scripts"]
}

UPDATE .eslintrc.json

  "parserOptions": {
-    "project": [
-       "tsconfig.json",
-       "tests/e2e/tsconfig.json"
-     ],
+    "project": ["tsconfig.eslint.json"],
-    "createDefaultProgram": true
+    "createDefaultProgram": false
  }

❯ time yarn lint                                                                                                                                                                         
yarn run v1.22.10
$ yarn check-env && yarn docs-lint && ng lint && yarn example-lint && yarn tools-lint
$ yarn ~~check-env
$ node scripts/check-environment
$ yarn aio-check-local
$ node tools/ng-packages-installer check .
$ eslint --ignore-path="tools/transforms/.eslintignore" tools/transforms

Linting "site"...

All files pass linting.

$ eslint content/examples
$ eslint tools/firebase-test-utils && eslint tools/ng-packages-installer
✨  Done in 62.91s.
yarn lint  71.74s user 7.41s system 125% cpu 1:03.25 total

Possibly going even further - no type-checking rules - updating .eslintrc.json

You are currently only using one type-checking aware rule - @typescript-eslint/dot-notation (coming via the ng-cli-compat extends).

If you can live without that rule, you can speed things up significantly because some expensive Program creation can be skipped.

In this scenario, no tsconfig.eslint.json is required and the .eslintrc.json just needs to be updated as follows:

  "parserOptions": {
-    "project": [
-       "tsconfig.json",
-       "tests/e2e/tsconfig.json"
-     ],
+    "project": null,
-    "createDefaultProgram": true
  }

  // ...further down in the rules section...
-   "dot-notation": "error",
+   "dot-notation": "off",
+   "@typescript-eslint/dot-notation": "off",

❯ time yarn lint                                                                                                                                                                            
yarn run v1.22.10
$ yarn check-env && yarn docs-lint && ng lint && yarn example-lint && yarn tools-lint
$ yarn ~~check-env
$ node scripts/check-environment
$ yarn aio-check-local
$ node tools/ng-packages-installer check .
$ eslint --ignore-path="tools/transforms/.eslintignore" tools/transforms

Linting "site"...

All files pass linting.

$ eslint content/examples
$ eslint tools/firebase-test-utils && eslint tools/ng-packages-installer
✨  Done in 31.45s.
yarn lint  35.01s user 3.48s system 120% cpu 31.927 total

Hi @JamesHenry , thank you so very much for your awesome comment and also for the great contribution you've done to the community with your eslint packages ( needless to say they are awesome 🙂 )

I must admit I didn't really read that part of the readme 😓
Anyways thanks a lot for the awesome suggestions, this looks really really great!!! 🙂

Seems like going with the last alternative would be the best thing for me as it really looks so much faster, but I am not totally sure. The fact is that if in the future we'd need to add new rules with type-checking we would have to change the eslintrc file and potentially fall back into the original time issue

Whilst with the second solution the linting would not be as fast but we would be more future-proof...

Did I get it right @JamesHenry ? that's you're view on this what would you go for?

Also since I am just a simple contributor I would really love to hear what someone from the angular team would have to say about this 🙂 (@gkalpak I'm counting on you! 😄)

You can preview 93bd4fc at https://pr43218-93bd4fc.ngbuilds.io/.
You can preview 7a520cb at https://pr43218-7a520cb.ngbuilds.io/.
You can preview a36817e at https://pr43218-a36817e.ngbuilds.io/.
You can preview 2678b4e at https://pr43218-2678b4e.ngbuilds.io/.
You can preview 442de90 at https://pr43218-442de90.ngbuilds.io/.
You can preview 4068711 at https://pr43218-4068711.ngbuilds.io/.

You can preview 8420d60 at https://pr43218-8420d60.ngbuilds.io/.

@gkalpak what do you think, is this one ready to go? 😃

It still needs one more public-api approval. Then it's good to go 🚀

This PR was merged into the repository by commit 835987b.

@dario-piotrowicz just wanted to say Thank You for all your work on this PR (including communication and addressing the feedback), this is very impressive (150 files changed)! 👍

@dario-piotrowicz just wanted to say Thank You for all your work on this PR (including communication and addressing the feedback), this is very impressive (150 files changed)! +1

Thanks a lot @AndrewKushnir! ❤️

But I can't take all the credit, @gkalpak helped me a lot, and even added a couple of commits for me too 😍
(also everyone who chipped in and reviewed the PR has been of great help as well ☺️)

The cool thing is, I think (I may need to double check), that with this PR merged all instances of tslint have been removed from all of the aio project (including the site, tooling and here the examples) 😃 🥳 🚀 😎 🎉

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}