Typescript: Typescript: TSServer: Code Fixes: Import missing imports with a symlinked node_modules folder is very slow

[frederikdekegel]

TypeScript Version: 3.9.7

PS: I am using Angular and therefor cannot try to use a later version of Typescript at this time.

Search Terms:
PNPM Typescript import slow

System
OS: Windows 10
Disk: SSD

Expected behavior:
When using PNPM or another packagemanager that uses a symlinked node_modules folder, the code Auto Import fixes like "Imports x from modules" and "import all missing imports" are performed within a reasonable delay. Preferably on par with NPM. With NPM as packagemanager, the code fixes "Imports x from modules" or "import all missing imports" are much faster.

Actual behavior:
The upgrade to Angular 10, also upgraded Typescript to version 3.9.7. In 3.9, support for code Auto Import fixes for imports from symlinked node_modules folder were added.

So I replaced npm with pnpm, and yes indeed, code Auto Imports fixes do function, however extremely slow.

In PNPM TSServer.log, you can see I performed 2 actions

1. Import OnInit from @angular/core
   In Visual Studio Code, the yellow lightblub appeared; I selected OnInit from @angular/core, the import statement was generated after approximately
   PNPM: 3 seconds
   NPM: < 1 second

PS: Also please note that with NPM, the Yellow Lightblub did not appear but the blue one did.

1. Import all missing imports
   In Visual Studio Code, I selected a missing import and from the blue lightblub, I selected "Import all missing imports", the import statements were generated after approximately
   PNPM: 51 seconds
   NPM: 2 seconds

This is the NPM TSServer.log.

When doing each action, CPU was at 100%.
.

Related Issues:

[sheetalkamat]

@frederikdekegel Please provide detailed repro steps along with sample code and auto import to invoke so we can investigate this.
Thanks

[frederikdekegel]

Hi @sheetalkamat, for the repro steps, I described the repro steps in the original issue. Is there anything that is not clear? I will add screenshots of my actions for sure.I do not have access to my laptop before tuesday.

As for the code, I can give you my repro, but I do not have experience with github, apart from logging issues. How do I delivery the code to you?

[sheetalkamat]

we have made lot of changes to auto import scenarios recently so we want exact repro steps, code to setup and observe difference.
So we want to able to try these steps with latest typescript and precisely.

[frederikdekegel]

Hi @sheetalkamat,

Code
I created a private repository, acv-csc-codebase, and I invited you as a collaborator. This should give you access to the code

Steps

1. Execute npm -g install pnpm

1. Open Visual Studio Code

2. In Visual Studio Code, open workspace Apps/ws.code-workspace

3. In Visual Studio Code, open a terminal

4. Execute pnpm install

5. In Visual Studio Code, make sure to use the Workspace version of Typescript, 4.0.5.

6. In Visual Studio Code, open Apps/acv-csc/apps/contact/src/app/_components/physical-direct-contact-detail/physical-direct-contact-detail.component.ts. This file is missing all of it's imports

7. Action 1: Add single missing import = In the class definition, there is a text OnInit. Put the cursor inside OnInit. On the left, the lightbulb appears. Select Import OnInit from module "@angular/core".

1. Action 2: Add all missing imports = In the class definition, there is a text SafeUnsubscribe. Put the cursor inside the text SafeUnsubscribe. On the left, the lightbulb appears. Select Add all missing imports.

Note
The codebase already uses typescript 4.0.5. I see some improvement in the speed for action 1 (add single missing import). Action 2, however, remains much slower compared to npm.

[sheetalkamat]

Adding @andrewbranch @jessetrinity to see if this is working as intended or something that can be done.
I can see that import fix all does take a while but then there are lot many errors to go through.

Info 986  [13:53:49.475] request:
    {"seq":125,"type":"request","command":"getCombinedCodeFix","arguments":{"scope":{"type":"file","args":{"file":"c:/temp/acv-csc-codebase/Apps/acv-csc/apps/contact/src/app/_components/physical-direct-contact-detail/physical-direct-contact-detail.component.ts"}},"fixId":"fixMissingImport"}}
