ESLint does not re-compute cross-file information on file changes

[JoshuaKGoldberg]

There's a known issue with typescript-eslint that modifying one file doesn't impact the view of that file's types seen by other files:

• https://github.com/JoshuaKGoldberg/repros/tree/repro-vs-code-reparse-typescript-eslint has a reproduction & issue
• Docs: Add FAQ entry for out-of-date typed rule reports on file changes in editors such as VS Code typescript-eslint/typescript-eslint#8291 has more details

Copying that issue's explanation here: @typescript-eslint/parser sets up a TypeScript program behind-the-scenes when it parses files. The program for a file is later made on ESLintUtils.getParserServicescontext).program. This is documented in Custom Rules > Typed Rules. That program is what's used for type information in typed lint rules.

One known issue with the parser-generating-type-information strategy is that programs are only recreated when ESLint re-parses. I'm not sure how best to solve this on the editor side, so asking for help here 🙂. Would it be reasonable for vscode-eslint to trigger an ESLint re-parse whenever a file is modified on disk?

Edit: summarizing discussion below, this is not limited to just typescript-eslint's type-checked rules. Any lint rule that uses cross-file information is impacted. That includes plugins like eslint-plugin-import / eslint-plugin-import-x.

[dbaeumer]

@JoshuaKGoldberg thanks for reaching out to me.

Would it be reasonable for vscode-eslint to trigger an ESLint re-parse whenever a file is modified on disk

How does that re-parse API look like. What I would like to avoid is that the ESLint server needs to know about dependencies in TS files.

[dbaeumer]

And would there be API for me to find out (on the ESLint level) if the parser has TS type information loaded to avoid this in cases where it is not necessary.

[JoshuaKGoldberg]

How does that re-parse API look like

Looking at interface ESLintClass and its await eslintClass.lintText usage, I'm thinking calling the same validate(...) should be sufficient? Is that what you mean?

avoid this in cases where it is not necessary

Yeah the difficulty there is in theory any file can cause this issue. A global.d.ts that augments a global type, for example, could impact any other file in the project. In order to know about TypeScript augmentations you'd have to talk to the TypeScript language service (guessing: not doable?)... but even that wouldn't be enough for everyone:

• Ezno, Flow, and other non-TypeScript typed languages have different semantics
• Other ESLint plugins may also present similar cross-file issues even in vanilla JS, such as eslint-plugin-import

I can think of a few directions...

• Assuming any parser customization might necessitate re-computing
• Adding a setting to explicitly disable/enable this

[dbaeumer]

A reasonable approach would be like this:

• having a setting to support inter file dependencies. When enabled the server could re-validate all open files (the eslint server knows about them anyways)
• for TS it would be cool if we could find out whether the plugin has TS symbol information. If yes, we could automatically enable the above setting
• have an action that enforces re-validation of all files. This is to avoid listening to file changes and doing the re-validation to often since the server has no idea which changes would need a re-validation.

A "correct" approach would be like this:

• the TypeScript ESLint plugin runs the TypeScript language service in watch mode. In this mode the service already listens to all relevant file changes and updates its program when necessary.
• the TypeScript ESLint plugin provides a way to notify about such a change and then the server could re-validate the files

This would be the most effective implementation since it would only revalidate files if the TS program has changed.

Let me know if you think the second approach would be doable for the plugin.

[JoshuaKGoldberg]

API for me to find out (on the ESLint level) if the parser has TS type information
find out whether the plugin has TS symbol information

Theoretically yes: the resolved ESLint parserOptions for the file can be checked to determine if project or EXPERIMENTAL_useProjectService are truthy. That would determine whether typical TypeScript files should be re-linted.

But that wouldn't fix things for the users of other multi-file-linting parsers mentioned in #1774 (comment). That list is non-exhaustive: I also remembered n/no-missing-import and n/no-missing-require suffer from this issue.

have an action that enforces re-validation of all files

Would this be noticeably better than the existing Restart ESLint Server action? That one is pretty fast in my experience. And most of the slowness folks experience from expensive linting occurs from out-of-file analysis that would happen in both commands.

the TypeScript ESLint plugin runs the TypeScript language service in watch mode

I think this watch mode is theoretically doable. Two difficulties:

• It would be a pretty big code addition. We're still iterating on EXPERIMENTAL_useProjectService and only plan on releasing it on stable in v7 v8 later this year (docs: blog post on parserOptions.projectService typescript-eslint/typescript-eslint#8031). The soonest it would likely land is H2 2024, very optimistically.
• There's no sanctioned way for a parser to know ESLint is running it in a long-lived session. feat: parsing session objects eslint/rfcs#102 was closed last year pending a longer-term rewrite of ESLint (Complete rewrite of ESLint eslint/eslint#16557). We'd want to coordinate with ESLint and other editors (e.g. WebStorm) to make sure other folks get the benefits too.

the TypeScript ESLint plugin provides a way to notify about such a change and then the server could re-validate the files

I suppose this is possible, with more of the same two difficulties noted in the previous session.

---

cc @bradzacher - who has often had a superset of my knowledge about these workings.

[ljharb]

