Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

auto imports are broken with yarn PNP on Windows #39331

Closed
trulysinclair opened this issue Jun 28, 2020 · 23 comments
Closed

auto imports are broken with yarn PNP on Windows #39331

trulysinclair opened this issue Jun 28, 2020 · 23 comments
Assignees
Labels
External Relates to another program, environment, or user action which we cannot control.

Comments

@trulysinclair
Copy link

Issue Type: Bug

I haven't changed my settings at all, but after upgrading to the newest patch my imports and suggestions no longer show which package the import recommendation is coming from. Say I have 2 Configurations and I want to auto-import the one from webpack, it should say which package the current suggestion comes from, and when hitting ENTER it doesn't import, but rather just finishes the line.

Screenshot_7

VS Code version: Code 1.46.1 (cd9ea6488829f560dc949a8b2fb789f3cdc05f5d, 2020-06-17T21:13:20.174Z)
OS version: Windows_NT x64 10.0.18363

System Info
Item Value
CPUs Intel(R) Core(TM) i5-8350U CPU @ 1.70GHz (8 x 1896)
GPU Status 2d_canvas: enabled
flash_3d: enabled
flash_stage3d: enabled
flash_stage3d_baseline: enabled
gpu_compositing: enabled
multiple_raster_threads: enabled_on
oop_rasterization: disabled_off
protected_video_decode: enabled
rasterization: enabled
skia_renderer: disabled_off_ok
video_decode: enabled
viz_display_compositor: enabled_on
viz_hit_test_surface_layer: disabled_off_ok
webgl: enabled
webgl2: enabled
Load (avg) undefined
Memory (System) 7.92GB (1.60GB free)
Process Argv C:\Users\adria\Desktop\Grim\projects\yarnberry-toolkit
Screen Reader no
VM 0%
Extensions (30)
Extension Author (truncated) Version
better-comments aar 2.0.5
vscode-zipfs arc 2.0.0
github-markdown-preview bie 0.0.2
markdown-checkbox bie 0.1.3
markdown-emoji bie 0.0.9
markdown-preview-github-styles bie 0.1.6
markdown-yaml-preamble bie 0.0.4
vscode-eslint dba 2.1.5
javascript-ejs-support Dig 1.3.1
gitlens eam 10.2.2
vsc-material-theme Equ 32.8.0
vsc-material-theme-icons equ 1.1.4
prettier-vscode esb 5.1.0
vscode-systemd-support han 0.1.1
vscode-docker ms- 1.3.1
vscode-kubernetes-tools ms- 1.2.1
remote-containers ms- 0.122.1
remote-ssh ms- 0.51.0
remote-ssh-edit ms- 0.51.0
remote-wsl ms- 0.44.4
vscode-remote-extensionpack ms- 0.20.0
vscode-markdown-notebook ms- 0.0.7
debugger-for-chrome msj 4.12.8
env-cmd-file-syntax Nix 0.1.2
material-icon-theme PKi 4.2.0
vscode-xml red 0.12.0
vscode-yaml red 0.8.0
code-spell-checker str 1.9.0
vscodeintellicode Vis 1.2.8
vscode-nginx wil 0.7.2

(1 theme extensions excluded)

@trulysinclair trulysinclair changed the title auto imports are broken auto imports are broken (Yarn PnP issue probably) Jun 28, 2020
@trulysinclair
Copy link
Author

So I renamed this when I accidentally typed a NodeJS package name and realized it supports those auto-imports, but not my dependencies. Given I'm using Yarn 2 which is defacto PnP'd now, I'll assume this is relevant to that. I recall the newest Code update fixed the issue with Go To Definition with PnP, or it was thanks to ZipFS or both.

However, it looks like auto-imports are broken in PnP projects.

@mjbvz mjbvz transferred this issue from microsoft/vscode Jun 29, 2020
@mjbvz
Copy link
Contributor

mjbvz commented Jun 29, 2020

Possible duplicate of #28289

@mjbvz mjbvz removed their assignment Jun 29, 2020
@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label Jun 30, 2020
@RyanCavanaugh RyanCavanaugh added this to the Typescript 4.0.1 milestone Jun 30, 2020
@andrewbranch
Copy link
Member

@TheGrimSilence can you grab the TS Server log generated during that action? Also, can you try this with the JS/TS Nightly extension if you haven’t already?

@trulysinclair
Copy link
Author

Looks like it's trying to look for "@vsintellicode/typescript-intellicode-plugin" but can't find it. So this stems from Yarn 2's patch not including it. I tried yarn pnpify --sdk to see if that would pull a patch but it didn't. But it's definitely a Yarn issue.
tsserver.log

@trulysinclair
Copy link
Author

Using the Nightly wouldn't work since Yarn 2 uses Plug'n'Play and a virtual filesystem which uses a custom version of the latest TypeScript release. Effectively pointless in this scenario unfortunately.

@andrewbranch
Copy link
Member

Would you be able to provide either an example repo or step-by-step instructions on how to create a simple reproduction of the problem? Where does that version of TypeScript live? In order for VS Code to use it, you’d have to point to it in .vscode/settings.json. Do you have a custom typescript.tsdk entry in there? Or is there a VS Code extension that’s required for editor operations to work properly with PNP?

@trulysinclair
Copy link
Author

https://github.com/thegrimsilence/yarnberry-toolkit uses Yarn 2's Zero-Install so cloning that will give you a direct state of my own local repo since I commit frequently.

And yeah Yarn 2 has you install @yarnpkg/pnpify and run yarn pnpify --sdk to auto-configure every supported sdk, including typescript. Basically, it updates your local settings and installs a local .yarn/cache/sdks/typescript which hosts a custom TS Server and binaries. It's then added into your .vscode/settings.json to point to that server.

@trulysinclair
Copy link
Author

Got it, it's an issue with the Visual Studio Intellicode Extension, but disabling it does nothing except removing the error from the server log. Reloaded the workspace and it still has auto-import problems.

@trulysinclair
Copy link
Author

Screenshot_10
The issue is that Command in the suggestions comes from Clipanion. In a normal workflow (no pnp or virtual files) the suggestion would show what module it's from (or local file) and then hitting Enter would of course auto import it and append it to the already active import list for Clipanion.
Screenshot_11

@andrewbranch
Copy link
Member

Kapture 2020-06-30 at 15 25 11

Seems to work for me; am I missing something obvious here?

@trulysinclair
Copy link
Author

I'm going to have to play around with settings this is a fresh install of Code
ezgif com-video-to-gif (1)

@trulysinclair
Copy link
Author

Running with extensions disabled did nothing to help. I'm really not sure what it could be at this point.

@andrewbranch
Copy link
Member

I'll try to give a try on Windows to see if that makes a difference.

@trulysinclair
Copy link
Author

If it's windows I'm throwing my surface book out the window 😂

@andrewbranch
Copy link
Member

For the brief time that I had a Surface Book, I was frequently concerned for the safety of pedestrians walking underneath my office window 🙊

@trulysinclair
Copy link
Author

trulysinclair commented Jun 30, 2020

Yeah it's a nice-to-have but development has been horrible. WSL 2 makes things... okay, but I just wish Microsoft made a variant of Windows that ran exclusively on Linux since I can't afford a mac

@trulysinclair
Copy link
Author

For whatever reason, this has to be related to Windows. I fresh installed VS Code, no custom settings, no extensions, still the same problem. A headache is what it is

@andrewbranch
Copy link
Member

Yep, it repros for me on Windows. Thanks for the help!

@andrewbranch andrewbranch added Bug A bug in TypeScript and removed Needs Investigation This issue needs a team member to investigate its status. labels Jun 30, 2020
@andrewbranch andrewbranch changed the title auto imports are broken (Yarn PnP issue probably) auto imports are broken with yarn PNP on Windows Jun 30, 2020
@trulysinclair
Copy link
Author

Thank you. I was worried I'd have to give up my career goals if it was a local issue I was overlooking 😂😂

@trulysinclair
Copy link
Author

By the way, @andrewbranch what software did you use to capture and save as a Gif? cause currently I have to use Xbox Game Bar to capture and then save it as a horribly compressed gif which as you saw was really small even though I have a 4K screen.

@andrewbranch
Copy link
Member

andrewbranch commented Jul 1, 2020

I use https://getkap.co, which is macOS-only unfortunately.

Ok, so I’ve tracked this down to be the fault of yarn’s wrapper around TypeScript. It’s intercepting messages to TS Server, deserializing them, looking for file paths, modifying those paths before passing them on. Unfortunately, it seems to be doing this incorrectly on Windows, at least some of the time. First, when we send back the full list of completions, the Configuration item looks like this on the server:

{
  "name": "Configuration",
  "kind": "interface",
  "kindModifiers": "declare",
  "sortText": "5",
  "hasAction": true,
  "source": "c:/Users/andrew/yarnberry-toolkit/.yarn/cache/@types-webpack-npm-4.41.18-f868232c99-43fefaccab.zip/node_modules/@types/webpack/index"
}

The TS Server wrapper sees the path with .zip in it, and does some processing to serialize it like this (note the zip:/ prefix on source) before sending it to the client:

{
  "name": "Configuration",
  "kind": "interface",
  "kindModifiers": "declare",
  "sortText": "5",
  "hasAction": true,
  "source": "zip:/c:/Users/andrew/yarnberry-toolkit/.yarn/cache/@types-webpack-npm-4.41.18-f868232c99-43fefaccab.zip/node_modules/@types/webpack/index"
}

Now, when you highlight that item in VS Code, we make another request to get the details for that completion item. That request includes the name and source we got from the last response. The raw incoming message looks like this:

{
  "seq": 53,
  "type": "request",
  "command": "completionEntryDetails",
  "arguments": {
    "file": "c:/Users/andrew/yarnberry-toolkit/src/tools/BaseCommand.ts",
    "line": 4,
    "offset": 18,
    "entryNames": [
      {
        "name": "Configuration",
        "source": "zip:/c:/Users/andrew/yarnberry-toolkit/.yarn/cache/@types-webpack-npm-4.41.18-f868232c99-43fefaccab.zip/node_modules/@types/webpack/index"
      }
    ]
  }
}

However, before sending it on to TS Server proper, the wrapper notices the zip:/ path and changes the response to this:

{
  "seq": 53,
  "type": "request",
  "command": "completionEntryDetails",
  "arguments": {
    "file": "c:/Users/andrew/yarnberry-toolkit/src/tools/BaseCommand.ts",
    "line": 4,
    "offset": 18,
    "entryNames": [
      {
        "name": "Configuration",
        "source": "/c:/Users/andrew/yarnberry-toolkit/.yarn/cache/@types-webpack-npm-4.41.18-f868232c99-43fefaccab.zip/node_modules/@types/webpack/index"
      }
    ]
  }
}

Uh oh! Now we’ve wound up with a superfluous leading slash on that source. The rest of the process entails trying to match up existing module specifiers with that source, which we will obviously not be able to do since it’s been mangled.

So yep, this is a yarn bug.

@andrewbranch andrewbranch added External Relates to another program, environment, or user action which we cannot control. and removed Bug A bug in TypeScript labels Jul 1, 2020
@trulysinclair
Copy link
Author

Thank you for your help! I'll take this over to yarnpkg/berry and let them know the details so they can fix it.

@arcanis
Copy link

arcanis commented Jul 1, 2020

Thanks for investigating! This is now fixed in yarnpkg/berry#1534

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
External Relates to another program, environment, or user action which we cannot control.
Projects
None yet
Development

No branches or pull requests

5 participants