-
-
Notifications
You must be signed in to change notification settings - Fork 216
fix: respect tsconfig/jsconfig exclude patterns in file watcher #2807
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
base: master
Are you sure you want to change the base?
fix: respect tsconfig/jsconfig exclude patterns in file watcher #2807
Conversation
Thank you for the contribution, but this isn't the right approach. We already have Also, as I mentioned in #2797. I think the right approach is to watch only directories that match the |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as outdated.
This comment was marked as outdated.
41cd9b4
to
57e5c64
Compare
@jasonlyu123 I have cleaned this up. |
- Add getProjectConfig() to language server to expose parsed tsconfig - Process wildcard directories in svelte-check to determine watch paths - Support both recursive and non-recursive directory watching based on TypeScript's configuration - Handle relative paths correctly for directories outside workspace This ensures svelte-check only watches directories included by the tsconfig, improving performance and avoiding unnecessary file watching.
d269e56
to
09930e2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, I left some comments.
addressed feedback, thank you! |
There is another problem with this change. This will no longer watch the files that are loaded through module resolution and under the workspace root, but not under the "include" pattern. Of course, the old behaviour isn't ideal either. It should also watch files that aren't under the workspace root. Having thought about this more, I am wondering if we should add the parent directory of these files to the result of |
Thoughts on this solution? |
8b53cb8
to
c63b64f
Compare
* Optional callback invoked when a new snapshot is created. | ||
* Used by svelte-check to dynamically add parent directories to file watchers. | ||
*/ | ||
onSnapshotCreated?: (dirPath: string) => void; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
onSnapshotCreated
paired with a directory feels weird. It would probably be better if it sent the filename or replaced onSnapshotCreated
with a more specific name.
|
||
private async updateWatchedDirectories() { | ||
const watchDirs = await this.svelteCheck.getWatchDirectories(); | ||
const dirsToWatch = watchDirs || [{ path: this.workspaceUri.fsPath, recursive: true }]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't see the usage of recursive here. Did you forget to pass it to chokidar, by chance? Also, maybe the currentWatchedDirs
should also keep the recursive information. This way, we can skip the directory watch if the directories were watched recursively.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah. Chokidar doesn't support specifying recursive here. I'm fine with it always being recursive. In this case, we don't need to keep the recursive info here. We just need to check if it's under a directory we already watched.
this.watcher.add(dir); | ||
this.currentWatchedDirs.add(dir); | ||
// New files might now be visible; schedule a run | ||
this.scheduleDiagnostics(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we don't need this. If a new file is added, either it got added because of a file-watcher or it's added because of module resolution. In both ways, the file watcher should already trigger scheduleDiagnostics
.
Fix: Respect tsconfig exclude patterns in file watcher
svelte-check --watch was watching all files in the workspace regardless of what the tsconfig said to include/exclude. This caused performance issues in large projects and unnecessary diagnostics for excluded files.
What we did
How it works
TypeScript already parses the tsconfig and figures out which directories to watch via its wildcardDirectories property - this has all the include/exclude logic already applied. We just use that instead of watching everything.
When the tsconfig changes, we diff the old and new directory lists and use chokidar's add() and unwatch() methods to update what we're watching without recreating the watcher.