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

Docs: mention type checked rules being incompatible with eslint caching #6373

Closed
4 tasks done
benjaminjkraft opened this issue Jan 24, 2023 · 3 comments
Closed
4 tasks done
Labels
duplicate This issue or pull request already exists

Comments

@benjaminjkraft
Copy link

Before You File a Bug Report Please Confirm You Have Done The Following...

  • I have tried restarting my IDE and the issue persists.
  • I have updated to the latest version of the packages.
  • I have searched for related issues and found none that matched my issue.
  • I have read the FAQ and my problem is not listed.

Issue Description

We are using eslint caching to speed up our builds in a large repo. But we've now learned, the hard way, that typescript-eslint is not safe to use with eslint caching: because changes in one file can break lint in another, eslint caching can cause incorrect results. The repro below is for no-misused-promises but this applies to most of the typecheck-dependent rules.

As a start, it would be good to document this alongside #5845. The underlying reason is similar, but the documentation in #5845 suggests that this only applies to editors; here the workaround is to avoid using eslint caching or nuke the cache between runs. Ideally, the plugin could also warn if caching is enabled, although I don't know if it can tell. Of course I'd love to hear any better workarounds folks have, as turning off caching for eslint globally isn't an option for us.

I'm not sure how possible it would be to fix this in a better way. In the Go linting world, there is a concept of what data can be passed between runs of the linter on separate files (so here the linter would persist the output types), such that caching can depend on that data (here we'd only re-run lint on files whose imports' type changed). But it's not clear to me how well this would even work for typescript, where import type dependencies can be circular. It would also presumably require significant changes to upstream eslint's caching architecture, unless there's a way for typescript-eslint to hook into that or to hack this in from the plugin side.

Reproduction Repository Link

https://github.com/benjaminjkraft/typescript-eslint-caching-issue

Repro Steps

see the README

Versions

latest everything, see the repro package.json for details

@benjaminjkraft benjaminjkraft added bug Something isn't working triage Waiting for maintainers to take a look labels Jan 24, 2023
@JoshuaKGoldberg
Copy link
Member

Related:

This got asked of me a few times over the last conferences/meetups I've presented at. So I think it'd be good to add docs to https://typescript-eslint.io/linting/troubleshooting. Thanks for filing @benjaminjkraft!

@JoshuaKGoldberg JoshuaKGoldberg added documentation Documentation ("docs") that needs adding/updating accepting prs Go ahead, send a pull request that resolves this issue and removed triage Waiting for maintainers to take a look labels Jan 24, 2023
@bradzacher
Copy link
Member

bradzacher commented Jan 24, 2023

This is just a known limitation of eslint caching - it does not allow plugins to specify file dependencies because it just assumes every rule operates on a single file.

However there are many rules that don't operate on single files. Our type-aware rules are one example and many of eslint-plugin-import's rules are another example.

Most users don't use caching because (a) it's not well publicised and (b) it's just fraught with foot guns. But regardless it would be a great idea to document it.

@JoshuaKGoldberg JoshuaKGoldberg changed the title Bug: typescript-eslint incompatible with eslint caching Docs: mention type checked ruels being incompatible with eslint caching Dec 18, 2023
@JoshuaKGoldberg JoshuaKGoldberg changed the title Docs: mention type checked ruels being incompatible with eslint caching Docs: mention type checked rules being incompatible with eslint caching Dec 18, 2023
@JoshuaKGoldberg
Copy link
Member

JoshuaKGoldberg commented Dec 18, 2023

This has ended up being a dup of #4694. Re-opening that one and marking this as a dup. Cheers!

And, thanks for filing - this was useful for us to bump up the priority of documenting caching.

@JoshuaKGoldberg JoshuaKGoldberg closed this as not planned Won't fix, can't repro, duplicate, stale Dec 18, 2023
@JoshuaKGoldberg JoshuaKGoldberg added duplicate This issue or pull request already exists and removed bug Something isn't working documentation Documentation ("docs") that needs adding/updating accepting prs Go ahead, send a pull request that resolves this issue labels Dec 18, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
duplicate This issue or pull request already exists
Projects
None yet
Development

No branches or pull requests

3 participants