Watcher recursively watches irrelevant directories

[dmichon-msft]

🔎 Search Terms

watch, excess, files, directories

🕗 Version & Regression Information

• This changed between versions 5.0.4 and 5.1.3, still present in typescript@next

⏯ Playground Link

No response

💻 Code

Repro

Steps:

npm install -g pnpm
pnpm -r install
pnpm -r build
cd b/2/b-impl/b
./node_modules/.bin/tsc --watch --extendedDiagnostics

Once the build finishes, create a file tmp.txt in a/2/logs and observe that TypeScript acts on the file change.

🙁 Actual behavior

If a folder is added to TypeScript's list of "failed lookup locations", any file change at any level within said folder will trigger a rebuild, even if the change is in a nested folder that was not probed during module resolution.

🙂 Expected behavior

TypeScript only watches contents of directories that contain files that were referenced in a tsconfig or probed during module resolution.

Additional information about the issue

Environment: GitHub Codespaces
OS: #19~22.04.1-Ubuntu SMP Wed Jan 10 22:57:03 UTC 2024
node 18.20.2

[dmichon-msft]

Looking further, this seems to be triggered by TypeScript unexpectedly trying to resolve an import of ./something from an index.d.ts file to a ./something.ts file instead of going directly to ./something.d.ts.

[dmichon-msft]

Issue updated with minimal repro.

[sheetalkamat]

This is intentional as otherwise many watchers are created. for different resolutions and we want to avoid that. The watcher is smart to use lot of stuff so even if it triggers rebuild it will not do anything major.

[RyanCavanaugh]

@dmichon-msft do you have a reproduction of the infinite build loop you mentioned on Teams?

[dmichon-msft]

This is intentional as otherwise many watchers are created. for different resolutions and we want to avoid that. The watcher is smart to use lot of stuff so even if it triggers rebuild it will not do anything major.
Having a recursive watcher on an ancestor is fine, we just need TypeScript to filter the data it receives from the watcher to not act on events for which the affected file is not directly in a folder of concern. This shouldn't be terribly expensive to add.
That said, my repro is on Linux, which doesn't support native recursive file watching, so TypeScript would have to internally be spinning up a lot of irrelevant directory watchers on these child folders that don't contain relevant files.

@dmichon-msft do you have a reproduction of the infinite build loop you mentioned on Teams?

The infinite build loop is a consequence of that the orchestrator I use plugs into the CompilerHost's setTimeout event and treats any call to it as "code change detected, need to rebuild". A log file written by a downstream project (which is not anywhere in the npm dependency graph of the project with the watcher) gets detected by TypeScript's file watcher and invokes a setTimeout on the grounds that a resolution-affecting change may have occurred.

If there's a proper way for me to wire heft-typescript-plugin into TypeScript's watch compiler such that I can only intercept when it would actually trigger a rebuild, I'd be happy to adopt that instead and that would also provide a way to resolve.
The current implementation is here:
https://github.com/microsoft/rushstack/blob/f46053ca7ec8a36f81785e27713099d6c76debb5/heft-plugins/heft-typescript-plugin/src/TypeScriptBuilder.ts#L292-L319

[RyanCavanaugh]

plugs into the CompilerHost's setTimeout event and treats any call to it as "code change detected, need to rebuild".

This is definitely not safe to do (as you've found out)

If you're going to go into internals anyway... you could intercept writeFile ?

[sheetalkamat]

You will notice that we use setTimeout to postpone determination of whether program needs to be rebuilt so definitely something you shouldnt be hacking into.

You may also be able to override onWatchStatusChange to find if program was rebuilt. Again this doesnt guarantee if new files are emitted. writeFile will give you that.

[dmichon-msft]

You will notice that we use setTimeout to postpone determination of whether program needs to be rebuilt so definitely something you shouldnt be hacking into.

You may also be able to override onWatchStatusChange to find if program was rebuilt. Again this doesnt guarantee if new files are emitted. writeFile will give you that.

The reason I hook setTimeout is so that I can prevent TypeScript from doing any work whatsoever until I have been informed that all dependencies have finished rebuilding. Essentially the requested timeout callback gets postponed until other tasks have finished, and only then does it allow TypeScript to proceed.

This works cleanly in webpack, because the API contract for file watching always specifies immediate parent directories, even though the backend is implemented with one or more recursive file watchers.

I would argue that the decision to implement a watch via a recursive watcher should be an implementation detail of the WatchHost, not something that happens on the calling side.

As it stands, even if I override watchDirectory on the watch host I can't properly implement this logic, because TypeScript doesn't tell me which subdirectories of a directory are relevant (I have no issue with noise from immediate children of a watched directory, my issue is only with files in irrelevant subfolders).