tools-aoeur / eslint-config

0 stars 0 forks source link

fix: Don't apply rules that require type information to JS files #11

Closed amannn closed 2 weeks ago

amannn commented 2 weeks ago

I finally found the culprit that we previously discussed in https://github.com/tools-aoeur/eslint-config/pull/7#discussion_r1827883240.

TS-only rules fail when being run on JavaScript files. The error I previously saw in Horusnome was caused by a JavaScript file in . (probably eslint.config.mjs), therefore the fix was to run ESLint only on src. Now, in the CL I ran into the error again, but was able to isolate it to linting a JavaScript file within src (src/vendor/react-split-pane/pane.js).

The fix is to set parser options always when the typescript preset is used.

The error for reference:

Oops! Something went wrong! :(

ESLint: 9.12.0

Error: Error while loading rule '@typescript-eslint/dot-notation': You have used a rule which requires type information, but don't have parserOptions set to generate type information for this file. See https://typescript-eslint.io/getting-started/typed-linting for enabling linting with type information.
Parser: typescript-eslint/parser
menelaos commented 2 weeks ago

I don't quite understand what you mean by "TS-only rules". I think the problem here is that some rules require type information (e.g. those in stylisticTypeChecked). For those we need to set the languageOptions (as described in the documentation).

Due to https://github.com/tools-aoeur/eslint-config/blob/45d5f6b70081b1962593d67127ce2eaf999656e5/src/typescript.js#L24, this option was only set for TS files.

As typescript-eslint handles JS files just fine (after all, TS is a superset of JS), it tries to run the rule but is missing the type information.

So the fix here is correct.

I think, however, that there is another issue. If I read your comment correctly

// Only apply TypeScript rules to TypeScript file [...]

you intend to run @typescript-eslint rules only on TS files. This is currently not the case.

You can check this by inserting /* tslint:disable */ somewhere in src/vendor/react-split-pane/pane.js.

This will trigger the ban-ts-lint-comment rule which is part of stylisticTypeChecked.

So a TypeScript rule is applied to a JavaScript file.

As I mentioned in https://github.com/tools-aoeur/eslint-config/pull/3/files#r1807474756, I'm not sure whether this is really a problem, but if you intend to keep it this way, we should probably use the approach from the typescript-eslint documentation: https://typescript-eslint.io/getting-started/typed-linting/#how-can-i-disable-type-aware-linting-for-a-subset-of-files

amannn commented 2 weeks ago

You can check this by inserting /* tslint:disable */ somewhere in src/vendor/react-split-pane/pane.js.

Interesting find, yes. Seems to be the case because we unconditionally spread typescriptEslint configs into the exported config.

Will give the approach for opting-out JS files a shot.

amannn commented 2 weeks ago

Yep, seems to work too! 👍