Skip to content
This repository was archived by the owner on Jun 24, 2025. It is now read-only.

chore: add eslint as linter #1315

Merged
merged 9 commits into from
Mar 5, 2025
Merged

chore: add eslint as linter #1315

merged 9 commits into from
Mar 5, 2025

Conversation

pano9000
Copy link
Member

@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 TriliumNext/Trilium#5524
closes TriliumNext/Trilium#5026

@pano9000
Copy link
Member 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
Member 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

@pano9000
Copy link
Member Author

pano9000 commented Mar 3, 2025

ok, it has been released -> will update and rebase the branch accordingly later this evening:
https://github.com/typescript-eslint/typescript-eslint/releases/tag/v8.26.0

@pano9000 pano9000 marked this pull request as ready for review March 3, 2025 22:09
@pano9000 pano9000 marked this pull request as draft March 3, 2025 22:09
@pano9000 pano9000 marked this pull request as ready for review March 3, 2025 22:11
@pano9000
Copy link
Member Author

pano9000 commented Mar 3, 2025

PR ready for a first start :-)
I'll keep this PR without actual eslint fixes – I'd do these in (one or several) separate steps :-)

@pano9000 pano9000 force-pushed the chore_add-eslint branch 3 times, most recently from 77d9423 to b568307 Compare March 4, 2025 08:37
pano9000 added 8 commits March 4, 2025 12:19
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 :-)
this is already used across codebase: using "_" as placeholder for deliberately unused vars (e.g. when destructuring arrays)
@eliandoran eliandoran merged commit 4c0ecc4 into develop Mar 5, 2025
5 checks passed
@eliandoran eliandoran deleted the chore_add-eslint branch March 5, 2025 10:55
@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Jun 21, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
size:M This PR changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Investigate Linting Tool for Codebase Linting Tool for Codebase
2 participants