Info 987  [13:53:50.516] forEachExternalModuleToImportFrom: filtered out 13 modules by package.json contents
Info 988  [13:53:52.039] forEachExternalModuleToImportFrom: filtered out 13 modules by package.json contents
Info 989  [13:53:53.391] forEachExternalModuleToImportFrom: filtered out 13 modules by package.json contents
Info 990  [13:53:54.706] forEachExternalModuleToImportFrom: filtered out 13 modules by package.json contents
Info 991  [13:53:56.171] forEachExternalModuleToImportFrom: filtered out 13 modules by package.json contents
Info 992  [13:53:57.822] forEachExternalModuleToImportFrom: filtered out 13 modules by package.json contents
Info 993  [13:53:59.142] forEachExternalModuleToImportFrom: filtered out 13 modules by package.json contents
Info 994  [13:54:00.523] forEachExternalModuleToImportFrom: filtered out 13 modules by package.json contents
Info 995  [13:54:02.091] forEachExternalModuleToImportFrom: filtered out 13 modules by package.json contents
Info 996  [13:54:03.637] forEachExternalModuleToImportFrom: filtered out 13 modules by package.json contents
Info 997  [13:54:04.905] forEachExternalModuleToImportFrom: filtered out 13 modules by package.json contents
Info 998  [13:54:06.079] forEachExternalModuleToImportFrom: filtered out 13 modules by package.json contents
Info 999  [13:54:07.277] forEachExternalModuleToImportFrom: filtered out 13 modules by package.json contents
Info 1000 [13:54:08.493] forEachExternalModuleToImportFrom: filtered out 13 modules by package.json contents
Info 1001 [13:54:09.861] forEachExternalModuleToImportFrom: filtered out 13 modules by package.json contents
Info 1002 [13:54:11.124] forEachExternalModuleToImportFrom: filtered out 13 modules by package.json contents
Info 1003 [13:54:12.642] forEachExternalModuleToImportFrom: filtered out 13 modules by package.json contents
Info 1004 [13:54:14.279] forEachExternalModuleToImportFrom: filtered out 13 modules by package.json contents
Info 1005 [13:54:15.542] forEachExternalModuleToImportFrom: filtered out 13 modules by package.json contents
Info 1006 [13:54:16.874] forEachExternalModuleToImportFrom: filtered out 13 modules by package.json contents
Info 1007 [13:54:18.100] forEachExternalModuleToImportFrom: filtered out 13 modules by package.json contents
Info 1008 [13:54:19.508] forEachExternalModuleToImportFrom: filtered out 13 modules by package.json contents
Info 1009 [13:54:21.024] forEachExternalModuleToImportFrom: filtered out 13 modules by package.json contents
Info 1010 [13:54:22.288] forEachExternalModuleToImportFrom: filtered out 13 modules by package.json contents
Info 1011 [13:54:23.672] forEachExternalModuleToImportFrom: filtered out 13 modules by package.json contents
Info 1012 [13:54:25.124] forEachExternalModuleToImportFrom: filtered out 13 modules by package.json contents
Info 1013 [13:54:26.330] forEachExternalModuleToImportFrom: filtered out 13 modules by package.json contents
Info 1014 [13:54:27.672] forEachExternalModuleToImportFrom: filtered out 13 modules by package.json contents
Info 1015 [13:54:28.890] forEachExternalModuleToImportFrom: filtered out 13 modules by package.json contents
Info 1016 [13:54:30.067] forEachExternalModuleToImportFrom: filtered out 13 modules by package.json contents
Info 1017 [13:54:31.237] forEachExternalModuleToImportFrom: filtered out 13 modules by package.json contents
Info 1018 [13:54:32.696] forEachExternalModuleToImportFrom: filtered out 13 modules by package.json contents
Info 1019 [13:54:34.060] forEachExternalModuleToImportFrom: filtered out 13 modules by package.json contents
Info 1020 [13:54:35.268] forEachExternalModuleToImportFrom: filtered out 13 modules by package.json contents
Info 1021 [13:54:36.529] forEachExternalModuleToImportFrom: filtered out 13 modules by package.json contents
Info 1022 [13:54:37.733] forEachExternalModuleToImportFrom: filtered out 13 modules by package.json contents
Perf 1023 [13:54:38.629] 125::getCombinedCodeFix: elapsed time (in milliseconds) 49148.5302
Info 1024 [13:54:38.629] response:
    {"seq":0,"type":"response","command":"getCombinedCodeFix","request_seq":125,"success":true,"body":{"changes

[sheetalkamat]

@frederikdekegel you would want to add @andrewbranch and @jessetrinity to private repo so they have access to it

[frederikdekegel]

@andrewbranch, @jessetrinity, you have been invited...

[rhyek]

I am also experiencing this in a pnpm monorepo with a medium-sized nestjs app and a couple of very small shared apps.

I also noticed that the auto import suggestions suggest importing packages from <root>/node_modules/.pnpm/... as in @frederikdekegel's screenshots. I was initially looking for a way to tell vscode to ignore that directory thinking i don't ever need to import from there and it could also maybe help with performance, and then I found this issue.

Is it possible to ignore that folder in a setting somewhere as a workaround?

[andrewbranch]

It seems like this is just because of the huge, huge number of symlinks that pnpm creates in a project with a large, interconnected dependency graph like this. Enabling preserveSymlinks in compilerOptions removes all the superfluous suggestions from node_modules/.pnpm, and consequently improves the performance by many, many times. Enabling that setting could potentially have some unintended effects if you were actually trying to use symlinks in your project _outside_ of node_modules, but since that ought to be extremely uncommon, I think enabling preserveSymlinks might just be a good idea for pnpm users. @frederikdekegel, could you try a full build with that and see if it causes any problems?

[ldiego08]

@andrewbranch that worked for me! 👍🏽

[rhyek]

@andrewbranch that worked for me! 👍🏽

Do you know if import suggestions are now as fast as npm or just better?

[ldiego08]

@rhyek not sure if faster, but at least just as fast as it was with yarn or npm.

[rhyek]

@ldiego08 Great! Yeah, that's what I meant, sorry. Thanks for confirming.

[frederikdekegel]

@andrewbranch,
That worked for me also! Performance for "Add all missing imports" is still laging a bit behind the performance with npm but it is definitely much better and feasible to work with.

Thank you and @sheetalkamat for your effort and time.

[nwparker]

Although there is a work around for this issue, I'm not sure if it should be closed
.
There may be valid cases where the compiler option cannot be set.
Further, it's a bad experience for any users who end up experiencing the same issue. It's pretty non-obvious what the source of the slowdowns might be. And for a user to then find this post is asking alot. Ideally the problem can be solved without user intervention.

[rhyek]

It also seems even with the workaround, there are still some performance issues that should/could be improved.

Nevertheless, it is unclear to me whose responsibility it is to guarantee a performant UX here. Could it possibly be something the PNPM maintainers need to look at?

[walkerburgin]

We just migrated a very large monorepo to PNPM and I think we're hitting the same issue. We're on OSX & [email protected] if it's helpful.

@andrewbranch I'm not sure that preserveSymlinks actually works as a workaround with PNPM, although it definitely made things faster. For example, if I have a package foo-app which takes a dev dependency on @types/jest, here's what PNPM sets up:

node_modules/.pnpm/@types/[email protected]/
└── node_modules
    ├── @types
    │   └── jest
    │       ├── LICENSE
    │       ├── README.md
    │       ├── index.d.ts
    │       └── package.json
    ├── jest-diff -> ../../../[email protected]/node_modules/jest-diff
    └── pretty-format -> ../../../[email protected]/node_modules/pretty-format
packages/foo-app/node_modules/
└── @types/jest -> ../../../node_modules/.pnpm/@types/[email protected]/node_modules/@types/jest

This structure relies on resolving @types/jest to node_modules/.pnpm/@types/[email protected]/node_modules/@types/jest instead of packages/foo-app/node_modules/@types/jest so that importing jest-diff & pretty-format resolve correctly when node walks up the tree. Setting preserveSymlinks to true results in compile errors because jest-diff and pretty-format can't be found from packages/foo-app/node_modules/@types/jest:

../../node_modules/@types/jest/index.d.ts:498:51 - error TS2307: Cannot find module 'jest-diff' or its corresponding type declarations.

498             diff(a: any, b: any, options?: import("jest-diff").DiffOptions): string | null;
                                                      ~~~~~~~~~~~

../../node_modules/@types/jest/index.d.ts:552:44 - error TS2307: Cannot find module 'pretty-format' or its corresponding type declarations.

552     type SnapshotSerializerPlugin = import('pretty-format').Plugin;
                                               ~~~~~~~~~~~~~~~

If I understand correctly, this is basically the same reason that PNPM has this in their limitations:

1. Node.js doesn't work with the --preserve-symlinks flag when executed in a project that uses pnpm.

I'm not sure if anything could be done on the PNPM side without fundamentally changing the way that it works. I'm happy to provide any additional information I can if it's helpful, folks started complaining about IDE perf pretty quickly when we switched over from Yarn to PNPM.

[andrewbranch]

Ok, I was concerned that resolution errors like that might crop up. I’ll have to do some experimentation to see how much work it would be, and whether it is even possible, to minimize the number of symlinks we process with pnpm without breaking module resolution. I’m not 100% sure whether anything can be done on _our_ side “without fundamentally changing the way that it works” either, but I’ll investigate.

[walkerburgin]

Can you help explain the intuition behind why/how preserveSymlinks makes things so much faster? I started poking around in importFixes.ts over the weekend but haven't gotten very far yet in understanding how things from the .pnpm directory are being picked up for suggestions.

A common (unfortunate) situation in our monorepo is to have two conflicting major versions of a package installed. With NPM or Yarn this ended up looking like this:

node_modules/.pnpm/
└── [email protected]/
packages/
├── app-one/node_modules          # 16.13.1 (hoisted)
├── app-two/node_modules          # 16.13.1 (hoisted)
├── app-three/node_modules        # 16.13.1 (hoisted)
├── app-four/node_modules/react   # 15.6.2
└── app-five/node_modules/react   # 15.6.2

The "dominant" version would be hoisted and conflicting version would be installed repeatedly in the node_modules directory where it's being consumed. With PNPM everything is symlinked and it looks like this:

node_modules/.pnpm/
├── [email protected]/
└── [email protected]/
packages/
├── app-one/node_modules/react --> ../../../node_modules/.pnpm/[email protected]/node_modules/react
├── app-two/node_modules/react --> ../../../node_modules/.pnpm/[email protected]/node_modules/react
├── app-three/node_modules/react --> ../../../node_modules/.pnpm/[email protected]/node_modules/react
├── app-four/node_modules/react --> ../../../node_modules/.pnpm/[email protected]/node_modules/react
└── app-five/node_modules/react --> ../../../node_modules/.pnpm/[email protected]/node_modules/react

The PNPM version should result in fewer distinct copies of libraries being installed, especially in the extreme case where roughly half the repo is on one version and half is on another. I would have thought that enabling preserveSymlinks would actually make things worse by causing all five dependencies to be treated as distinct.

Is it possible that PNPM's default "semi-strict", behind the scenes hoisting behavior could somehow expose more files to TypeScript than strictly necessary?

node_modules/.pnpm/
│    ├── node_modules/
│    │    ├── react/     # "Hoisted" so that any transitives with missing dependencies have a chance of working
│    │    ├── # ... 
├── [email protected]/
└── [email protected]/
packages/

One last thing I have noticed about PNPM is that the way it handles peer dependencies could artificially inflate the number of .d.ts files that need to be processed unless you're really strict about fulfilling peer dependencies correctly, which we've been bad at:

node_modules/.pnpm/
├── [email protected]                  # Somebody consumed `foo-lib` without fulfilling its peer dependency on react
└── [email protected][email protected]     # Peer dependency on `react` was fulfilled correctly

We're making some small progress just going through and cleaning things like that up.

Thank you so much for your help!

[andrewbranch]

This is @sheetalkamat’s area of expertise, so all I know off the top of my head is that the relevant logic begins here:

https://github.com/microsoft/TypeScript/blob/669305b914ffa2c5ed4ef78a4c54da0a282e854c/src/compiler/program.ts#L2463-L2471

You can tell because when you make all those .pnpm/* files disappear from the program via preserveSymlinks (causing this condition to evaluate to true), you can make them _reappear_ by also setting disableSourceOfProjectReferenceRedirect to true (causing the outer condition to evaluate to false). Frankly I’ve never fully understood this block of code, but it does seem odd that disableSourceOfProjectReferenceRedirect could toggle the inclusion of additional symlinked node_modules, particularly ones that are not project references. So maybe there is a bug here.

[sheetalkamat]

Well all that code is doing is that it tries to see if resolved module is d.ts file from project reference so that it can map to its source file instead. This is done with preserveSymlinks because thats when the path is not resolved to real path so we have to actually use realPath when this option is true to see if its the source of project reference redirect.

I see the issue that if its not project reference redirect we want to keep the path that we were using and not the realPath. That definitely is bug and needs fix.

[sheetalkamat]

I see the issue that if its not project reference redirect we want to keep the path that we were using and not the realPath. That definitely is bug and needs fix.

I spoke too soon. i looked at this and i dont see anything wrong with the code. It is working as intended. (for some reason i thought even though we cant find referenced project source, we are using resolved real path instead of preserved name which isnt the case.) Is there small repro depicting the issue that i can take a look where the file included goes away?

[walkerburgin]

@sheetalkamat I setup a small repro illustrating the difference in VSCode between Yarn & PNPM here: https://github.com/walkerburgin/tsserver-pnpm-repro

Here are the suggestions when installing dependencies with Yarn:

And with PNPM:

If you set "preserveSymlinks": true, PNPM matches Yarn's behavior in VSCode but it doesn't actually compile for the reasons I described above. I'm not sure how to reproduce the root performance issue itself in a small or self contained way.

[walkerburgin]

// Edit: I don't think this is actually useful. The duplication is confusing, but it doesn't seem to be directly related to the performance issue.

We spent some time looking at this today and I think we may have something useful. I've updated the repro to introduce a second dependency, @blueprintjs/table, which depends on the original package (@blueprintjs/core). Here's what the import suggestions look like now:

The thing that's interesting is that .pnpm/@blueprintjs/[email protected][email protected][email protected]/node_modules/@blueprintjs/core (the fifth suggestion) is actually still a symlink:

$ pwd
/Users/wburgin/Repositories/walkerburgin/tsserver-pnpm-repro/node_modules/.pnpm/@blueprintjs/[email protected][email protected][email protected]/node_modules/@blueprintjs/core
$ pwd -P
/Volumes/git/walkerburgin/tsserver-pnpm-repro/node_modules/.pnpm/@blueprintjs/[email protected][email protected][email protected]/node_modules/@blueprintjs/core

It looks like in this case tsserver is somehow traversing at least three different paths to the same module. It seems plausible that at the scale of a real project this could cause a performance issue?

[sheetalkamat]

i looked at the repro in https://github.com/walkerburgin/tsserver-pnpm-repro specified in https://github.com/microsoft/TypeScript/issues/40584#issuecomment-735000243

The error for module resolution failure seems like they are correct as per module resolution logic.
Here is one of the error:

node_modules/@blueprintjs/core/lib/esm/common/props.d.ts:2:26 - error TS2307: Cannot find module '@blueprintjs/icons' or its corresponding type declarations.

2 import { IconName } from "@blueprintjs/icons";
                           ~~~~~~~~~~~~~~~~~~~~

And here is the trace for resolution with preserveSymlinks set to true

======== Resolving module '@blueprintjs/icons' from 'c:/temp/tsserver-pnpm-repro/packages/foo-app/node_modules/@blueprintjs/core/lib/esm/common/props.d.ts'. ========
Explicitly specified module resolution kind: 'NodeJs'.
Loading module '@blueprintjs/icons' from 'node_modules' folder, target file type 'TypeScript'.
Directory 'c:/temp/tsserver-pnpm-repro/packages/foo-app/node_modules/@blueprintjs/core/lib/esm/common/node_modules' does not exist, skipping all lookups in it.
Scoped package detected, looking in 'blueprintjs__icons'
Directory 'c:/temp/tsserver-pnpm-repro/packages/foo-app/node_modules/@blueprintjs/core/lib/esm/node_modules' does not exist, skipping all lookups in it.
Scoped package detected, looking in 'blueprintjs__icons'
Directory 'c:/temp/tsserver-pnpm-repro/packages/foo-app/node_modules/@blueprintjs/core/lib/node_modules' does not exist, skipping all lookups in it.
Scoped package detected, looking in 'blueprintjs__icons'
Directory 'c:/temp/tsserver-pnpm-repro/packages/foo-app/node_modules/@blueprintjs/core/node_modules' does not exist, skipping all lookups in it.
Scoped package detected, looking in 'blueprintjs__icons'
Directory 'c:/temp/tsserver-pnpm-repro/packages/foo-app/node_modules/@blueprintjs/node_modules' does not exist, skipping all lookups in it.
Scoped package detected, looking in 'blueprintjs__icons'
File 'c:/temp/tsserver-pnpm-repro/packages/foo-app/node_modules/@blueprintjs/icons.ts' does not exist.
File 'c:/temp/tsserver-pnpm-repro/packages/foo-app/node_modules/@blueprintjs/icons.tsx' does not exist.
File 'c:/temp/tsserver-pnpm-repro/packages/foo-app/node_modules/@blueprintjs/icons.d.ts' does not exist.
Scoped package detected, looking in 'blueprintjs__icons'
File 'c:/temp/tsserver-pnpm-repro/packages/foo-app/node_modules/@types/blueprintjs__icons.d.ts' does not exist.
Directory 'c:/temp/tsserver-pnpm-repro/packages/node_modules' does not exist, skipping all lookups in it.
Scoped package detected, looking in 'blueprintjs__icons'
Directory 'c:/temp/tsserver-pnpm-repro/node_modules/@types' does not exist, skipping all lookups in it.
Scoped package detected, looking in 'blueprintjs__icons'
Directory 'c:/temp/node_modules' does not exist, skipping all lookups in it.
Scoped package detected, looking in 'blueprintjs__icons'
Directory 'c:/node_modules' does not exist, skipping all lookups in it.
Scoped package detected, looking in 'blueprintjs__icons'
Loading module '@blueprintjs/icons' from 'node_modules' folder, target file type 'JavaScript'.
Directory 'c:/temp/tsserver-pnpm-repro/packages/foo-app/node_modules/@blueprintjs/core/lib/esm/common/node_modules' does not exist, skipping all lookups in it.
Directory 'c:/temp/tsserver-pnpm-repro/packages/foo-app/node_modules/@blueprintjs/core/lib/esm/node_modules' does not exist, skipping all lookups in it.
Directory 'c:/temp/tsserver-pnpm-repro/packages/foo-app/node_modules/@blueprintjs/core/lib/node_modules' does not exist, skipping all lookups in it.
Directory 'c:/temp/tsserver-pnpm-repro/packages/foo-app/node_modules/@blueprintjs/core/node_modules' does not exist, skipping all lookups in it.
Directory 'c:/temp/tsserver-pnpm-repro/packages/foo-app/node_modules/@blueprintjs/node_modules' does not exist, skipping all lookups in it.
File 'c:/temp/tsserver-pnpm-repro/packages/foo-app/node_modules/@blueprintjs/icons.js' does not exist.
File 'c:/temp/tsserver-pnpm-repro/packages/foo-app/node_modules/@blueprintjs/icons.jsx' does not exist.
Directory 'c:/temp/tsserver-pnpm-repro/packages/node_modules' does not exist, skipping all lookups in it.
Directory 'c:/temp/node_modules' does not exist, skipping all lookups in it.
Directory 'c:/node_modules' does not exist, skipping all lookups in it.
======== Module name '@blueprintjs/icons' was not resolved. ========

You will note that there is no @blueprintjs/icons in the tree so this error seems correct.

When preserveSymlinks is not set the module is resolved, here is the trace:

======== Resolving module '@blueprintjs/icons' from 'c:/temp/tsserver-pnpm-repro/packages/foo-app/node_modules/.registry.npmjs.org/@blueprintjs/core/3.36.0/node_modules/@blueprintjs/core/lib/esm/common/props.d.ts'. ========
Explicitly specified module resolution kind: 'NodeJs'.
Loading module '@blueprintjs/icons' from 'node_modules' folder, target file type 'TypeScript'.
Directory 'c:/temp/tsserver-pnpm-repro/packages/foo-app/node_modules/.registry.npmjs.org/@blueprintjs/core/3.36.0/node_modules/@blueprintjs/core/lib/esm/common/node_modules' does not exist, skipping all lookups in it.
Scoped package detected, looking in 'blueprintjs__icons'
Directory 'c:/temp/tsserver-pnpm-repro/packages/foo-app/node_modules/.registry.npmjs.org/@blueprintjs/core/3.36.0/node_modules/@blueprintjs/core/lib/esm/node_modules' does not exist, skipping all lookups in it.
Scoped package detected, looking in 'blueprintjs__icons'
Directory 'c:/temp/tsserver-pnpm-repro/packages/foo-app/node_modules/.registry.npmjs.org/@blueprintjs/core/3.36.0/node_modules/@blueprintjs/core/lib/node_modules' does not exist, skipping all lookups in it.
Scoped package detected, looking in 'blueprintjs__icons'
Directory 'c:/temp/tsserver-pnpm-repro/packages/foo-app/node_modules/.registry.npmjs.org/@blueprintjs/core/3.36.0/node_modules/@blueprintjs/core/node_modules' does not exist, skipping all lookups in it.
Scoped package detected, looking in 'blueprintjs__icons'
Directory 'c:/temp/tsserver-pnpm-repro/packages/foo-app/node_modules/.registry.npmjs.org/@blueprintjs/core/3.36.0/node_modules/@blueprintjs/node_modules' does not exist, skipping all lookups in it.
Scoped package detected, looking in 'blueprintjs__icons'
Found 'package.json' at 'c:/temp/tsserver-pnpm-repro/packages/foo-app/node_modules/.registry.npmjs.org/@blueprintjs/core/3.36.0/node_modules/@blueprintjs/icons/package.json'.
'package.json' does not have a 'typesVersions' field.
File 'c:/temp/tsserver-pnpm-repro/packages/foo-app/node_modules/.registry.npmjs.org/@blueprintjs/core/3.36.0/node_modules/@blueprintjs/icons.ts' does not exist.
File 'c:/temp/tsserver-pnpm-repro/packages/foo-app/node_modules/.registry.npmjs.org/@blueprintjs/core/3.36.0/node_modules/@blueprintjs/icons.tsx' does not exist.
File 'c:/temp/tsserver-pnpm-repro/packages/foo-app/node_modules/.registry.npmjs.org/@blueprintjs/core/3.36.0/node_modules/@blueprintjs/icons.d.ts' does not exist.
'package.json' has 'typings' field 'lib/esm/index.d.ts' that references 'c:/temp/tsserver-pnpm-repro/packages/foo-app/node_modules/.registry.npmjs.org/@blueprintjs/core/3.36.0/node_modules/@blueprintjs/icons/lib/esm/index.d.ts'.
File 'c:/temp/tsserver-pnpm-repro/packages/foo-app/node_modules/.registry.npmjs.org/@blueprintjs/core/3.36.0/node_modules/@blueprintjs/icons/lib/esm/index.d.ts' exist - use it as a name resolution result.
Resolving real path for 'c:/temp/tsserver-pnpm-repro/packages/foo-app/node_modules/.registry.npmjs.org/@blueprintjs/core/3.36.0/node_modules/@blueprintjs/icons/lib/esm/index.d.ts', result 'c:/temp/tsserver-pnpm-repro/packages/foo-app/node_modules/.registry.npmjs.org/@blueprintjs/icons/3.23.0/node_modules/@blueprintjs/icons/lib/esm/index.d.ts'.
======== Module name '@blueprintjs/icons' was successfully resolved to 'c:/temp/tsserver-pnpm-repro/packages/foo-app/node_modules/.registry.npmjs.org/@blueprintjs/icons/3.23.0/node_modules/@blueprintjs/icons/lib/esm/index.d.ts' with Package ID '@blueprintjs/icons/lib/esm/[email protected]'. ========

Note how it resolves to sibling folder icons of core. There is symlink for core but not icons at C:\temp\tsserver-pnpm-repro\packages\foo-app\node_modules\@blueprintjs which results in the error.

I am not sure if this is configuration error or not. Depending on that, @andrewbranch the workaround of using preserveSymlinks might not be viable.

[walkerburgin]

I don't think preserveSymlinks is a viable workaround. It's actually called out explicitly in their docs that PNPM is not expected to work with --preserve-symlinks for the reason that you illustrated. From what I understand, it's one of the core mechanisms that PNPM is built around.

If it's helpful, we spent some time trying to profile it this evening. It looks like this is where most of the time is being spent: https://github.com/microsoft/TypeScript/blob/v4.0.5/src/compiler/moduleSpecifiers.ts#L194-L209

With PNPM:

And the same action with Yarn:

[walkerburgin]

To add another anecdotal data point, I've played around a little bit with manually patching tsserver to get a feel for the impact of various hacks/changes. All of these measurements are from getCodeFixes: elapsed time (in milliseconds) lines in tsserver's log after triggering the codefix within a file from our codebase.

Description | One (ms) | Two (ms) | Three (ms) | Mean (ms)
-- | -- | -- | -- | --
| Baseline (yarn) | ~2500 | - | - | -
| Baseline (pnpm) | 15249.4588 | 14719.7665 | 14498.8932 | 14822.70617
| (1) No host.fileExists check (here) | 9747.4674 | 11207.9277 | 10076.5404 | 10343.9785
| (2) No ts.startsWithDirectory check (here) | 9960.6242 | 9699.2911 | 9261.542 | 9640.48577
| (3) Add an inverted symlink cache | 6476.5615 | 6512.0528 | 6368.8129 | 6452.47573
| (4) 1 + 2 | 4137.2129 | 4505.9394 | 4469.6268 | 4370.92637
| (5) 1 + 2 + 3 | 2075.4725 | 2266.394 | 1974.0617 | 2105.3094

(1) seemed like a nice to have since it doesn't run if the host doesn't support it

I'm sure (2) negatively impacts the quality of the suggestions, but maybe it's possible to remove these after the fact instead of in this inner loop?

For (3) I added an additional data structure to the symlink cache which maps from the real path to an array of symlinks which point to that real directory. For this specific example/test, symlinkedDirectories has 1025 entries where every value is looped over in forEachEntry. The inverted map has 259 entries (lots of symlinks that point to the same real path, which kind of makes sense) and I'm looping through and matching against the keys instead before iterating over the values.

Code for (5) is here: https://github.com/microsoft/TypeScript/compare/release-4.0...jeremydorne:release-4.0-symlinks

It feels like it should be possible to get the performance of this back in the same ballpark as Yarn/NPM. I'm not sure I understand this code well enough to put up a PR for 3, but happy to try if it's helpful.

[styu]

I work on the same team as @walkerburgin – Internally we have decided to point VSCode to the above forked version of tsserver, and have anecdotally seen a 7-8x improvement from the devs that have tried it thus far. Is there any more information we can provide or any way we could help to contribute a mainline fix? We don't feel great about being on a fork, but folks' VSCode performance was close to unusable without it, so we're willing to help in any way we can 🙂

We really appreciate the engagement and the investigation on this issue so far!