sveltejs / language-tools

The Svelte Language Server, and official extensions which use it
MIT License
1.25k stars 200 forks source link

svelte-check: check only files mentioned as arguments #353

Closed AlexAegis closed 3 months ago

AlexAegis commented 4 years ago

I'm using lint-staged with husky to automatically lint files before commits. In large codebases it could be useful to lint only the changed files

If this is not possible because it needs to read the entire workspace to validate even a single file, then I don't think this feature is worth the effort

example usage:

svelte-check src/app.svelte src/button.svelte
FredKSchott commented 4 years ago

If this were performant, I'd love to move the Snowpack Svelte plugin onto this (since we're already running our own file watcher internally, running svelte-check --watch in parallel works but is a bit overkill).

dummdidumm commented 4 years ago

I guess your use case would be to only check the changed files? Not sure if this would have better performance in this case because starting a new svelte-check each time means the TypeScript compiler cannot reuse the previous output and will likely take longer to construct the new program. Also the checks might not give all diagnostics: if you change one file and get a type error in another unchanged file because of this, that error will not show up. I think for your case it would be better to provide some kind of JavaScript API to have more fine grained control over svelte-check.

dummdidumm commented 4 years ago

Having said the above it came to me that it may not be good to add the requested feature for the reasons outlined above: svelte-check is not a per-file linter, it also does checks across svelte files. Suppose you change Svelte file A by changing a property name. You forget to change that property usage in Svelte file B. If you only check the changed files, that bug would not get caught.

The request for a svelte-check JavaScript API is unrelated to that.

AlexAegis commented 4 years ago

means the TypeScript compiler cannot reuse the previous output and will likely take longer to construct the new program

Could incremental builds aleviate this? https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-4.html

svelte-check is not a per-file linter, it also does checks across svelte files.

I feared that in the issue description. For this svelte-check would need to use the provided file list as an affected module list and traverse the directly referenced modules and check those too. TypeScript compilation would be needed but if the aforementioned incremental builds are viable (Maybe not? I don't know if the svelte compiler would need a similar functionality for this to be viable) then it would be possible.

dummdidumm commented 4 years ago

If we would construct a module graph we would need to read all files anyway. I also would not want to implement such a module resolution myself and rather rely on the ts language service to do this. The second step would be to do some kind of "find references" on each changed file and then delete all unreferenced from the language server. I'm not sure this would save that much time in the end. The --incremental flag sounds like a more promising solution to speed things up. But I'm not sure if we should magically use that flag, maybe rather add a readme entry like "takes too long? Try incremental builds".

dummdidumm commented 3 months ago

Closing as we won't implement this. As pointed out above, svelte-check is not a per-file linter, it needs the whole module graph (similar to TypeScript) to emit useful diagnostics.