-
-
Notifications
You must be signed in to change notification settings - Fork 215
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?
Changes from all commits
09930e2
97db972
ea51052
12f3c0c
c63b64f
3bd5b2c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,7 +2,7 @@ | |
* This code's groundwork is taken from https://github.com/vuejs/vetur/tree/master/vti | ||
*/ | ||
|
||
import { watch } from 'chokidar'; | ||
import { watch, FSWatcher } from 'chokidar'; | ||
import * as fs from 'fs'; | ||
import { fdir } from 'fdir'; | ||
import * as path from 'path'; | ||
|
@@ -143,49 +143,93 @@ async function getDiagnostics( | |
} | ||
} | ||
|
||
const FILE_ENDING_REGEX = /\.(svelte|d\.ts|ts|js|jsx|tsx|mjs|cjs|mts|cts)$/; | ||
const VITE_CONFIG_REGEX = /vite\.config\.(js|ts)\.timestamp-/; | ||
|
||
class DiagnosticsWatcher { | ||
private updateDiagnostics: any; | ||
private watcher: FSWatcher; | ||
private currentWatchedDirs = new Set<string>(); | ||
private userIgnored: Array<(path: string) => boolean>; | ||
private pendingWatcherUpdate: any; | ||
|
||
constructor( | ||
private workspaceUri: URI, | ||
private svelteCheck: SvelteCheck, | ||
private writer: Writer, | ||
filePathsToIgnore: string[], | ||
ignoreInitialAdd: boolean | ||
private ignoreInitialAdd: boolean | ||
) { | ||
const fileEnding = /\.(svelte|d\.ts|ts|js|jsx|tsx|mjs|cjs|mts|cts)$/; | ||
const viteConfigRegex = /vite\.config\.(js|ts)\.timestamp-/; | ||
const userIgnored = createIgnored(filePathsToIgnore); | ||
const offset = workspaceUri.fsPath.length + 1; | ||
this.userIgnored = createIgnored(filePathsToIgnore); | ||
|
||
watch(workspaceUri.fsPath, { | ||
// Create watcher with initial paths | ||
this.watcher = watch([], { | ||
ignored: (path, stats) => { | ||
if ( | ||
path.includes('node_modules') || | ||
path.includes('.git') || | ||
(stats?.isFile() && (!fileEnding.test(path) || viteConfigRegex.test(path))) | ||
(stats?.isFile() && | ||
(!FILE_ENDING_REGEX.test(path) || VITE_CONFIG_REGEX.test(path))) | ||
) { | ||
return true; | ||
} | ||
|
||
if (userIgnored.length !== 0) { | ||
path = path.slice(offset); | ||
for (const i of userIgnored) { | ||
if (i(path)) { | ||
if (this.userIgnored.length !== 0) { | ||
// Make path relative to workspace for user ignores | ||
const workspaceRelative = path.startsWith(this.workspaceUri.fsPath) | ||
? path.slice(this.workspaceUri.fsPath.length + 1) | ||
: path; | ||
for (const i of this.userIgnored) { | ||
if (i(workspaceRelative)) { | ||
return true; | ||
} | ||
} | ||
} | ||
|
||
return false; | ||
}, | ||
ignoreInitial: ignoreInitialAdd | ||
ignoreInitial: this.ignoreInitialAdd | ||
}) | ||
.on('add', (path) => this.updateDocument(path, true)) | ||
.on('unlink', (path) => this.removeDocument(path)) | ||
.on('change', (path) => this.updateDocument(path, false)); | ||
|
||
if (ignoreInitialAdd) { | ||
this.updateWatchedDirectories(); | ||
} | ||
|
||
addWatchDirectory(dir: string) { | ||
if (!dir || this.currentWatchedDirs.has(dir)) { | ||
return; | ||
} | ||
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 commentThe 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 |
||
} | ||
|
||
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 commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
const newDirs = new Set(dirsToWatch.map((d) => d.path)); | ||
|
||
// Fast diff: find directories to add and remove | ||
const toAdd = [...newDirs].filter((dir) => !this.currentWatchedDirs.has(dir)); | ||
const toRemove = [...this.currentWatchedDirs].filter((dir) => !newDirs.has(dir)); | ||
|
||
// Add new directories | ||
if (toAdd.length > 0) { | ||
this.watcher.add(toAdd); | ||
} | ||
|
||
// Remove old directories | ||
if (toRemove.length > 0) { | ||
this.watcher.unwatch(toRemove); | ||
} | ||
|
||
// Update current set | ||
this.currentWatchedDirs = newDirs; | ||
|
||
if (this.ignoreInitialAdd) { | ||
this.scheduleDiagnostics(); | ||
} | ||
} | ||
|
@@ -210,6 +254,11 @@ class DiagnosticsWatcher { | |
this.scheduleDiagnostics(); | ||
} | ||
|
||
updateWatchers() { | ||
clearTimeout(this.pendingWatcherUpdate); | ||
this.pendingWatcherUpdate = setTimeout(() => this.updateWatchedDirectories(), 1000); | ||
} | ||
|
||
scheduleDiagnostics() { | ||
clearTimeout(this.updateDiagnostics); | ||
this.updateDiagnostics = setTimeout( | ||
|
@@ -264,8 +313,16 @@ parseOptions(async (opts) => { | |
}; | ||
|
||
if (opts.watch) { | ||
svelteCheckOptions.onProjectReload = () => watcher.scheduleDiagnostics(); | ||
const watcher = new DiagnosticsWatcher( | ||
// Wire callbacks that can reference the watcher instance created below | ||
let watcher: DiagnosticsWatcher; | ||
svelteCheckOptions.onProjectReload = () => { | ||
watcher.updateWatchers(); | ||
watcher.scheduleDiagnostics(); | ||
}; | ||
svelteCheckOptions.onSnapshotCreated = (dirPath: string) => { | ||
watcher.addWatchDirectory(dirPath); | ||
}; | ||
watcher = new DiagnosticsWatcher( | ||
opts.workspaceUri, | ||
new SvelteCheck(opts.workspaceUri.fsPath, svelteCheckOptions), | ||
writer, | ||
|
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 replacedonSnapshotCreated
with a more specific name.