diff --git a/tsc/internal/execute/build/orchestrator.go b/tsc/internal/execute/build/orchestrator.go index a5ca48c30c234..f90f3d3de7826 100644 --- a/tsc/internal/execute/build/orchestrator.go +++ b/tsc/internal/execute/build/orchestrator.go @@ -507,10 +507,7 @@ func (o *Orchestrator) computeDesiredWatches() map[string]bool { // Input file directories not already covered for _, fileName := range task.resolved.FileNames() { absPath := tspath.GetNormalizedAbsolutePath(fileName, o.opts.Sys.GetCurrentDirectory()) - dir := tspath.GetDirectoryPath(absPath) - if !desiredDirs.Covered(dir) && watchmanager.CanWatchDirectory(dir) { - desiredDirs.Set(dir, false) - } + o.addProgramFileWatchDir(desiredDirs, tspath.GetDirectoryPath(absPath)) for _, mapper := range task.resolved.ContentMappers() { if mapper.PackageDirectory == "" || mapper.ContributionID != "" { continue @@ -549,10 +546,7 @@ func (o *Orchestrator) computeDesiredWatches() map[string]bool { if roots.Has(fp) { continue } - dir := tspath.GetDirectoryPath(absPath) - if !desiredDirs.Covered(dir) && watchmanager.CanWatchDirectory(dir) { - desiredDirs.Set(dir, false) - } + o.addProgramFileWatchDir(desiredDirs, tspath.GetDirectoryPath(absPath)) } for packageJson := range bi.buildInfo.GetPackageJsons(buildInfoDir) { o.addPackageJsonWatchDirs(desiredDirs, packageJson) @@ -575,6 +569,14 @@ func (o *Orchestrator) addWatchDir(desiredDirs *watchmanager.DirWatchSet, dir st } } +// addProgramFileWatchDir watches the directory of a program file at any depth, unlike addWatchDir, which guards lookup +// locations against watching something as generic as / or /home. +func (o *Orchestrator) addProgramFileWatchDir(desiredDirs *watchmanager.DirWatchSet, dir string) { + if !desiredDirs.Covered(dir) { + desiredDirs.Set(dir, false) + } +} + func (o *Orchestrator) addPackageJsonWatchDirs(desiredDirs *watchmanager.DirWatchSet, packageJson string) { dir := tspath.GetDirectoryPath(packageJson) dirs := []string{dir} diff --git a/tsc/internal/execute/tsctests/watch_shallow_test.go b/tsc/internal/execute/tsctests/watch_shallow_test.go new file mode 100644 index 0000000000000..79dc968eeb2cb --- /dev/null +++ b/tsc/internal/execute/tsctests/watch_shallow_test.go @@ -0,0 +1,134 @@ +package tsctests + +import ( + "context" + "strings" + "testing" + + "github.com/microsoft/TypeScript/tsc/internal/execute" + "github.com/microsoft/TypeScript/tsc/internal/fswatch" + "gotest.tools/v3/assert" +) + +// shallowProjectFiles is a project close to the filesystem root (a common Docker WORKDIR) that imports a file from a +// sibling directory which is not part of "include", plus a bare import whose failed lookups walk up to /node_modules. +func shallowProjectFiles(compilerOptions string) FileMap { + return FileMap{ + "/app/tsconfig.json": `{"compilerOptions":{` + compilerOptions + `"rootDir":"..","outDir":"out","noLib":true},"include":["*.ts"]}`, + "/app/index.ts": `import { s } from "../shared/s"; +// @ts-ignore +import "missing-package"; +export const x = s;`, + "/shared/s.ts": `export const s = 1;`, + } +} + +// isWatched reports whether dir has an open watch. The mock keeps closed watches in Dirs, so a nil check is not enough. +func isWatched(sys *TestSys, dir string) bool { + w := sys.mockWatchBackend.Dirs[dir] + return w != nil && !w.Closed +} + +func assertShallowProjectWatches(t *testing.T, sys *TestSys) { + t.Helper() + assert.Assert(t, isWatched(sys, "/app"), "the tsconfig directory /app must be watched") + assert.Assert(t, isWatched(sys, "/shared"), "the directory of the imported program file /shared/s.ts must be watched") + // Failed lookups of "missing-package" reach /node_modules. They must not turn into a watch on /. + assert.Assert(t, !isWatched(sys, "/"), "/ must never be watched") +} + +func editShallowProjectFiles(t *testing.T, sys *TestSys, w interface{ DoCycle() }) { + t.Helper() + fs := sys.fsFromFileMap() + + sys.writeFileNoError("/shared/s.ts", `export const s = 2;`) + sys.mockWatchBackend.SendEvents([]fswatch.Event{{Kind: fswatch.EventUpdate, Path: "/shared/s.ts"}}) + w.DoCycle() + out, _ := fs.ReadFile("/app/out/shared/s.js") + assert.Assert(t, strings.Contains(out, "s = 2"), "editing /shared/s.ts must rebuild, got:\n%s", out) + + sys.writeFileNoError("/app/index.ts", `import { s } from "../shared/s"; export const y = s;`) + sys.mockWatchBackend.SendEvents([]fswatch.Event{{Kind: fswatch.EventUpdate, Path: "/app/index.ts"}}) + w.DoCycle() + out, _ = fs.ReadFile("/app/out/app/index.js") + assert.Assert(t, strings.Contains(out, "y = "), "editing /app/index.ts must rebuild, got:\n%s", out) +} + +// TestWatchShallowProjectWithImportedFile verifies that tsc --watch rebuilds a project that lives close to the +// filesystem root, both for its own files and for a file it imports from outside "include". +func TestWatchShallowProjectWithImportedFile(t *testing.T) { + t.Parallel() + sys := newTestSys(&tscInput{files: shallowProjectFiles(""), cwd: "/app"}, false) + result := execute.CommandLine(context.Background(), sys, []string{"--watch"}, sys) + assert.Assert(t, result.Watcher != nil) + + assertShallowProjectWatches(t, sys) + editShallowProjectFiles(t, sys, result.Watcher) +} + +// TestBuildWatchShallowProjectWithImportedFile is the tsc -b --watch variant of +// TestWatchShallowProjectWithImportedFile. +func TestBuildWatchShallowProjectWithImportedFile(t *testing.T) { + t.Parallel() + sys := newTestSys(&tscInput{files: shallowProjectFiles(`"composite":true,`), cwd: "/app"}, false) + ctx, cancel := context.WithCancel(t.Context()) + defer cancel() + result := execute.CommandLine(ctx, sys, []string{"--build", "--watch"}, sys) + assert.Assert(t, result.Watcher != nil) + + assertShallowProjectWatches(t, sys) + editShallowProjectFiles(t, sys, result.Watcher) +} + +// shallowRootFileProjectFiles is a project in /app whose "files" list a root file in the sibling directory /shared. +func shallowRootFileProjectFiles(compilerOptions string) FileMap { + return FileMap{ + "/app/tsconfig.json": `{"compilerOptions":{` + compilerOptions + `"rootDir":"..","outDir":"out","noLib":true},"files":["index.ts","../shared/root.ts"]}`, + "/app/index.ts": `export const x = 1;`, + "/shared/root.ts": `export const r = 1;`, + } +} + +// deleteAndRecreateShallowRootFile deletes /shared/root.ts and writes it back. While the file is missing it is not +// part of the program, but it is still a root file, so /shared must stay watched for the rebuild on recreation. +func deleteAndRecreateShallowRootFile(t *testing.T, sys *TestSys, w interface{ DoCycle() }) { + t.Helper() + fs := sys.fsFromFileMap() + assert.Assert(t, isWatched(sys, "/shared"), "the directory of the root file /shared/root.ts must be watched") + + sys.removeNoError("/shared/root.ts") + sys.mockWatchBackend.SendEvents([]fswatch.Event{{Kind: fswatch.EventDelete, Path: "/shared/root.ts"}}) + w.DoCycle() + assert.Assert(t, isWatched(sys, "/shared"), "/shared must stay watched while the root file /shared/root.ts is missing") + assert.Assert(t, !isWatched(sys, "/"), "/ must never be watched") + + sys.writeFileNoError("/shared/root.ts", `export const r = 2;`) + sys.mockWatchBackend.SendEvents([]fswatch.Event{{Kind: fswatch.EventUpdate, Path: "/shared/root.ts"}}) + w.DoCycle() + out, _ := fs.ReadFile("/app/out/shared/root.js") + assert.Assert(t, strings.Contains(out, "r = 2"), "recreating /shared/root.ts must rebuild, got:\n%s", out) +} + +// TestWatchShallowProjectRecreatedRootFile verifies that tsc --watch rebuilds when a root file near the filesystem +// root is deleted and created again. +func TestWatchShallowProjectRecreatedRootFile(t *testing.T) { + t.Parallel() + sys := newTestSys(&tscInput{files: shallowRootFileProjectFiles(""), cwd: "/app"}, false) + result := execute.CommandLine(context.Background(), sys, []string{"--watch"}, sys) + assert.Assert(t, result.Watcher != nil) + + deleteAndRecreateShallowRootFile(t, sys, result.Watcher) +} + +// TestBuildWatchShallowProjectRecreatedRootFile is the tsc -b --watch variant of +// TestWatchShallowProjectRecreatedRootFile. +func TestBuildWatchShallowProjectRecreatedRootFile(t *testing.T) { + t.Parallel() + sys := newTestSys(&tscInput{files: shallowRootFileProjectFiles(`"composite":true,`), cwd: "/app"}, false) + ctx, cancel := context.WithCancel(t.Context()) + defer cancel() + result := execute.CommandLine(ctx, sys, []string{"--build", "--watch"}, sys) + assert.Assert(t, result.Watcher != nil) + + deleteAndRecreateShallowRootFile(t, sys, result.Watcher) +} diff --git a/tsc/internal/execute/watcher.go b/tsc/internal/execute/watcher.go index 3a3b9405340c9..ca5d5f8b54795 100644 --- a/tsc/internal/execute/watcher.go +++ b/tsc/internal/execute/watcher.go @@ -252,9 +252,22 @@ func (w *Watcher) computeDesiredWatches(seenFilePaths []string) map[string]bool for dir, recursive := range resolvedDirs { coverage.Set(dir, recursive) } + programFiles := w.program.GetProgram().FilesByPath() + caseSensitive := w.sys.FS().UseCaseSensitiveFileNames() + rootFiles := collections.NewSetFromItems(core.Map(w.config.FileNames(), func(fileName string) tspath.Path { + return tspath.ToPath(fileName, cwd, caseSensitive) + })...) for _, filePath := range seenFilePaths { dir := tspath.GetDirectoryPath(filePath) - if !coverage.Covered(dir) && watchmanager.CanWatchDirectory(dir) { + if coverage.Covered(dir) { + continue + } + // Seen files mix program files with lookup locations. Only lookups keep the depth check, so an imported + // file outside the tsconfig directory (say /shared next to /app) is still watched. A root file is not in + // the program while it is missing, but its directory stays watched so that recreating it rebuilds. + p := tspath.ToPath(filePath, cwd, caseSensitive) + _, isProgramFile := programFiles[p] + if isProgramFile || rootFiles.Has(p) || watchmanager.CanWatchDirectory(dir) { coverage.Set(dir, false) } } diff --git a/tsc/internal/execute/watchmanager/watchmanager.go b/tsc/internal/execute/watchmanager/watchmanager.go index f037d93e2f863..dd6490e9d6a68 100644 --- a/tsc/internal/execute/watchmanager/watchmanager.go +++ b/tsc/internal/execute/watchmanager/watchmanager.go @@ -195,6 +195,13 @@ func (wm *WatchManager) createDirWatchRequest(dir string, entry *watchedDir) Wat func (wm *WatchManager) ResolveDesiredDirs(desiredDirs map[string]bool) map[string]bool { resolved := make(map[string]bool, len(desiredDirs)) for dir, recursive := range desiredDirs { + // Only directories on disk can be watched. The embedded libs (bundled:///libs) exist in the FS but not on disk. + if !tspath.IsRootedDiskPath(dir) { + if wm.DebugLog != nil { + fmt.Fprintf(wm.DebugLog, "[watch] not a disk path: %s\n", dir) + } + continue + } watchDir := dir watchRecursive := recursive for !wm.dirExists(watchDir) { @@ -205,7 +212,10 @@ func (wm *WatchManager) ResolveDesiredDirs(desiredDirs map[string]bool) map[stri watchDir = parent watchRecursive = false // ancestor fallbacks are always non-recursive } - if !wm.dirExists(watchDir) || !CanWatchDirectory(watchDir) { + // CanWatchDirectory only guards against falling back to an ancestor that is too generic to watch + // (/, /home, ...). A directory that exists and was asked for is watched at any depth, otherwise a + // project that lives near the filesystem root (say /app or /srv/app) would never be watched. + if !wm.dirExists(watchDir) || (watchDir != dir && !CanWatchDirectory(watchDir)) { if wm.DebugLog != nil { fmt.Fprintf(wm.DebugLog, "[watch] no watchable ancestor for %s\n", dir) } diff --git a/tsc/internal/execute/watchmanager/watchmanager_test.go b/tsc/internal/execute/watchmanager/watchmanager_test.go index 633afde563c34..bbc17792290e6 100644 --- a/tsc/internal/execute/watchmanager/watchmanager_test.go +++ b/tsc/internal/execute/watchmanager/watchmanager_test.go @@ -1,6 +1,7 @@ package watchmanager import ( + "io" "testing" "github.com/microsoft/TypeScript/tsc/internal/tspath" @@ -135,3 +136,65 @@ func TestDirWatchSetDirs(t *testing.T) { assert.Equal(t, dirs["/repo/a"], false) assert.Equal(t, dirs["/repo/b"], true) } + +// TestResolveDesiredDirsShallowProject verifies that a directory that exists and was asked for is watched at any +// depth. A project close to the filesystem root (/app, /srv/app, a Docker WORKDIR) must not be silently ignored. +func TestResolveDesiredDirsShallowProject(t *testing.T) { + t.Parallel() + + existing := map[string]bool{ + "/": true, "/app": true, "/app/src": true, "/srv": true, "/srv/app": true, + "/home": true, "/home/user": true, "/home/user/project": true, + } + wm := NewWatchManager(io.Discard, func(dir string) bool { return existing[dir] }) + + resolved := wm.ResolveDesiredDirs(map[string]bool{ + "/app": true, + "/app/src": false, + "/srv/app": true, + "/home/user/project": true, + }) + + assert.DeepEqual(t, resolved, map[string]bool{ + "/app": true, + "/app/src": false, + "/srv/app": true, + "/home/user/project": true, + }) +} + +// TestResolveDesiredDirsAncestorFallback verifies that the depth check still guards the fallback to an ancestor, +// so a missing directory never turns into a watch on something too generic like /, /home or /home/user. +func TestResolveDesiredDirsAncestorFallback(t *testing.T) { + t.Parallel() + + existing := map[string]bool{ + "/": true, "/app": true, "/home": true, "/home/user": true, + "/repo": true, "/repo/a": true, "/repo/a/b": true, "/repo/a/b/c": true, + } + wm := NewWatchManager(io.Discard, func(dir string) bool { return existing[dir] }) + + resolved := wm.ResolveDesiredDirs(map[string]bool{ + "/app/missing": true, // ancestor /app is too shallow + "/home/user/missing": true, // ancestor /home/user is too shallow + "/repo/a/b/c/missing/deep": true, // ancestor /repo/a/b/c is deep enough, and is never recursive + "/nothing/exists/anywhere/": true, // no existing ancestor except / + }) + + assert.DeepEqual(t, resolved, map[string]bool{"/repo/a/b/c": false}) +} + +// TestResolveDesiredDirsSkipsNonDiskPaths verifies that a directory that is not on disk, such as the embedded libs +// (bundled:///libs), is never watched, even though the wrapped FS reports that it exists. +func TestResolveDesiredDirsSkipsNonDiskPaths(t *testing.T) { + t.Parallel() + + wm := NewWatchManager(io.Discard, func(dir string) bool { return true }) + + resolved := wm.ResolveDesiredDirs(map[string]bool{ + "bundled:///libs": false, + "/app": true, + }) + + assert.DeepEqual(t, resolved, map[string]bool{"/app": true}) +}