This is relevant to eslint-plugin-import, since editing any file could cause any other file to become invalid. Tightly integrating the concept of “vscode” or “a language server” into individual eslint plugins (that shouldn’t ever need to be aware of such concepts) doesn’t seem like a viable or maintainable approach to me, altho ofc I’m willing to make changes if that would solve the problem.

[bradzacher]

for TS it would be cool if we could find out whether the plugin has TS symbol information

calculateConfigForFile will give you back the ESLint config - then really just need to check if parserOptions.project is truthy.

We can build and maintain a util for you (eg pass a config and we return a boolean) to avoid you relying on our implementation details.

That being said - as Jordan mentioned there are more usecases for "refresh" beyond type info in the eslint ecosystem so tying it to one subset is probs more limited than we'd ultimately want.

---

FWIW I've asked eslint for a way to register file dependency at lint time and the response was essentially "not until we rewrite eslint". So we can either build a paradigm outside their APIs, or we can wait (probably another few years). The former is burden for us (or mostly the IDE extensions), yes, but would fix the DevX that everyone currently hates and complains about to us - a lot.

[bradzacher]

the TypeScript ESLint plugin runs the TypeScript language service in watch mode

This wouldn't help us. We already do this really - just using the lower-level APIs. But we don't attach file watchers because eslint provides no mechanism for us to know when the "lint run is done" - so we don't know when to detach the watchers and if we don't do that the ESLint CLI never exits.

But regardless - the server in watch mode doesn't really provide us with any wins because we don't coordinate the lint run - eslint does. Us recalculating types eagerly doesn't do anything unless a lint run is done.

Once the user saves a file it'll trigger a fresh lint run on the file which will make eslint call us and we'll recalculate the types. After that if you "re lint all open files" they should be mostly free to lint because we don't need to recalculate types.

[dbaeumer]

Thanks for all the comments. Reading through them shows that there isn't an easy solution to fix this since none of the participants has enough information right now to do the proper calculation whether a reparse is necessary or not. To summarize things:

• The VS Code ESLint extension could do it for TypeScript by checking the config.parserOptions.project property
• This will not work for plugins that have inter file dependencies but don't depend on TypeScript
• Adding a setting to VS Code to signal inter file dependencies is bad since most users will not be aware of this.
• Always re-linting all files is bad as well since it is IMO not necessary for 90% of the rules and will have bad performance impacts.

I also understand that not every rule want to build a dependency tree and have file watchers to determine this itself. Since VS Code as a file change and watcher capabilities it would be OK for me if the extension does the re-linting however I would like to request some more support from ESLint as well. Ideal would a solution where the ESLint extension could find out for every rule if it has inter file dependencies or not. Does anyone know if something is possible today? I haven't found an API to get to a rule object. If something like this is not possible I think we need to talk to the ESLint maintainers to see if something like this could be added.

What I could do is the following:

• assume inter file dependencies for TypeScript
• re-lint a file on focus again. This will at least remove an error as soon as a file gets focused and doesn't need additional user action.

Any additional thought?

[bradzacher]

In terms of eslint building something - we may be on our own for now, at least until eslint completes it's rewrite. Based on past discussions it sounds like they don't want to make any changes to the core until the rewrite.

Refs:

• Add ability to report dependencies for rule error eslint/eslint#15583 (this is the most relevant one)
• feat: parsing session objects eslint/rfcs#102
• Feedback Wanted: Long-running ESLint sessions eslint/eslint#16818
• Feedback Wanted: Project-aware ESLint eslint/eslint#16819
• Complete rewrite of ESLint eslint/eslint#16557
  • Complete rewrite of ESLint eslint/eslint#16482
• Plugin initialization API eslint/eslint#14139

[bradzacher]

• assume inter file dependencies for TypeScript
• re-lint a file on focus again. This will at least remove an error as soon as a file gets focused and doesn't need additional user action.

I'd be okay with either of these. Though the latter wouldn't clear the problems pane - at least it would clear as soon as the user focuses the file.

---

Always re-linting all files is bad as well since it is IMO not necessary for 90% of the rules and will have bad performance impacts.

If we're talking re-linting open files I'd have thought most user sessions would have at most a dozen files up at any given time? (I have no idea - pulling a number out by butt based on how I use it) so a full session lint should be fast - esp considering most files don't need type info re-calculated and just need the lint rules to run.
But regardless you're not wrong - linting the entire session on file change would be ridiculously slow for the user.

What about if we did it just asynchronously on save instead? Is that doable? Idk if vscode exposes the signals to do that.

[dbaeumer]

What about if we did it just asynchronously on save instead? Is that doable? Idk if vscode exposes the signals to do that.

VS Code has auto save and it is on for a lot of users. For them this would result in some performance problems as well.

[bradzacher]

Could we do a "throttle" on the event?
EG trigger a "session" lint on save but no more than once every 10s, and it is run asynchronously after the current file has been linted to completion (to ensure the type info is recalculated).

If we pair that with "re lint file on focus if a change has occurred since the file was last linted" then we could have good coverage of everything without being too expensive:

• we don't burn cycles for autosave users
• we don't burn cycles for compulsive savers
• we are still able to show the latest lint state for a file even if the throttle prevents a latest lint

The trade-off here is implementation complexity, of course.

If we could make the extension aware of the explicit dependencies of a given file then we'd be well off - though IIRC it's actually pretty hard to get this information performantly out of the ts.Program.