Watcher recursively watches irrelevant directories about typescript HOT 13 OPEN

Issue updated with minimal repro.

from typescript.

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/[email protected]/node_modules/typescript/lib/lib.d.ts 250 undefined Source file
sysLog:: /workspaces/typescript-watch-repo/node_modules/.pnpm/[email protected]/node_modules/typescript/lib/lib.d.ts:: Defaulting to watchFile
FileWatcher:: Added:: WatchInfo: /workspaces/typescript-watch-repo/node_modules/.pnpm/[email protected]/node_modules/typescript/lib/lib.es5.d.ts 250 undefined Source file
sysLog:: /workspaces/typescript-watch-repo/node_modules/.pnpm/[email protected]/node_modules/typescript/lib/lib.es5.d.ts:: Defaulting to watchFile
FileWatcher:: Added:: WatchInfo: /workspaces/typescript-watch-repo/node_modules/.pnpm/[email protected]/node_modules/typescript/lib/lib.decorators.d.ts 250 undefined Source file
sysLog:: /workspaces/typescript-watch-repo/node_modules/.pnpm/[email protected]/node_modules/typescript/lib/lib.decorators.d.ts:: Defaulting to watchFile
FileWatcher:: Added:: WatchInfo: /workspaces/typescript-watch-repo/node_modules/.pnpm/[email protected]/node_modules/typescript/lib/lib.decorators.legacy.d.ts 250 undefined Source file
sysLog:: /workspaces/typescript-watch-repo/node_modules/.pnpm/[email protected]/node_modules/typescript/lib/lib.decorators.legacy.d.ts:: Defaulting to watchFile
FileWatcher:: Added:: WatchInfo: /workspaces/typescript-watch-repo/node_modules/.pnpm/[email protected]/node_modules/typescript/lib/lib.dom.d.ts 250 undefined Source file
sysLog:: /workspaces/typescript-watch-repo/node_modules/.pnpm/[email protected]/node_modules/typescript/lib/lib.dom.d.ts:: Defaulting to watchFile
FileWatcher:: Added:: WatchInfo: /workspaces/typescript-watch-repo/node_modules/.pnpm/[email protected]/node_modules/typescript/lib/lib.webworker.importscripts.d.ts 250 undefined Source file
sysLog:: /workspaces/typescript-watch-repo/node_modules/.pnpm/[email protected]/node_modules/typescript/lib/lib.webworker.importscripts.d.ts:: Defaulting to watchFile
FileWatcher:: Added:: WatchInfo: /workspaces/typescript-watch-repo/node_modules/.pnpm/[email protected]/node_modules/typescript/lib/lib.scripthost.d.ts 250 undefined Source file
sysLog:: /workspaces/typescript-watch-repo/node_modules/.pnpm/[email protected]/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.

from typescript.

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.

from typescript.

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.

from typescript.

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

from typescript.

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

from typescript.

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 ?

from typescript.

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.

from typescript.

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).

from typescript.

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.

from typescript.

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.

Option 1 - TypeScript asks for watch of exact folders

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 / other platform without recursive watch support

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

Windows / OSX / other platform with recursive watch support

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.

Option 2 - TypeScript asks to recursively watch the ancestor

In this path TypeScript directly asks for the watch host to watch /test/foo recursively.

Linux / other platform without recursive watch support

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.

Windows / OSX / other platform with recursive watch support

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.

Summary

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.

from typescript.

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.

from typescript.

This was specifically handled to watch for mono repo scenarios correctly as part of #53591 , #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.

from typescript.