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

Linked dependencies resolves to file system path #45109

Closed
Muritavo opened this issue Jul 19, 2021 · 5 comments · Fixed by #45524
Closed

Linked dependencies resolves to file system path #45109

Muritavo opened this issue Jul 19, 2021 · 5 comments · Fixed by #45524
Assignees
Labels
Bug A bug in TypeScript Domain: Auto-import Fix Available A PR has been opened for this issue

Comments

@Muritavo
Copy link

Does this issue occur when all extensions are disabled?: Yes

  • VS Code Version: 1.58.0
  • OS Version: MacOS 11.4 (Big Sur)

Obs: Not related to OS, as other OSs have the same problem

Steps to Reproduce: (A reproducible cenario can be found here)

  1. Use the link feature of npm to link a dependency to a local copy by using a symbolic link
yarn link SOME_DEPENDENCY
  1. Now when using the intellisense feature to auto import, it resolves the dependency to the relative filesystem path instead of the node_modules require path.
    For example:
// Instead of resolving as a require path
import {something} from "dependency/something";

// It resolves to the relative path
import { something } from "../../../../some/unrelated/path/dependencyFolder/something"
@Muritavo Muritavo changed the title Linked dependencies resolve to file system path Linked dependencies resolves to file system path Jul 19, 2021
@mjbvz mjbvz transferred this issue from microsoft/vscode Jul 19, 2021
@mjbvz mjbvz removed their assignment Jul 19, 2021
@antoniohansen
Copy link

I have this issue too! Any updates?

@italofelipe
Copy link

I'm having this issue either.

@andrewbranch
Copy link
Member

There are so many reasons why this doesn’t work, and only one of them is something we can easily fix. I think the easiest way to explain the problems is by explaining how we would generally expect this to work under today’s architecture, and then point out all the reasons why this example code doesn’t work that way. Here’s what should happen, at a high level:

  1. You open the vscode_linked_dependency_bug folder in VS Code and open the Dependant/index.js file
  2. We see Dependant/package.json and use it to scan node_modules for auto-imports
  3. We see Dependant/node_modules/dependency_bug and add it to the auto-import list, discovering that it’s a symlink as we go
  4. You ask for completions and we query the auto-import list, preferring the symlink because it gives us a nice node_modules-style module specifier

Here are all the reasons why that doesn’t work:

  • (2) We don’t see/process Dependant/package.json for auto-imports, because it’s not at the project root. When you don’t have a tsconfig/jsconfig file, we assume the project root is the folder you opened in VS Code. So, the only place we look for a package.json to scan for auto-imports is vscode_linked_dependency_bug itself, which doesn’t have a package.json. You can fix this yourself by adding a Dependant/jsconfig.json file. But JS users usually don’t know to do this, and it would be nice if we could infer the monorepo structure of the folder you’ve opened and set up multiple inferred projects with their root directories set appropriately. But this would be a huge, scary change that could break a ton of stuff.
  • (2) Even if we did find that package.json, it doesn’t specify dependency_bug as a dependency. This purely a user configuration error; we would never offer auto-imports for a node_module that’s not listed in your package.json.
  • (3) Even if we discovered Dependant/node_modules/dependency_bug/package.json, its main field points to index.js which does not exist. The file you wanted to import was called soSomething.js, which can’t be found from its package.json file. This is probably part user error and part design limitation for us. It would be nice if we could just scan the whole directory for files to auto import, but that can get really expensive really fast. Maybe it would be reasonable to do when we discover that it’s a symlink to a local project, but this would actually be a pretty big change.
  • (3) Even if the main field of the dependency package.json was set correctly, the auto-import provider currently only resolves type definition files, not JS files, even for inferred JS projects. This is the one small, easy fix for us.

So, this issue is not just a bug, it’s one bug, one or two feature requests, and one or two user config errors wrapped up in one report. I can fix the bug as part of TS 4.4, but (@DanielRosenwasser) it sounds like we need to have a larger discussion around how to be smarter about unconfigured JS projects, or at least how we can nudge the authors of those kinds of projects toward successful configurations.

@Muritavo Muritavo reopened this Jul 27, 2021
@Muritavo
Copy link
Author

Muritavo commented Jul 27, 2021

@andrewbranch you are right about the user configuration, I will update the example repo with the dependency listed on package.json.

Really, how would TS know that it's a dependency if it's only on the node_modules folder.
But this cenario occurs on other projects that do have the dependency on package.json, and I think that would be 99% of the cases.

I will look into the points you described, looking into our real use case scenario, and update this issue with what I discover.

For last, it really is a change that was made on a recent release from TS and not with VSCode I believe. Because I've set it to use a previous TS version, and the imports worked correctly again.

@andrewbranch
Copy link
Member

What version of TS works? It’s probably working by accident 😬

The other thing to point out is that if you already have another file that imports "dependency_bug/soSomething", that’s another way that TS can find that file and it should become available as an auto-import to other files in the project.

However, without having a jsconfig.json or tsconfig.json file anywhere in here, a lot of features are going to work very poorly. TS will only be aware of files that are currently open or imported by files that are currently open. In general, your experience will improve a lot if you add jsconfig.json files at the root of each project. That, combined with the config changes I mentioned, along with the bugfix I mentioned in my last bullet point, are enough to fix the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Domain: Auto-import Fix Available A PR has been opened for this issue
Projects
None yet
7 participants