Repository navigation
Fix non-root dependency tracking in build watch mode - #64973
Draft
John Favret (johnfav03) wants to merge 3 commits into
Draft
John Favret (johnfav03) wants to merge 3 commits into
John Favret (johnfav03) wants to merge 3 commits into
Conversation
typescript-automation
Bot
added
Author: Team
For Uncommitted Bug
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
The changes require final human review because they are too complex or risky for automated approval.
No open issues 🎉
What changed in this PR
Fixes #64424 by tracking non-root dependencies independently of incremental file tables, so build watch mode can rebuild non-incremental projects when imported files change.
Changes:
- Capture dependencies, resolution lookups, and symlink identities; persist dependency inventories in build info.
- Use those inventories for event classification, watch registration, and freshness checks, with shared source-cache invalidation.
- Add regression tests and update affected build-info baselines.
Validation commands were not run in this review environment.
| File | Description |
|---|---|
| tsc/testdata/baselines/reference/tsc/declarationEmit/reports-dts-generation-errors.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuildWatch/programUpdates/works-when-noUnusedParameters-changes-to-false.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuildWatch/programUpdates/works-correctly-when-project-with-extended-config-is-removed.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuildWatch/programUpdates/tsbuildinfo-has-error.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuildWatch/noEmitOnError/noEmitOnError.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuildWatch/noEmitOnError/noEmitOnError-with-declaration.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuildWatch/noEmit/syntax-errors.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuildWatch/noEmit/semantic-errors.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuildWatch/noEmit/dts-errors.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuildWatch/noEmit/dts-errors-without-dts-enabled.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuildWatch/noEmit/does-not-go-in-loop-when-watching-when-no-files-are-emitted.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuildWatch/noEmit/does-not-go-in-loop-when-watching-when-no-files-are-emitted-with-incremental.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuildWatch/moduleResolution/build-mode-watches-package-json-lookups-from-existing-buildinfo.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuildWatch/moduleResolution/build-mode-watches-missing-package-json-lookups.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuildWatch/extends/configDir-template.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/transitiveReferences/builds-correctly.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/solution/does-not-have-empty-files-diagnostic-when-files-is-empty-and-references-are-provided.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/sample/tsbuildinfo-has-error.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/sample/builds-correctly-when-project-is-not-composite-or-doesnt-have-any-references.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/roots/when-two-root-files-are-consecutive.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/resolveJsonModule/without-outDir.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/resolveJsonModule/without-outDir-non-composite.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/resolveJsonModule/sourcemap.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/resolveJsonModule/sourcemap-non-composite.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/resolveJsonModule/include-only.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/resolveJsonModule/include-only-without-outDir.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/resolveJsonModule/include-only-without-outDir-non-composite.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/resolveJsonModule/include-only-with-json-without-rootDir-but-outside-configDirectory.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/resolveJsonModule/include-only-with-json-without-rootDir-but-outside-configDirectory-non-composite.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/resolveJsonModule/include-only-with-json-not-in-rootDir.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/resolveJsonModule/include-only-with-json-not-in-rootDir-non-composite.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/resolveJsonModule/include-only-non-composite.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/resolveJsonModule/include-of-json-along-with-other-include.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/resolveJsonModule/include-of-json-along-with-other-include-non-composite.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/resolveJsonModule/include-of-json-along-with-other-include-and-file-name-matches-ts-file.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/resolveJsonModule/include-of-json-along-with-other-include-and-file-name-matches-ts-file-non-composite.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/resolveJsonModule/include-and-files.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/resolveJsonModule/include-and-files-non-composite.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/resolveJsonModule/importing-json-module-from-project-reference.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/resolveJsonModule/files-containing-json-file.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/resolveJsonModule/files-containing-json-file-non-composite.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/projectReferenceWithRootDirInParent/reports-no-error-when-tsbuildinfo-differ.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/projectReferenceWithRootDirInParent/reports-error-for-same-tsbuildinfo-file.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/projectReferenceWithRootDirInParent/reports-error-for-same-tsbuildinfo-file-without-incremental.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/projectReferenceWithRootDirInParent/reports-error-for-same-tsbuildinfo-file-without-incremental-with-tsc.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/projectReferenceWithRootDirInParent/reports-error-for-same-tsbuildinfo-file-because-no-rootDir-in-the-base.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/programUpdates/tsbuildinfo-has-fewer-fileInfos-than-fileNames.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/outputPaths/when-rootDir-is-specified.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/outputPaths/when-rootDir-is-specified-but-not-all-files-belong-to-rootDir.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/outputPaths/when-rootDir-is-specified-but-not-all-files-belong-to-rootDir-and-is-composite.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/outputPaths/when-rootDir-is-not-specified.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/outputPaths/when-rootDir-is-not-specified-and-is-composite.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/noEmitOnError/syntax-errors.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/noEmitOnError/syntax-errors-with-declaration.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/noEmitOnError/semantic-errors.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/noEmitOnError/semantic-errors-with-declaration.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/noEmitOnError/dts-errors.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/noEmitOnError/dts-errors-with-declaration.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/noEmit/syntax-errors.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/noEmit/semantic-errors.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/noEmit/dts-errors.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/noEmit/dts-errors-without-dts-enabled.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/noEmit/dts-errors-with-declaration-enable-changes.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/moduleResolution/when-resolution-is-not-shared.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/moduleResolution/shared-resolution-should-not-report-error.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/moduleResolution/resolves-specifier-in-output-declaration-file-from-referenced-project-correctly.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/moduleResolution/resolves-specifier-in-output-declaration-file-from-referenced-project-correctly-with-preserveSymlinks.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/moduleResolution/resolution-from-d.ts-of-referenced-project.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/moduleResolution/impliedNodeFormat-differs-between-projects-for-shared-file.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/fileDelete/deleted-file-without-composite.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/extends/resolves-the-symlink-path.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/extends/configDir-template.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/declarationEmit/when-ts-file-is-referenced-through-triple-slash-from-another-project.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/declarationEmit/when-declaration-file-is-referenced-through-triple-slash-but-uses-no-references.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/declarationEmit/reports-dts-generation-errors.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/declarationEmit/reports-dts-generation-errors-with-incremental.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/contentMapperOptionDiagnostics/rebuild-to-report-mapper-option-diagnostics.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/contentMapperIdentity/content-mapper-identity-change-forces-rebuild.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/configFileErrors/when-tsconfig-extends-the-missing-file.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/commandLine/included-tsconfig-json-can-be-imported-as-json-input.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/commandLine/emitDeclarationOnly-on-commandline-with-declaration.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/commandLine/emitDeclarationOnly-false-on-commandline-with-declaration.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsbuild/clean/tsx-with-dts-emit.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/fourslash/state/findAllRefsSolutionWithDisableReferencedProjectLoadReferencingDefaultProjectDirectly.baseline | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/fourslash/state/findAllRefsSolutionReferencingDefaultProjectDirectly.baseline | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/fourslash/state/findAllRefsProjectWithOwnFilesReferencingFileFromReferencedProject.baseline | Updated as part of this pull request. |
| tsc/internal/execute/tsctests/readablebuildinfo.go | Updated as part of this pull request. |
| tsc/internal/execute/incremental/snapshottobuildinfo.go | Updated as part of this pull request. |
| tsc/internal/execute/incremental/buildInfo.go | Updated as part of this pull request. |
| tsc/internal/execute/incremental/buildinfo_path_test.go | Updated as part of this pull request. |
| tsc/internal/execute/build/watchDependencies_test.go | Updated as part of this pull request. |
| tsc/internal/execute/build/host.go | Updated as part of this pull request. |
| tsc/internal/execute/build/compilerHost.go | Updated as part of this pull request. |
Review effort: Balanced
Member
|
I had copilot review this and it found this test case with bad behavior: func TestBuildWatchDependencyDirectoryReplacement(t *testing.T) {
t.Parallel()
for _, incremental := range []bool{false, true} {
t.Run(fmt.Sprintf("incremental=%v", incremental), func(t *testing.T) {
t.Parallel()
const pkg = dependencyProject + "node_modules/widget"
const manifest = `{"name":"widget","types":"index.d.ts"}`
const output = dependencyProject + "out/index.d.ts"
files := FileMap{
dependencyProject + "tsconfig.json": fmt.Sprintf(`{
"compilerOptions": {
"outDir": "out",
"declaration": true,
"incremental": %v,
"module": "nodenext",
"target": "es2022",
"types": [],
"libReplacement": false
},
"include": ["**/*.ts"]
}`, incremental),
dependencyProject + "index.ts": `import { value } from "widget"; export const result = value;`,
pkg + "/package.json": manifest,
pkg + "/index.d.ts": `export const value: 1;`,
}
sys, watcher := startDependencyWatch(
t, files,
[]string{"--build", "--watch", "--verbose", "--pretty", "false"},
false, "",
)
checkDependencyOutput(t, sys, output, "result: 1;")
sys.removeNoError(pkg + "/index.d.ts")
sys.removeNoError(pkg + "/package.json")
sys.removeNoError(pkg)
sys.writeFileNoError(pkg+"/package.json", manifest)
sys.writeFileNoError(pkg+"/index.d.ts", `export const value: 2;`)
sys.mockWatchBackend.SendEvents([]fswatch.Event{
{Kind: fswatch.EventUpdate, Path: pkg},
})
watcher.DoCycle()
checkDependencyOutput(t, sys, output, "result: 2;")
checkDependencySuccess(t, sys)
})
}
}Both incremental variants leave the output at |
Member
|
I'm sort of having a hard time trying to review this; is this potentially possible to rebase into a few commits? Or is it definitely that all is required and one single fix? |
Member
|
Maybe Andrew Branch (@andrewbranch) can internalize this better than i can though |
John Favret (johnfav03)
force-pushed
the
fix/64424
branch
from
October 29, 2026 18:50
5f6784e to
b121a25
Compare
Contributor
Author
|
Jake Bailey (@jakebailey) I split it up into 3 commits, let me know if this is easier to review or if there's something else I can do |
Jake Bailey (jakebailey)
left a comment
Jake Bailey (jakebailey)
left a comment
Member
There was a problem hiding this comment.
#64424 (comment) though maybe this should be drafted?
|
|
||
| // NonIncrementalProgram info | ||
| SemanticErrors bool `json:"semanticErrors,omitzero"` | ||
| // A nil inventory predates dependency tracking; an empty inventory is complete. |
There was a problem hiding this comment.
This looks like some agent text which encodes a WIP discovery; i'd redo or remove this
| continue | ||
| } | ||
| for eventPath := range normalizedPaths { | ||
| if eventPath != inputPath && eventPath.ContainsPath(inputPath) { |
There was a problem hiding this comment.
This came from a local Copilot review: I think this misses directory-only events for lookup dependencies such as package.json. A manifest can redirect resolution to a different declaration file without changing either a consumed input or an alias. In that case, an update event for the package directory is ignored and the build output stays stale. Could directory events also invalidate when they cover a tracked lookup?
| sys, watcher := startDependencyWatch(t, files, []string{"--build", "--watch", "--verbose", "--pretty", "false"}, startup != "live", "") | ||
| checkDependencyOutput(t, sys, output, "result: 1;") | ||
| notify := func() { | ||
| sys.mockWatchBackend.SendEvents([]fswatch.Event{{Kind: fswatch.EventUpdate, Path: pkg}}) |
There was a problem hiding this comment.
This came from a local Copilot review: could we add a regression case where a package manifest redirects from one existing declaration file to another, and the watcher receives only an update for the package directory? That would cover lookup changes that don’t involve replacing a consumed input file.
Member
|
The previous behavior was intentional in Strada: #64424 (comment) I think it's worth reconsidering, but this deserves a deep dive into why the old design was the way it was, an analysis of the semantic implications of changing it, a design meeting to agree on the direction and scope of the change, a report of the performance tradeoffs... This is feature work that belongs in the beginning of a release cycle, not a bug fix we should be considering for an RC. |
John Favret (johnfav03)
marked this pull request as draft
October 29, 2026 20:24
This branch has not been deployed
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.
Fixes #64424
Build watch mode relied on configured roots and incremental build-info file tables to identify affected projects. Non-incremental builds lack that same file table, so changes to non-root imported files could be ignored even when a filesystem notification arrived.
Example: If we have
"files": ["index.ts"]andindex.tsimports./other.js, editingother.tsshould invalidate the project and cause a rebuild even whenincrementalandcompositeare disabled.