TypeScript icon indicating copy to clipboard operation
TypeScript copied to clipboard

Watch failed lookups, affecting locations only if resolution was failed

Open sheetalkamat opened this issue 6 months ago • 9 comments

This is the most aggressive version : 3 types of tests failure

  1. edits to package.json are ignored - most scenarios these edits will be watched for files's mode
  2. alternate js result to report error is not watched - but that changing and hence change to error message is almost not a normal scenario
  3. file was resolved in a node_modules folder but it appears in node_modules somewhere we would have found before that node_modules lookup - restart tsserver as this seems uncommon scenario, Left the tests to fail

Number of watched resolutions went down!! so invalidating time should go down drastically

Info 17690[12:45:13.042] Finishing updateGraphWorker: Project: tsconfig.json projectStateVersion: 1 projectProgramVersion: 0 structureChanged: true structureIsReused:: Not Elapsed: 47915.4494ms
Info 17691[12:45:13.043] ResolutionCache:: {
 "resolutionsWithFailedLookups": 14105,
 "resolutionsWithOnlyAffectingLocations": 4,
 "directoryWatchesOfFailedLookups": 1288,
 "fileWatchesOfAffectingLocations": 3157,
 "resolvedFileToResolution": 11309
}

to

Info 13556[13:26:19.042] Finishing updateGraphWorker: Project: tsconfig.json projectStateVersion: 1 projectProgramVersion: 0 structureChanged: true structureIsReused:: Not Elapsed: 42156.215200000006ms
Info 13557[13:26:19.042] ResolutionCache:: {
 "resolutionsWithFailedLookups": 151,
 "resolutionsWithOnlyAffectingLocations": 0,
 "directoryWatchesOfFailedLookups": 20,
 "fileWatchesOfAffectingLocations": 2796,
 "resolvedFileToResolution": 11309
}

sheetalkamat avatar Jun 13 '25 20:06 sheetalkamat

@typescript-bot pack this

sheetalkamat avatar Jun 13 '25 20:06 sheetalkamat

Starting jobs; this comment will be updated as builds start and complete.

Command Status Results
pack this ✅ Started ✅ Results

typescript-bot avatar Jun 13 '25 20:06 typescript-bot

Hey @sheetalkamat, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/165350/artifacts?artifactName=tgz&fileId=2771ED305D29E25E36FCABC5F9002D96578FDF58FFA22A77769BA30DF2B9868C02&fileName=/typescript-5.9.0-insiders.20250613.tgz"
    }
}

and then running npm install.


There is also a playground for this build and an npm module you can use via "typescript": "npm:@typescript-deploys/[email protected]".;

typescript-bot avatar Jun 13 '25 20:06 typescript-bot

@typescript-bot cherry-pick this to release-5.8

sheetalkamat avatar Jun 13 '25 22:06 sheetalkamat

Starting jobs; this comment will be updated as builds start and complete.

Command Status Results
cherry-pick this to release-5.8 ✅ Started ✅ Results

typescript-bot avatar Jun 13 '25 22:06 typescript-bot

Hey, @sheetalkamat! I've created #61864 for you. This involved updating baselines; please check the diff.

typescript-bot avatar Jun 13 '25 22:06 typescript-bot

So, for anyone reading this, and my own understanding - the problem is that typically if you do an import of foo, we have to run through many different possible locations to resolve a file. However, even if we find the module, we will still have to install file watchers for all of the locations we looked at before successfully resolving. This is to accurately capture some new file coming into existence and taking precedence. That might be an npm install on an inner directory, or a file with a "higher priority" extension, or something else entirely.

But that is generally uncommon if we've actually resolved a module. And so it sounds like the idea here is that if a module was actually resolved and found, we won't add watchers for any place we would have looked. If you do find yourself in an uncommon situation, you'll need to restart tsserver.

DanielRosenwasser avatar Jun 13 '25 22:06 DanielRosenwasser

edits to package.json are ignored - most scenarios these edits will be watched for files's mode

Why does this happen anyway? Shouldn't this trigger a re-resolve?

alternate js result to report error is not watched - but that changing and hence change to error message is almost not a normal scenario

This one I'm not clear on - can you elaborate?

DanielRosenwasser avatar Jun 13 '25 23:06 DanielRosenwasser

edits to package.json are ignored - most scenarios these edits will be watched for files's mode

Why does this happen anyway? Shouldn't this trigger a re-resolve?

https://github.com/microsoft/TypeScript/pull/61861/commits/3eb3e21021d4aba663dae5b1a102d005c8543bb9#r2152786800 This shows example of when it doesnt retrigger re-resolve. I can tweak it to watch it if its node10

alternate js result to report error is not watched - but that changing and hence change to error message is almost not a normal scenario

This one I'm not clear on - can you elaborate?

https://github.com/microsoft/TypeScript/pull/61861/commits/3eb3e21021d4aba663dae5b1a102d005c8543bb9#r2152817116 This shows example and how this comes into play with what error is displayed

sheetalkamat avatar Jun 17 '25 17:06 sheetalkamat

edits to package.json are ignored - most scenarios these edits will be watched for files's mode

I know this came from an internal repro; do we know if they're actually using a non-node10 mode such that this would be the case? That and, if I have a package consisting solely of .cjs/.mjs files, will package.json ever be watched?

Do we know how many watches removed correspond to the three things you've listed?

jakebailey avatar Jun 30 '25 20:06 jakebailey

They use esnext for resolution. I didnt do the three numbers but if you would like, i would have to do builds to get those number.. the only numbers i got were mentioned in the description which shows how many resolutions are not watched any more as well as reduction in number of watches

sheetalkamat avatar Jul 01 '25 05:07 sheetalkamat

https://github.com/microsoft/TypeScript/commit/3eb3e21021d4aba663dae5b1a102d005c8543bb9#r2152786800 This shows example of when it doesnt retrigger re-resolve. I can tweak it to watch it if its node10

https://github.com/microsoft/TypeScript/commit/3eb3e21021d4aba663dae5b1a102d005c8543bb9#r2152817116 This shows example and how this comes into play with what error is displayed

Unfortunately the links aren't directing me to the right section, and I'll be honest - it's not obvious what is changing from the diffs.


With esnext, I can understand why package.json wouldn't affect resolution. It probably should be for node10 too, but especially so for node16+. Maybe I've misunderstood though. When you said

edits to package.json are ignored - most scenarios these edits will be watched for files's mode

were you saying that that's expected for all resolution? Or that only some tests are affected by this?


alternate js result to report error is not watched

I'm still not clear on this. Are you just saying that we don't watch for the existence of a .js file so that we can give an error message when a .ts file doesn't exist?

DanielRosenwasser avatar Jul 01 '25 21:07 DanielRosenwasser