Watch project directories that are close to the filesystem root - #64366
Open
soso (Generalsimus) wants to merge 1 commit into
Open
soso (Generalsimus) wants to merge 1 commit into
soso (Generalsimus) wants to merge 1 commit into
Conversation
ResolveDesiredDirs ran CanWatchDirectory on every desired directory, including the ones the project itself declares (wildcard include directories, the tsconfig directory, the cwd). CanWatchDirectory needs at least five path components, so a project in /app, /srv/app or /home/user/project was silently dropped and `tsc --watch` never rebuilt. Only apply the check when falling back to an ancestor of a directory that does not exist, which is what it is meant to guard against (/, /home, ...). A directory that exists and was asked for is watched at any depth, like tsc 6.0 watches every program file.
|
This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise. |
Contributor
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The focused logic change matches the intended behavior and includes appropriate regression coverage.
Review effort: Balanced
Findings: None
What changed in this PR
Fixes shallow-root project directories being excluded from tsc --watch.
Changes:
- Applies directory-depth filtering only to ancestor fallbacks.
- Adds tests for shallow projects and unsafe fallback ancestors.
| File | Description |
|---|---|
tsc/internal/execute/watchmanager/watchmanager.go |
Allows explicitly requested existing directories at any depth. |
tsc/internal/execute/watchmanager/watchmanager_test.go |
Covers shallow directories and ancestor safeguards. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
Author
|
Author
|
@microsoft-github-policy-service agree |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
tsc --watchnever rebuilds when the project lives close to the filesystem root, for example/app,/srv/app,/server(a common DockerWORKDIR) or/home/user/project. Nothing is printed. TypeScript 6.0 rebuilds in the same setup, so this is a 6.0/7.0 difference.Reproduce
TS_WATCH_DEBUG=1shows why nothing happens:Cause
WatchManager.ResolveDesiredDirsrunsCanWatchDirectoryon every directory it is given, including the ones the project itself declares: the wildcard directories frominclude, the tsconfig directory and the cwd.CanWatchDirectoryrequires at least 5 path components (/a/b/c/d), so a shallower directory is dropped without any fallback or diagnostic and no source file in it is ever watched. Onlynode_moduleswas still watched, which made it look like the watcher was running.In TypeScript 6.0 the same heuristic (
canWatchDirectoryOrFile, identical numbers) is only used by the module resolution cache for failed lookup locations. Program source files are watched individually at any depth. Here it also filters the project's own directories.The rule is meant to stop the ancestor fallback (a missing directory resolving to
/,/home,/home/user) from watching something far too generic. That is the only place it should apply.Fix
In
ResolveDesiredDirs, applyCanWatchDirectoryonly when the directory had to fall back to an ancestor. A directory that exists and was asked for is watched at any depth. Directories thattscinfers on its own (failedpackage.jsonlookups,bundled:///libs, ...) are still guarded, because they are not added through this path.Verification
Same project, unfixed and fixed native
tsc, edita.tsonce after the first build:/probe(Docker overlayfs)/srv/app(Docker overlayfs)/home/<user>/proj(host, 3 dirs deep)Also checked with a real project laid out as
/serverplus/shared(rootDir: "..",include: ["**/*.ts", "../shared/**/*.ts"]): before,no watchable ancestor for /serverand/sharedand no rebuild; after, both are watched and edits rebuild. TypeScript 6.0.3 rebuilds for the same shallow path.Tests:
TestResolveDesiredDirsShallowProjectfails without the change (all four shallow directories are dropped) and passes with it.TestResolveDesiredDirsAncestorFallbackpins the part that must stay: a missing directory never falls back to/app,/home/useror similar.go test ./internal/execute/...passes.Known limitation
Directories that are only inferred from files the program reads (for example a file imported from outside
include, or afiles-only tsconfig importing from a shallow subdirectory) still go through the depth check incomputeDesiredWatchesand thetsc -borchestrator. I did not change that: the same list also holds failed lookups such as/home/<user>/package.jsonand the virtualbundled:///libs, and loosening the check there makes the watch setup fail on those. Telling real source files apart from lookups needs a separate, larger change. Happy to follow up if you want it.AI disclosure
Written with the help of an AI coding assistant, as described in CONTRIBUTING.md. I ran into this in a Docker container and will follow up on review feedback.