microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
100.16k stars 12.38k forks source link

auto-import using yarn link protocol, is importing the path, rather than the name #48712

Open thepuzzlemaster opened 2 years ago

thepuzzlemaster commented 2 years ago

In my package.json, I have the following yarn link protocols set up in dependencies:

"modules": "link:./src/modules",
"mocks": "link:./__mocks__",

When I auto-import a dependency from either of these files, the import statement looks like this:

import { foo } from 'src/modules/foo'
import { bar } from '__mocks__/bar

And while this does compile and work fine, I would expect the imports to look like this:

import { foo } from 'modules/foo'
import { bar } from 'mocks/bar'

If I manually adjust the imports to my preferred format, it also works fine. But why would vscode be pulling the import path from the right side of the declaration, rather than the left side? Am I incorrect in my assumption that it should be using the 'name', rather than the path for the imports? All other dependencies reference the package name when being imported.

Verified that this behaves the same with all extensions disabled.

VS Code version: Code 1.62.3 (Universal) (ccbaa2d27e38e5afa3e5c21c1c7bef4657064247, 2021-11-17T07:59:13.865Z) OS version: Darwin arm64 21.1.0 Restricted Mode: No

System Info |Item|Value| |---|---| |CPUs|Apple M1 Max (10 x 24)| |GPU Status|2d_canvas: enabled
gpu_compositing: enabled
metal: disabled_off
multiple_raster_threads: enabled_on
oop_rasterization: enabled
opengl: enabled_on
rasterization: enabled
skia_renderer: disabled_off_ok
video_decode: enabled
webgl: enabled
webgl2: enabled| |Load (avg)|3, 3, 4| |Memory (System)|32.00GB (1.13GB free)| |Process Argv|--crash-reporter-id eee85e01-b198-4016-a620-0f026921ed6f| |Screen Reader|no| |VM|0%|
Extensions (26) Extension|Author (truncated)|Version ---|---|--- multi-cursor-case-preserve|Car|1.0.5 path-intellisense|chr|2.6.0 vscode-svgviewer|css|2.0.0 vscode-eslint|dba|2.2.2 githistory|don|0.6.19 prettier-vscode|esb|9.0.0 beautify|Hoo|1.5.0 vscode-peacock|joh|4.0.0 chat|kar|0.35.0 expand-region|let|0.1.4 HTMLHint|mka|0.10.0 vscode-duplicate|mrm|1.2.1 vsliveshare|ms-|1.0.5090 vsliveshare-audio|ms-|0.1.91 vsliveshare-pack|ms-|0.4.0 vetur|oct|0.35.0 vscode-subword-navigation|ow|1.2.0 partial-diff|ryu|1.4.3 vscode-multiclip|sle|0.1.5 language-stylus|sys|1.14.1 vscode-counter|uct|2.3.0 alphabetical-sorter|ue|2.0.1 vscode-icons|vsc|11.7.0 gitblame|wad|8.1.0 wallaby-vscode|Wal|1.0.319 intelligence-change-case|zhe|1.1.0 (1 theme extensions excluded)
A/B Experiments ``` vsliv368cf:30146710 vsreu685:30147344 python383cf:30185419 vspor879:30202332 vspor708:30202333 vspor363:30204092 vstes627:30244334 pythontb:30283811 pythonptprofiler:30281270 vshan820:30294714 vstes263:30335439 vscoreces:30384385 pythondataviewer:30285071 vscod805:30301674 pythonvspyt200:30340761 binariesv615:30325510 bridge0708:30335490 bridge0723:30353136 pythonrunftest32:30373476 pythonf5test824:30373475 javagetstartedt:30391933 pythonvspyt187:30373474 vsaa593:30376534 vscexrecpromptt2:30404948 vscop804:30404766 vs360:30404995 vsrem710:30405998 ```
thepuzzlemaster commented 2 years ago

I'll also add, fwiw, if you already have an existing import using the module name, it will add to that existing import, rather than create a new one using the path.

e.g. in the example above, if I also add baz as an auto-imported dependency, it would look like:

import { bar, baz } from 'mocks/bar'

rather than:

import { bar } from 'mocks/bar'
import { baz } from '__mocks__/bar'
mjbvz commented 2 years ago

Please share a minimal example project that demonstrates this problem

thepuzzlemaster commented 2 years ago

Okay, after further investigation while creating a minimal example project, I realized that vscode actually is completely unaware of the yarn link protocols when it comes to auto-importing. What is actually happening here, is setting up a "baseUrl": "." in my tsconfig.json file, and so all import paths are based off that baseUrl, which just so happens to always align with the "path" portion of the yarn link protocol.

So I suppose, rather than a bug, this should be a feature-request for vscode to recognize the existence of a yarn link protocol, and use that for imports.

Here's the repro (note the yarn-link-protocol-defect branch).

If you auto-import myMock in any of the files under "pages", you'll see it imports based on the baseUrl set up in the tsconfig. If you remove the "baseUrl" property in tsconfig, it'll import a relative path.

If you've run yarn install, you can do import {myMock} from 'mocks/myMock'. It'd be lovely if vscode could use that by default, as it (usually) does with path aliases setup in tsconfig.json.

JeppeKlitgaard commented 2 years ago

I would also love to see this feature, which is sorely missed for people using Yarn 2+ and absolute imports via the link protocol.

mjbvz commented 2 years ago

Moving upstream to get more feedback from the TS team

Possible related: #37414

andrewbranch commented 2 years ago

Does any package manager besides yarn respect this protocol?

thepuzzlemaster commented 2 years ago

Not that I'm aware of. I believe this is a yarn-specific protocol.

andrewbranch commented 2 years ago

if you already have an existing import using the module name, it will add to that existing import

This should also work cross-file in the same project as long as you have a tsconfig.json/jsconfig.json. I.e., once anything in your project imports from the symlink path, do auto-imports in other files use the symlink path too?

thepuzzlemaster commented 2 years ago

Oh, interesting! So this definitely was not happening for me. Every time I'd auto-import, it would use the incorrect version, based on the root path, regardless if that import exists in other files, or other similar imports exist in the current file. But your comment about tsconfig/jsconfig gave me an idea. I removed the baseUrl entry in my tsconfig.json, and now auto-importing IS using the symlink path! This is a huge improvement!

andrewbranch commented 2 years ago

Hesitant to commit to adding yarn-specific resolution logic when the time-tested workaround of “just write it once and you’re good to go” is working, but will listen for feedback and gauge demand here in this thread.

thepuzzlemaster commented 2 years ago

FWIW, this will be much less painful for me now, knowing the behavior without a baseUrl entry gets me 95% of the way there. I can't think of any reason I would need the baseUrl, so I would be okay with this not meeting the bar of things to work on. I'm sure your backlog is full of great ideas that would have a larger impact than this. I'd certainly be open to hearing other peoples' thoughts if anyone feels differently. But I wouldn't mind closing the issue if no one else chimes in.