Closed dmichon-msft closed 1 week ago
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
.
Issue updated with minimal repro.
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.
@dmichon-msft do you have a reproduction of the infinite build loop you mentioned on Teams?
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
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
?
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.
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).
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.
This isn't an implementation detail; instead you're describing a different level of abstraction.
At the level of abstraction that it actually handles, the implemented object can and does have a finite number of watch handles, e.g. trying to put individual file watches on several thousand folders in node_modules
will run the system out of handles and you need to instead put a recursive watch on the parent folder and filter the events.
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.
This isn't an implementation detail; instead you're describing a different level of abstraction.
At the level of abstraction that it actually handles, the implemented object can and does have a finite number of watch handles, e.g. trying to put individual file watches on several thousand folders in
node_modules
will run the system out of handles and you need to instead put a recursive watch on the parent folder and filter the events.
I don't think we are disagreeing?
For clarity, by "watch host" I mean the interface in the public TypeScript API called "WatchHost" that has methods for watchFile
and watchDirectory
, for which the default implementation on Node (provided in ts.sys
) wraps calls to the fs
calls and implements recursive watching on Linux.
Suppose we have a folder /test/foo
that contains:
/test/foo/a/b
/test/foo/a/irrelevant
/test/foo/b/c
/test/foo/b/irrelevant
/test/foo/irrelevant
TypeScript observes that it probed for non-existent files in /test/foo/a/b
and /test/foo/b/c
.
In this path, TypeScript asks the watch host to watch for children in exactly /test/foo/a/b
and /test/foo/b/c
. Possibly it may also ask to watch /test/foo/a
to see if /test/foo/a/b
gets renamed, and likewise for /test/foo/b
.
Linux has no native recursive watch support, so it has to end up watching every relevant directory directly regardless. This means it allocates the following non-recursive directory watchers:
/test/foo/a
/test/foo/a/b
/test/foo/b
/test/foo/b/c
The watch host is free to determine that allocating directory watchers for each of:
/test/foo/a
/test/foo/a/b
/test/foo/b
/test/foo/b/c
is too many handles, and consolidate on the common ancestor of /test/foo
, at which point it takes responsibility for filtering the events and invoking the watch callbacks only for immediate children of one of the four above folders.
In this path TypeScript directly asks for the watch host to watch /test/foo
recursively.
The OS doesn't know which parts of /test/foo
are relevant, so is required to allocate native system directory watchers on each of:
/test/foo/a
/test/foo/a/b
/test/foo/a/irrelevant
/test/foo/b
/test/foo/b/c
/test/foo/b/irrelevant
/test/foo/irrelevant
This results in 3 more system file watchers than in option (1), as well as notifying TypeScript of irrelevant file changes and forcing it to calculate whether or not the change affects results.
The OS allocates a recursive watcher for /test/foo
and forwards all events to TypeScript. Responsibility for identifying if a change is in a relevant folder is delegated to the compiler, rather than the watch host.
In option (1), we use fewer system resources on platforms without native recursive watch support, and if there is recursive watch support, preliminary event filtering is handled in a self-contained component, rather than being the responsibility of the TypeScript compiler.
In option (2), custom watch providers have no way of identifying which changes are actually of concern to the TypeScript compiler and can't provide any preliminary filtering.
using settimeout
is incorrect strategy and relying on how we implement details. Eg in scenario you mentioned, we don't even build project. We are just batching things to see if they are going to need project updates.
Watching just needed folder can still cause this since you could still add some irrelevant file there and we need to handle that. Creating many watchers has perf impact as well as memory and resources impact. We have tried this. Average size projects become unusable with this approach.
Watching just needed folder can still cause this since you could still add some irrelevant file there and we need to handle that. Creating many watchers has perf impact as well as memory and resources impact. We have tried this. Average size projects become unusable with this approach.
My issue is that, when running under Linux, in TypeScript >=5.1
, TypeScript is allocating too many directory watchers. To highlight this regression, I have updated the repro repository at https://github.com/dmichon-msft/typescript-watch-repo
Following the steps in the repro, when using TypeScript 5.0.6, the complete log:
@dmichon-msft β .../b/2/b-impl/b (main) $ pnpm run repro-5.0
> b@ repro-5.0 /workspaces/typescript-watch-repo/b/2/b-impl/b
> node ./node_modules/ts-5.0/lib/tsc.js --watch --extendedDiagnostics
[6:33:33 PM] Starting compilation in watch mode...
Current directory: /workspaces/typescript-watch-repo/b/2/b-impl/b CaseSensitiveFileNames: true
FileWatcher:: Added:: WatchInfo: /workspaces/typescript-watch-repo/b/2/b-impl/b/tsconfig.json 2000 undefined Config file
sysLog:: /workspaces/typescript-watch-repo/b/2/b-impl/b/tsconfig.json:: Changing to watchFile
Synchronizing program
CreatingProgramWith::
roots: ["/workspaces/typescript-watch-repo/b/2/b-impl/b/src/index.ts"]
options: {"moduleResolution":2,"module":99,"declaration":true,"declarationMap":true,"outDir":"/workspaces/typescript-watch-repo/b/2/b-impl/b/lib","watch":true,"extendedDiagnostics":true,"configFilePath":"/workspaces/typescript-watch-repo/b/2/b-impl/b/tsconfig.json"}
FileWatcher:: Added:: WatchInfo: /workspaces/typescript-watch-repo/b/2/b-impl/b/src/index.ts 250 undefined Source file
sysLog:: /workspaces/typescript-watch-repo/b/2/b-impl/b/src/index.ts:: Defaulting to watchFile
FileWatcher:: Added:: WatchInfo: /workspaces/typescript-watch-repo/a/1/a-impl/a/lib/index.d.ts 250 undefined Source file
sysLog:: /workspaces/typescript-watch-repo/a/1/a-impl/a/lib/index.d.ts:: Defaulting to watchFile
FileWatcher:: Added:: WatchInfo: /workspaces/typescript-watch-repo/a/1/a-impl/a/lib/a.d.ts 250 undefined Source file
sysLog:: /workspaces/typescript-watch-repo/a/1/a-impl/a/lib/a.d.ts:: Defaulting to watchFile
FileWatcher:: Added:: WatchInfo: /workspaces/typescript-watch-repo/node_modules/.pnpm/typescript@5.0.4/node_modules/typescript/lib/lib.d.ts 250 undefined Source file
sysLog:: /workspaces/typescript-watch-repo/node_modules/.pnpm/typescript@5.0.4/node_modules/typescript/lib/lib.d.ts:: Defaulting to watchFile
FileWatcher:: Added:: WatchInfo: /workspaces/typescript-watch-repo/node_modules/.pnpm/typescript@5.0.4/node_modules/typescript/lib/lib.es5.d.ts 250 undefined Source file
sysLog:: /workspaces/typescript-watch-repo/node_modules/.pnpm/typescript@5.0.4/node_modules/typescript/lib/lib.es5.d.ts:: Defaulting to watchFile
FileWatcher:: Added:: WatchInfo: /workspaces/typescript-watch-repo/node_modules/.pnpm/typescript@5.0.4/node_modules/typescript/lib/lib.decorators.d.ts 250 undefined Source file
sysLog:: /workspaces/typescript-watch-repo/node_modules/.pnpm/typescript@5.0.4/node_modules/typescript/lib/lib.decorators.d.ts:: Defaulting to watchFile
FileWatcher:: Added:: WatchInfo: /workspaces/typescript-watch-repo/node_modules/.pnpm/typescript@5.0.4/node_modules/typescript/lib/lib.decorators.legacy.d.ts 250 undefined Source file
sysLog:: /workspaces/typescript-watch-repo/node_modules/.pnpm/typescript@5.0.4/node_modules/typescript/lib/lib.decorators.legacy.d.ts:: Defaulting to watchFile
FileWatcher:: Added:: WatchInfo: /workspaces/typescript-watch-repo/node_modules/.pnpm/typescript@5.0.4/node_modules/typescript/lib/lib.dom.d.ts 250 undefined Source file
sysLog:: /workspaces/typescript-watch-repo/node_modules/.pnpm/typescript@5.0.4/node_modules/typescript/lib/lib.dom.d.ts:: Defaulting to watchFile
FileWatcher:: Added:: WatchInfo: /workspaces/typescript-watch-repo/node_modules/.pnpm/typescript@5.0.4/node_modules/typescript/lib/lib.webworker.importscripts.d.ts 250 undefined Source file
sysLog:: /workspaces/typescript-watch-repo/node_modules/.pnpm/typescript@5.0.4/node_modules/typescript/lib/lib.webworker.importscripts.d.ts:: Defaulting to watchFile
FileWatcher:: Added:: WatchInfo: /workspaces/typescript-watch-repo/node_modules/.pnpm/typescript@5.0.4/node_modules/typescript/lib/lib.scripthost.d.ts 250 undefined Source file
sysLog:: /workspaces/typescript-watch-repo/node_modules/.pnpm/typescript@5.0.4/node_modules/typescript/lib/lib.scripthost.d.ts:: Defaulting to watchFile
DirectoryWatcher:: Added:: WatchInfo: /workspaces/typescript-watch-repo/b/2/b-impl/b/src 1 undefined Failed Lookup Locations
sysLog:: /workspaces/typescript-watch-repo/b/2/b-impl/b/src:: Defaulting to watchFile
Elapsed:: 0.9419100000000071ms DirectoryWatcher:: Added:: WatchInfo: /workspaces/typescript-watch-repo/b/2/b-impl/b/src 1 undefined Failed Lookup Locations
DirectoryWatcher:: Added:: WatchInfo: /workspaces/typescript-watch-repo/b/2/b-impl/b/node_modules 1 undefined Failed Lookup Locations
sysLog:: /workspaces/typescript-watch-repo/b/2/b-impl/b/node_modules:: Defaulting to watchFile
Elapsed:: 9.65250200000014ms DirectoryWatcher:: Added:: WatchInfo: /workspaces/typescript-watch-repo/b/2/b-impl/b/node_modules 1 undefined Failed Lookup Locations
FileWatcher:: Added:: WatchInfo: /workspaces/typescript-watch-repo/a/1/a-impl/a/package.json 2000 undefined File location affecting resolution
sysLog:: /workspaces/typescript-watch-repo/a/1/a-impl/a/package.json:: Defaulting to watchFile
DirectoryWatcher:: Added:: WatchInfo: /workspaces/typescript-watch-repo/b/2/b-impl/b/node_modules/@types 1 undefined Type roots
Elapsed:: 0.1385210000000825ms DirectoryWatcher:: Added:: WatchInfo: /workspaces/typescript-watch-repo/b/2/b-impl/b/node_modules/@types 1 undefined Type roots
DirectoryWatcher:: Added:: WatchInfo: /workspaces/typescript-watch-repo/b/2/b-impl/node_modules/@types 1 undefined Type roots
Elapsed:: 0.07700199999999313ms DirectoryWatcher:: Added:: WatchInfo: /workspaces/typescript-watch-repo/b/2/b-impl/node_modules/@types 1 undefined Type roots
DirectoryWatcher:: Added:: WatchInfo: /workspaces/typescript-watch-repo/b/2/node_modules/@types 1 undefined Type roots
Elapsed:: 0.050179999999954816ms DirectoryWatcher:: Added:: WatchInfo: /workspaces/typescript-watch-repo/b/2/node_modules/@types 1 undefined Type roots
[6:33:35 PM] Found 0 errors. Watching for file changes.
Files: 10
Lines of Library: 24020
Lines of Definitions: 4
Lines of TypeScript: 1
Lines of JavaScript: 0
Lines of JSON: 0
Lines of Other: 0
Identifiers: 38917
Symbols: 27630
Types: 11084
Instantiations: 2656
Memory used: 70483K
Assignability cache size: 3484
Identity cache size: 0
Subtype cache size: 0
Strict subtype cache size: 0
I/O Read time: 0.07s
Parse time: 0.52s
ResolveModule time: 0.03s
Program time: 0.65s
Bind time: 0.22s
Check time: 0.96s
transformTime time: 0.01s
commentTime time: 0.00s
I/O Write time: 0.01s
Source Map time: 0.00s
printTime time: 0.02s
Emit time: 0.02s
Total time: 1.85s
DirectoryWatcher:: Added:: WatchInfo: /workspaces/typescript-watch-repo/b/2/b-impl/b/src 1 undefined Wild card directory
Elapsed:: 0.015758000000005268ms DirectoryWatcher:: Added:: WatchInfo: /workspaces/typescript-watch-repo/b/2/b-impl/b/src 1 undefined Wild card directory
When running with typescript@latest
, the command doesn't finish in any reasonable amount of time because it is busy enumerating and creating directory watchers on all 1111110 irrelevant directories in a/2/junk
that were created during the setup phase (TypeScript 5.0.6 correctly ignored these).
Edit: After letting it run to see if it would finish, eventually the OS killed the process:
sysLog:: /workspaces/typescript-watch-repo/a/2/junk/5/3/3/3/5/6:: Defaulting to watchFile
sysLog:: /workspaces/typescript-watch-repo/a/2/junk/5/3/3/3/5/7:: Defaulting to watchFile
sysLog:: /workspaces/typescript-watch-repo/a/2/junk/5/3/3/3/5/8:: Defaulting to watchFile
sysLog:: /workspaces/typescript-watch-repo/a/2/junk/5/3/3/3/5/9:: Defaulting to watchFile
sysLog:: /workspaces/typescript-watch-repo/a/2/junk/5/3/3/3/6:: Defaulting to watchFile
sysLog:: /workspaces/typescript-watch-repo/a/2/junk/5/3/3/3/6/0:: Defaulting to watchFileTerminated
βELIFECYCLEβ Command failed with exit code 143.
This was specifically handled to watch for mono repo scenarios correctly as part of https://github.com/microsoft/TypeScript/pull/53591 , https://github.com/microsoft/TypeScript/pull/55738 and some other changes . In 5.0 not watching a
meant that any changes will not be reflected and we had lots of reports about that. So we have enabled that path. Also depending on how and where you are hosting the repro ( if its in path deep enough) we would have watched for "a" ..
We allow passing watchOptions
on command line or config and that can be used to configure not ignoring watches in a
or its subfolders.
This issue has been marked as "Working as Intended" and has seen no recent activity. It has been automatically closed for house-keeping purposes.
π Search Terms
watch, excess, files, directories
π Version & Regression Information
β― Playground Link
No response
π» Code
Repro
Steps:
Once the build finishes, create a file
tmp.txt
ina/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