Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Convert ES Lint to Flat Config #1631

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

elliot-huffman
Copy link

@elliot-huffman elliot-huffman commented May 3, 2024

Changes:

Change the packages to TypeScript Lint to the next gen package.
Migrate all the settings over from the existing ESLint config.
Remove `deprecated` and `removed` rules.
Enable the typescript strict type checked and stylistic checked rule collections.
Update lint config to ES 2021 instead of ES6.
Add the new ESLint config to the typescript config so that it is aware if it and can provide typing services in the config file.
The next gen version of ES Lint no longer supports passing multiple values with a single instance of the parameter, now it needs to be split into multiple calls of the param.

The ext param is no longer needed however as ESlint knows what it should and should not lint.

Split out the TSC command into a post script since it is technically its own invocation.
The Lib, Examples and benchmarks folders should be excluded for now.
I would recommend including them in the future as everything should be linted, including examples.
Items are excluded in the linter config, and should not be in the `package.json`.
The `eqeqeq` option should be stricter as JavaScript's type safety is notoriously lacking..
@elliot-huffman elliot-huffman marked this pull request as draft May 3, 2024 20:03
@elliot-huffman
Copy link
Author

Once the #1630 PR is merged, then I can add the IDE configs required for the new flat files into this PR

@elliot-huffman elliot-huffman changed the title Convert ES Lint to Flat Config chore: Convert ES Lint to Flat Config May 3, 2024
Copy link

codecov bot commented May 4, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.45%. Comparing base (934e98e) to head (e07eb2f).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1631      +/-   ##
==========================================
- Coverage   79.19%   78.45%   -0.75%     
==========================================
  Files          93       93              
  Lines        4860     4860              
  Branches      933      933              
==========================================
- Hits         3849     3813      -36     
- Misses        707      750      +43     
+ Partials      304      297       -7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@arthurschreiber
Copy link
Collaborator

This is great, thank you! ❤️

Would you mind changing this pull request to only focus on switching to the flat config, without changing the rules that are applied? I'm open to changing the linting rules, but I'd prefer a separate PR per changed lint rule so that we can have all config change plus all the required code changes per rule change together.

Otherwise, this PR will just become too unwieldy and impossible to review. 🙇‍♂️

@elliot-huffman
Copy link
Author

elliot-huffman commented May 4, 2024 via email

These will be enabled in future PRs...
@elliot-huffman
Copy link
Author

@arthurschreiber, the linting rulesets have been disabled. I will enable one ruleset at a time and have the contents of the project fixed and make that a PR.

@arthurschreiber arthurschreiber marked this pull request as ready for review May 6, 2024 23:22
Add the parser configs that are necessary to run the typescript linter.
Remove duplicate rule config and and use the more specifically configured option as the source of truth.
Enable the es lint recommended ruleset, the same way that it was configured in the legacy config.
Read here for the specifics of what it does:
https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/typescript-eslint/src/configs/eslint-recommended.ts
@elliot-huffman
Copy link
Author

elliot-huffman commented May 7, 2024

no-loss-of-precision is a new rule that was just introduced to check for number casting. It was not previously listed in the rule config and has found a bunch of potential cast errors (that look accurate to me).
This commit will fail linting because of this new rule that is enabled by default.
This is because the package version were bumped to 7.8.0 up from 7.0.2. 7.1.0 introduced this new rule.

Configured these rules, even through they are not best practice so that lint would pass more.
(Upon request)
This directive was not used, so removing the disable comment.
@arthurschreiber
Copy link
Collaborator

arthurschreiber commented May 7, 2024

The reason for the new lint failures for files in the test folder is because there exists a separate test/.eslintrc file that defines the linter configuration for these files. Can you convert that into the new config format as well? That should remove all the test failures.

We can later focus on aligning the configuration between the test folder and the rest of the source to use the same linter settings.

@elliot-huffman
Copy link
Author

The reason for the new lint failures for files in the test folder is because there exists a separate test/.eslintrc file that defines the linter configuration for these files. Can you convert that into the new config format as well? That should remove all the test failures.

We can later focus on aligning the configuration between the test folder and the rest of the source to use the same linter settings.

Oh, I missed this, can do!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants