Skip to content

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

Closed
@frederikdekegel

Description

@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%. cpu
.

Related Issues:

Activity

changed the title [-]Typescript: TSServer: Code Fixes: Import missing imports with a symlinked node_modules folder[/-] [+]Typescript: TSServer: Code Fixes: Import missing imports with a symlinked node_modules folder is very slow[/+] on Sep 16, 2020
sheetalkamat

sheetalkamat commented on Oct 27, 2020

@sheetalkamat
Member

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

frederikdekegel

frederikdekegel commented on Nov 1, 2020

@frederikdekegel
Author

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

sheetalkamat commented on Nov 2, 2020

@sheetalkamat
Member

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

frederikdekegel commented on Nov 3, 2020

@frederikdekegel
Author

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

  2. Open Visual Studio Code

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

  4. In Visual Studio Code, open a terminal

  5. Execute pnpm install

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

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

  8. 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".

action1a

action1b

  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.

action2

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

sheetalkamat commented on Nov 4, 2020

@sheetalkamat
Member

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

39 remaining items

andrewbranch

andrewbranch commented on Jan 4, 2021

@andrewbranch
Member

@walkerburgin if you wouldn’t mind taking measurements from #42150, that would be great! The last comment from typescript-bot has an npm link you can use.

walkerburgin

walkerburgin commented on Jan 4, 2021

@walkerburgin

@andrewbranch here's what I'm seeing. Same file and action as before, but a different commit:

Version  One (ms) Two (ms) Three (ms) Mean (ms)
4.1.2-patch (our patch from here ) 1,066.24 976.72 1,046.99 1,029.98
4.1.2 20,222.36 18,742.93 19,074.43 19,346.57
4.2.0-insiders.20201229 (#42150) 702.38 679.80 678.70 686.96

Before:
image

After:
image

Looks better to me!

rhyek

rhyek commented on Jan 27, 2021

@rhyek

Currently using 4.2.0-dev.20210126 and can confirm the fix works.

added a commit that references this issue on Feb 3, 2021
Jack-Works

Jack-Works commented on May 25, 2021

@Jack-Works
Contributor

Seems this has partially regressed.

I'm using 4.4.0-dev.20210522

Type import use and trigger completion, some of the results are pointing into the .pnpm folder.

image

If I accept the completion, it will becomes

import useControlled from '.pnpm/@material-ui+utils@5.0.0-alpha.33_bbe24f3d390b2dd762addfb9b4a0c180/node_modules/@material-ui/utils/useControlled'

If I trigger the code fix on undefined useControlled, the .pnpm one appears on the first.

image

andrewbranch

andrewbranch commented on May 25, 2021

@andrewbranch
Member

Regression was somewhere in #43149. I’ll take a look. Thanks @Jack-Works!

andrewbranch

andrewbranch commented on May 26, 2021

@andrewbranch
Member

@Jack-Works I was able to recreate some problems like this, but would you mind trying out #44259 to make sure it handles the complexities of a real project?

nwparker

nwparker commented on Jan 1, 2022

@nwparker

In ts 4.5.4 this seems broken again

image

image

andrewbranch

andrewbranch commented on Feb 1, 2022

@andrewbranch
Member

@nwparker can you share a repro?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

Domain: PerformanceReports of unusually slow behaviorFix AvailableA PR has been opened for this issueNeeds InvestigationThis issue needs a team member to investigate its status.

Type

No type

Projects

No projects

Relationships

None yet

    Development

    Participants

    @ldiego08@rhyek@DanielRosenwasser@styu@walkerburgin

    Issue actions

      Typescript: TSServer: Code Fixes: Import missing imports with a symlinked node_modules folder is very slow · Issue #40584 · microsoft/TypeScript