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

Disable more rules covered by TypeScript in the typescript config #3135

Open
FloEdelmann opened this issue Jan 2, 2025 · 13 comments
Open

Disable more rules covered by TypeScript in the typescript config #3135

FloEdelmann opened this issue Jan 2, 2025 · 13 comments

Comments

@FloEdelmann
Copy link
Contributor

The typescript config already disables the import/named rule because it does not have an advantage over errors thrown by the TypeScript compiler:

// TypeScript compilation already ensures that named imports exist in the referenced module
'import/named': 'off',

There are some more rules that fall in the same category, which also typescript-eslint recommends to disable:

To avoid checking the same things the TypeScript compiler has already checked, I think it makes sense to also disable those rules (maybe except for import/no-unresolved) in the typescript config, so users don't have to (know and) disable them manually.

(It would also mitigate #2340 for TypeScript users.)

@soryy708
Copy link
Collaborator

soryy708 commented Jan 8, 2025

What if the user is on an old version of TypeScript?

@FloEdelmann
Copy link
Contributor Author

I guess that those users will then not be on the latest version of eslint-plugin-import either, so that shouldn't be an issue.

@ljharb
Copy link
Member

ljharb commented Jan 10, 2025

That’s not a reasonable assumption; many things might delay a typescript upgrade that wouldn’t affect an upgrade of an eslint plugin.

@FloEdelmann
Copy link
Contributor Author

FloEdelmann commented Jan 10, 2025

I just tried the ancient TypeScript 1.8.10 (latest minor version of v1, released in 2016), and all of the checks were already included in that version. See my Stackblitz and run npm run build there.

@ljharb
Copy link
Member

ljharb commented Jan 10, 2025

Then in that case it shouldn't be an issue :-)

@michaelfaith
Copy link
Contributor

So move forward with the disable on those rules? What about import/no-unresolved?

@ljharb
Copy link
Member

ljharb commented Jan 10, 2025

It'd be fine to configure that rule to be import: false, but it'd have to have require: true (off the top of my head I can't remember how granular that rule gets). Worst case we can just leave that rule enabled.

@michaelfaith
Copy link
Contributor

michaelfaith commented Jan 10, 2025

I was just looking at the settings, and there doesn't seem to be a way to turn off just import. The only options are around case sensitivity and ignoring specific modules.

@ljharb
Copy link
Member

ljharb commented Jan 10, 2025

Then best to play it safe and leave that one enabled.

@michaelfaith
Copy link
Contributor

michaelfaith commented Jan 10, 2025

Oh, wait. actually, require checking isn't on by default anyway. There's an option for enabling require checks, but the recommended config isn't enabling it. So the rule that's declared in the recommended config is currently only checking imports. In which case turning it off in the typescript config would only be impacting imports anyway.

Image

@ljharb
Copy link
Member

ljharb commented Jan 10, 2025

That's fair, altho we should probably enable require:true in the recommended config anyways :-p

@michaelfaith
Copy link
Contributor

Wouldn't that be a breaking change though? Or did you mean sometime down the road when the next major happens?

@ljharb
Copy link
Member

ljharb commented Jan 11, 2025

At the next major, yeah

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

No branches or pull requests

4 participants