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

[FEATURE REQUEST] Modernize ESLint #1625

Open
elliot-huffman opened this issue May 2, 2024 · 5 comments
Open

[FEATURE REQUEST] Modernize ESLint #1625

elliot-huffman opened this issue May 2, 2024 · 5 comments

Comments

@elliot-huffman
Copy link
Contributor

Is your feature request related to a problem? If so, please give a brief summary of the problem and how the feature would resolve it

ESLint is moving to the new flat file config spec and the current *eslint* file set that you are using is deprecated.

Additionally, are all the rules that are currently set the set of rules that you want to keep?
You don't have the typed strict rules enabled which can pair down your existing ruleset by having the best practices enforced by the linter automatically without having to explicitly configure every rule.

Also, some of your style rules are not natively supported by VS Code auto formatting, would you like to align your linting config to VS Code's auto formatting for: spaces, commas, brackets, new line and other built-in formatters?
I can closely match your current styles, there are just some semi colons that aren't fully supported in VS Code.
We can also enforce more auto styling in VS Code if this is something you want to do as you only do a subset of stylization at the moment.

Describe the preferred solution

  • Migrate to the flat file config spec.
  • Modernize/review lint rules
  • Migrate from custom spec to ruleset

Describe alternatives you've considered

Not updating deps? (bad practice)

Additional context

https://typescript-eslint.io/blog/announcing-typescript-eslint-v7
https://eslint.org/blog/2024/04/eslint-v9.0.0-released/

Rulesets:
https://typescript-eslint.io/getting-started/#additional-configs

Reference Documentations/Specifications

https://eslint.org/docs/latest/use/configure/configuration-files

@arthurschreiber
Copy link
Collaborator

All of this sounds sensible to me! Looking forward to see pull requests for this coming in. 🙇‍♂️

@arthurschreiber
Copy link
Collaborator

ESLint is moving to the new flat file config spec and the current *eslint* file set that you are using is deprecated.

👍

Additionally, are all the rules that are currently set the set of rules that you want to keep?
You don't have the typed strict rules enabled which can pair down your existing ruleset by having the best practices enforced by the linter automatically without having to explicitly configure every rule.

I don't quite remember why I enabled specific rules instead of using one of the presets. I guess switching to a pre-set could help me remember why we didn't use one in the first place. 😅

Also, some of your style rules are not natively supported by VS Code auto formatting, would you like to align your linting config to VS Code's auto formatting for: spaces, commas, brackets, new line and other built-in formatters?

Yeah, if we can do that, that would be great. Would definitely make it easier for people that use VS Code / Codespaces to work on tedious.

@elliot-huffman
Copy link
Contributor Author

elliot-huffman commented May 3, 2024

I have the configs converted and wow, after enabling the recommended rule sets, there are a lot of fixes that have been bubbled up.
I am going to break this into two parts.
The ES Lint config update and the rules fixes.

Please see the below for work I am gonna start on:
screenshot of linting info with recommended rulesets enabled

It looks like the data is not properly typed throughout the project.
The majority of errors are unsafe 'any' assignments, which in most cases is lack of generics configurations or someone suppressing type checking by placing an any type on something that should have unknown or the correct type instead. any could be used to suppress type checking to hide stuff or to reduce the amount of validation that is required on an object, and in these cases, this is usually a security risk due to incorrect or assumed data.
Looks like I will also have to convert some files that are currently JavaScript files to typescript, I see a few in the tests folder.

@elliot-huffman
Copy link
Contributor Author

elliot-huffman commented May 3, 2024

PR Part 1 is up, let me know what you all think #1631 :-)
The linting fixes will be introduced as additional commits to it, hence the draft state.

@elliot-huffman
Copy link
Contributor Author

elliot-huffman commented May 7, 2024

Got the error count down to ~150 with 50 auto-fixable.

screenshot of the lint results

The config is now almost identical to your original config. It appears that some of the new issues that are bubbling up are due to updated rulesets because of the updated packages.
These issues appear to be correctly identified by the linting system.

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

No branches or pull requests

2 participants