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: add eslint as linter #1315

Draft
wants to merge 4 commits into
base: develop
Choose a base branch
from
Draft

chore: add eslint as linter #1315

wants to merge 4 commits into from

Conversation

pano9000
Copy link
Contributor

@pano9000 pano9000 commented Mar 1, 2025

Hi,

this PR adds an initial eslint configuration that we can use for linting our code base.
https://typescript-eslint.io/

It still needs some rule refinement for our current stage, because we still have lots of "any" types, which it complains about with the default rules

✖ 1030 problems (1030 errors, 0 warnings)
  60 errors and 0 warnings potentially fixable with the `--fix` option.

majority is complains about use of "any".

I'll likely work on the rules first, before marking the PR as ready :-)

closes #1293

pano9000 added 3 commits March 1, 2025 10:48
rules will need some finetuning still
purposely *not* named as dev:eslint, just to "decouple" the script from eslint, in case there ever is the need to change that :-)
@pano9000
Copy link
Contributor Author

pano9000 commented Mar 2, 2025

so, I've ran a quick count on the errors using
npm run dev:linter-check | grep -Po "@.+$" | sort | uniq -c | sort -hr

    363 @typescript-eslint/no-explicit-any
    244 @typescript-eslint/no-unused-vars
     50 @typescript-eslint/no-empty-object-type
     22 @ts-expect-error" instead of "@ts-ignore", as "@ts-ignore" will do nothing if the following line is error-free  @typescript-eslint/ban-ts-comment
     15 @typescript-eslint/no-wrapper-object-types
      5 @typescript-eslint/no-unused-expressions
      3 @typescript-eslint/no-require-imports
      2 @typescript-eslint/no-this-alias
      1 @typescript-eslint/no-unsafe-function-type

(edit: the data above is only showing the typescript-eslint related entries, the "regular" eslint rules are missing, just if anyones wondering, why the numbers don't add up).

The "no explicit any" error is not that bad after all – I though it would've been a higher count, after doing a first scroll through :-D
I'd maybe say let's keep the check active and keep it as reminder to get rid of as much any as possible – gradually.
(and in the cases, where it is not really possible → we can use eslint ignore comments)

@eliandoran what do you think?

@pano9000
Copy link
Contributor Author

pano9000 commented Mar 2, 2025

quick aside, regarding failing CI:
since we now depend on TypeScript 5.8.2, we will need to wait until typescript-eslint is updated to support 5.8 as well

looks like it will land soon though :-)
typescript-eslint/typescript-eslint#10903

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.

Investigate Linting Tool for Codebase
1 participant