Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves the tsgo --watch implementation to avoid rebuilding on a fixed interval when inputs haven’t changed, and adds coverage for a broad set of watch-trigger scenarios (file edits, config changes, module resolution, and filesystem operations).
Changes:
- Track compilation file-system dependencies and skip rebuilds when no watched inputs changed.
- Add debouncing logic for non-test watch cycles.
- Expand watch baseline coverage with new command-line watch scenarios.
Reviewed changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/execute/watcher.go | Reworks watch to track accessed FS paths, detect changes via polling, debounce rebuilds, and reparse tsconfig for semantic change detection. |
| internal/execute/tsctests/tscwatch_test.go | Adds extensive new watch scenarios (file lifecycle, tsconfig changes, module resolution, symlinks). |
| testdata/baselines/reference/tscWatch/commandLineWatch/watch-skips-build-when-no-files-change.js | New baseline asserting no rebuild when no inputs change. |
| testdata/baselines/reference/tscWatch/commandLineWatch/watch-rebuilds-when-file-is-modified.js | New baseline asserting rebuild on source edit. |
| testdata/baselines/reference/tscWatch/commandLineWatch/watch-rebuilds-when-source-file-is-deleted.js | New baseline asserting rebuild/diagnostics when an imported source is deleted. |
| testdata/baselines/reference/tscWatch/commandLineWatch/watch-detects-new-file-resolving-failed-import.js | New baseline asserting rebuild when a missing import target is created. |
| testdata/baselines/reference/tscWatch/commandLineWatch/watch-detects-imported-file-added-in-new-directory.js | New baseline asserting rebuild when a new directory+file resolves an import. |
| testdata/baselines/reference/tscWatch/commandLineWatch/watch-detects-imported-directory-removed.js | New baseline asserting rebuild when an imported directory/file is removed. |
| testdata/baselines/reference/tscWatch/commandLineWatch/watch-detects-import-path-restructured.js | New baseline asserting rebuild when files move and import paths change. |
| testdata/baselines/reference/tscWatch/commandLineWatch/watch-rebuilds-when-tsconfig-include-pattern-adds-file.js | New baseline asserting rebuild when include is widened and adds a file. |
| testdata/baselines/reference/tscWatch/commandLineWatch/watch-rebuilds-when-tsconfig-is-modified-to-change-strict.js | New baseline asserting rebuild when compiler options change (strict). |
| testdata/baselines/reference/tscWatch/commandLineWatch/watch-detects-file-added-to-previously-non-existent-include-path.js | New baseline asserting rebuild when a file appears under an include glob directory. |
| testdata/baselines/reference/tscWatch/commandLineWatch/watch-detects-node-modules-package-added.js | New baseline asserting rebuild when a missing node_modules package is installed. |
| testdata/baselines/reference/tscWatch/commandLineWatch/watch-detects-node-modules-package-removed.js | New baseline asserting rebuild when a previously resolved package is removed. |
| testdata/baselines/reference/tscWatch/commandLineWatch/watch-handles-tsconfig-deleted.js | New baseline asserting behavior when tsconfig.json is deleted. |
| testdata/baselines/reference/tscWatch/commandLineWatch/watch-handles-tsconfig-with-extends-base-modified.js | New baseline asserting rebuild when an extended base config changes. |
| testdata/baselines/reference/tscWatch/commandLineWatch/watch-skips-rebuild-when-tsconfig-is-touched-but-content-unchanged.js | New baseline asserting no rebuild when tsconfig mtime changes but semantics don’t. |
| testdata/baselines/reference/tscWatch/commandLineWatch/watch-with-tsconfig-files-list-entry-deleted.js | New baseline asserting rebuild when a files entry is removed via deletion. |
| testdata/baselines/reference/tscWatch/commandLineWatch/watch-detects-module-going-missing-then-coming-back.js | New baseline asserting rebuild when an imported module disappears and reappears. |
| testdata/baselines/reference/tscWatch/commandLineWatch/watch-detects-scoped-package-installed.js | New baseline asserting rebuild when a scoped node_modules package appears. |
| testdata/baselines/reference/tscWatch/commandLineWatch/watch-detects-package-json-main-field-edited.js | New baseline asserting rebuild when a package’s package.json changes (currently tests types field). |
| testdata/baselines/reference/tscWatch/commandLineWatch/watch-detects-at-types-package-installed-later.js | New baseline asserting rebuild when @types/* is installed after an initial missing-types error. |
| testdata/baselines/reference/tscWatch/commandLineWatch/watch-detects-file-renamed-and-renamed-back.js | New baseline asserting rebuild when a file is renamed and renamed back. |
| testdata/baselines/reference/tscWatch/commandLineWatch/watch-detects-file-deleted-and-new-file-added-simultaneously.js | New baseline asserting rebuild when delete+create happen together with import update. |
| testdata/baselines/reference/tscWatch/commandLineWatch/watch-handles-file-rapidly-recreated.js | New baseline asserting rebuild for rapid delete/recreate with changed content. |
| testdata/baselines/reference/tscWatch/commandLineWatch/watch-detects-change-in-symlinked-file.js | New baseline asserting rebuild when a symlink target changes. |
|
The two things I see off the bat that are missing that we discussed while whiteboarding:
|
I just implemented this change by seperating the file tracking into a |
|
|
||
| type watchCompilerHost struct { | ||
| inner compiler.CompilerHost | ||
| cache *collections.SyncMap[tspath.Path, *cachedSourceFile] |
There was a problem hiding this comment.
I'm not sure about this method; this will grow indefinitely, no? There's a delete below, but that's only going to happen if a second run happens to call "get" on the source file and it doesn't exist, but if a user removes an import, thus leaving files in say node_modules unreferenced, those will stick around forever, right?
There was a problem hiding this comment.
Good catch - I'll add logic to evict any unused files from the cache after the build completes
|
I don't think 5ef0db6 is sufficient for wildcard watching—I can't quite figure out what the baseline tests are saying there (what does “incremental skips emit for new unreferenced file” mean?) but I think you’re going to have to read the directory (recursively if the pattern is recursive) to see if there are any new files. |
The watch CLI option previously recompiled the program on a set interval even when files didn't change; this PR implements several efficiency fixes for this feature including debouncing, caching config, and parsing dependencies to recompile only when files have been changed.