Skip to content

Stale version of interface from another file being used on second linter run with caching disabled #4733

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

Closed
3 tasks done
kowsen opened this issue Mar 25, 2022 · 3 comments
Closed
3 tasks done
Labels
package: parser Issues related to @typescript-eslint/parser wontfix This will not be worked on

Comments

@kowsen
Copy link

kowsen commented Mar 25, 2022

  • I have tried restarting my IDE and the issue persists.
  • I have updated to the latest version of the packages.
  • I have read the FAQ and my problem is not listed.

Repro

If you run node index.js on this stackblitz and follow the instructions it prints, you'll run into the error: https://stackblitz.com/edit/node-skp6gk?file=index.js

Generally, I run into this when I have one file containing an interface with a property and another file that references that property. Then following these steps gets me a "@typescript-eslint/no-unnecessary-type-assertion" error on step 3:

  1. Run esLint.lintFiles(["code_file_path.ts", "interface_file_path.ts"])
  2. Make the property on the interface optional in interface_file_path.ts and add a non-null assertion to where we reference it in code_file_path.ts
  3. Run the same command from step 1

If I then kill the node process that I'm running the linter from and restart it, it doesn't throw the error on step 1. Additionally, if I swap the order that the files go into the linter that seems to fix it too.

Expected Result

I expect that without any caching enabled, there shouldn't be a case where running eslint.lintFiles on a file a second time would give me an error based on the state of the files when they were passed into eslint.lintFiles the first time.

Actual Result

Even after disabling any kind of ESLint caching, following the steps in the repro case causes a linter error even though the passed in files are correctly formatted.

Additional Info

Sorry if this is on the wrong repo, I don't understand enough about the inner workings to be sure this is the right place.

I ran into this using ESLintWebpackPlugin, which uses webpack's compilation hooks to determine what files have changed during a rebuild and passes an array of their absolute paths into eslint.lintFiles. Before opening a bug with them to see if there's a way to order this list to put dependencies before files they depend on, I want to make sure that the lintFiles function having a required order for files that are passed into it is deliberate.

Versions

package version
@typescript-eslint/parser 5.16.0
TypeScript 4.6.3
ESLint 8.11.0
node 14.19.0
@kowsen kowsen added package: parser Issues related to @typescript-eslint/parser triage Waiting for team members to take a look labels Mar 25, 2022
@bradzacher
Copy link
Member

I expect that without any caching enabled

We actually do do caching! We have to because for any non-trivial sized project it will take way too long to recalculate the state for the entire project.

However, for various reasons we cannot attach file watchers. ESLint isn't designed for this sort of persistent system.
So we work on the premise that no file changes on disk ever. We assume that any and and all changes will be linted by ESLint (and thus passed to our parser).

This is how an IDE works - all changes you make will be linted.
However if there are ever any stale lints you will have to close and re-open the file to trigger a new lint run using whatever latest changes you've made.

This is all also why we cannot work with ESLint's inbuilt caching mechanism.


In your given example the order matters because in one order we are informed of the changes in dependency order, and thus we can lint correctly. In the other order we cannot.

Sadly this isn't much we can do here. If the tool doesn't inform us of the changes in an order based on the file dependencies, then we can't work correctly.

@bradzacher bradzacher added awaiting response Issues waiting for a reply from the OP or another party and removed triage Waiting for team members to take a look labels Mar 26, 2022
@kowsen
Copy link
Author

kowsen commented Mar 28, 2022

Thank you so much for the quick response! So if I'm understanding you right, what I'm seeing here is the expected behavior and it's ESLintWebpackPlugin's responsibility to pass files into eslint.lintFiles in the proper order so I should file an issue against them?

@bradzacher
Copy link
Member

I wouldn't say that it's specifically their fault - they don't know how TS-eslint works or its
constraints.

In a nutshell linting over "changed files only" is exactly the same as doing an eslint cached lint run (it caches state to disk so the next run only lints changed files).

We do not and cannot support this style of linting.


Perhaps the webpack plugin can add some logic to satisfy most usecases? But it's probably going to be super difficult

As an example - if there are cycles in your dependency graph then it's not possible to satisfy the constraint of "in dependency order", which would cause buggy behaviour.

Most projects avoid runtime cycles, but type cycles are very, very common - which would cause problems.

I don't know if this is a solvable problem, unfortunately. We here have to focus on the main two usecases - CLI runs ("one and done" linting) and IDE runs ("continuous" linting).
The webpack plugin is close to the IDE usecase - but unless it does exactly the same things the IDE can do, it won't be 100% compatible.

I'd be open to having someone work on a specification to help aid this usecase! But it won't be a small amount of work, to say the least.

@bradzacher bradzacher added wontfix This will not be worked on and removed awaiting response Issues waiting for a reply from the OP or another party labels May 3, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
package: parser Issues related to @typescript-eslint/parser wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

2 participants