Reduce number of fs.stat call for files under node modules

[mjbvz]

Bug Report

🔎 Search Terms

• fs.stat
• statSync
• performance

Problem

While working on web project wide IntelliSense, I noticed that TS makes a number of fs.statSync calls for files stored under the global typings cache. At least some of these calls seem to be unnecessary, such as here where we check for files called mkdirp.ts, mkdirp.tsx, mkdirp.d.ts directly under node_modules:

/Users/matb/Library/Caches/typescript/4.9/node_modules/mkdirp
/Users/matb/Library/Caches/typescript/4.9/node_modules
/Users/matb/Library/Caches/typescript/4.9/node_modules/mkdirp.ts
/Users/matb/Library/Caches/typescript/4.9/node_modules/mkdirp.tsx
/Users/matb/Library/Caches/typescript/4.9/node_modules/mkdirp.d.ts
/Users/matb/Library/Caches/typescript/4.9/node_modules/mkdirp

I don't think these specific files would ever exist, would they?

These calls are relatively fast on desktop but do have more of an overhead on web. Even on desktop, I see around 250ms total spent on all the stat calls when starting TS Server in a simple project

Can we avoid making these calls?

[Andarist]

I noticed this in September and asked @andrewbranch about this:

So Node's CJS resolution algorithm actually does look there, but its ESM one doesn't IIRC
And I think our ESM one incorrectly does, so it can be removed from ESM mode and type reference directives, but I don't want to change our CJS one, I think
Unless it could be shown to make a huge perf difference, then I think we'd consider it.

[andrewbranch]

I thought about this more and decided having these files would be such an antipattern that I’m fine to deviate from Node’s CJS algorithm here. I pitched it to the team and nobody had objections. Just forgot to follow through. So yes, we can almost certainly avoid making these calls.

[mjbvz]

Similar case: when there's an import such as node:perf_hooks in your project, we end up stating these files:

stat /Users/matb/projects/vscode/node_modules/node:perf_hooks
stat /Users/matb/projects/vscode/node_modules/node:perf_hooks.ts
stat /Users/matb/projects/vscode/node_modules/node:perf_hooks.tsx
stat /Users/matb/projects/vscode/node_modules/node:perf_hooks.d.ts
stat /Users/matb/projects/vscode/node_modules/@types/node:perf_hooks
stat /Users/matb/projects/vscode/node_modules/@types/node:perf_hooks.d.ts

Can we skip these checks for node:___ package names?

[andrewbranch]

Yeah, I think so. We probably shouldn’t do directory searches for anything that looks like an absolute URI (#35749)