-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
Rush's node_modules structure is not resolved in vscode correctly #28689
Comments
@mjbvz let us know if there's any help you need investigating this issue. Rush is pretty widely used internally at Microsoft, so if there is a problem with its symlinking strategy, we'd like to get that fixed. I'll point out that the symbolic links were recently converted to use relative paths. I wonder if that could be related... |
Seems I am getting this error in the TSServer output panel of vscode:
This correlates with the TS Server showing The tsserver-log contains the below around that time (before).
The |
Do I need to do anything special apart from cloning repo, opening file and typing the statement mentioned? I followed those steps (with typescript nightly build) but that seem to work as intended. Do not see any process shutdown or crash. Let me know if I need to do something else. If possible please share full tsserver log so we can see where things might be going south. Thanks. You can enable log level verbose in settigns and open and log and share tsserver and ti log or upload them using
|
To replicate, by following those steps you should have not have gotten the auto-import suggestion and no auto-fix bubble. Is this the case for you? The TS Server crashing is intermittent and harder to replicate. I'll be sure to use that command when it happens again, as |
Yes that's exactly what I saw. (Used typescript@next) |
Uploaded the logs when I noticed that the TS Server was hanging while trying to initialize (going on 5mins now!). VScode is basically unusable with TS:
|
@nfour Looking at the log it seems like there are lot of DirectoryWatchers being invoked. Many of them are through failed lookup locations of the sources in project. Is it possible to share the repro project? We can sign nda if needed. (send it to me in email) I would also like to discuss with you about the different directoryWatcher locations that I see and reason about them but I am not sure if your code is private so let me know if we can continue the discussion here on email thread. If email, please contact me at shkamat at microsoft dot com. Thanks. |
We're looking into whether we can share the source :). I can tell you the project structure is pretty much like this (which you could likely discern from the log outputs I uploaded): /services/
/myService/ // (depends on myLib)
/libraries/
/myLib/ // (depends on myLib2)
/myLib2/
rush.json As an alternative, do you think the |
@nfour it would be hard to figure out what might be going wrong just from logs. In past we have used source code and figured out if there is something user can do or something we can do to enhance the experience. But that comes from looking at the source code, what kind of things module resolution is looking at etc. So just source structure is not enough (since from the log it seemed module resolution played role in the directory watchers being set). Please let me know once you are ready with the repro code. Thanks. |
@sheetalkamat I think we'll be able to get you access to the source, will email! |
@sheetalkamat sent you an email requesting info to provide the NDA. |
@nfour Thanks for email. I am working on getting you the information. |
@sheetalkamat we seem to be having issues getting this NDA underway for various reasons. Ignoring that, has anyone made progress with this issue in other rush repos at Microsoft? We are considering a workflow which attempts to use rush for its useful tools and to use something else for linking & installing dependencies. |
@nfour No we haven't been able to make any progress on this since we are waiting on repro. Have you tried reducing the repro such that you can remove any NDA code and yet reproduce the issue. |
@sheetalkamat working on that now - going to try converting our codebase to use That may prove that rush's If you could send the NDA again to my email address that we conversed on I can pass it on to the right people. |
thanks! feels a bit confusig all the different configurations about how to load modules. |
Any update ? Quick update: Webstorm solve correctly import :) |
I'm sorry, but we have a concrete repo to replicate the issue with, have we not ? The issue is reproductible on Microsoft own web tools : https://github.com/Microsoft/web-build-tools We are trying to switch to Rush + VsCode (both MS products) and we encountered this issue, which is pretty much a showstopper. Anyway, here is a really small repo to reproduce the issue : https://github.com/RDeluxe/rush-repro-955 Simply go to libraries/dtos/src/dtos/user.ts You will have a compiler error. Normally vscode should be able to auto import the missing decorators from class-validator
Or to autocomplete and autoimport while typing |
Hello, any news ? Could you reproduce on your end ? Thanks ! |
I'm sorry to bump this again, is there anything I could do ? I'm not 100% confident jumping in Typescript's codebase, but if somebody could point me in the right direction I may be able to help. |
Anyone have any workarounds here? Ideally something that doesn't require circumventing the symlinks. I'm still unable to get autocomplete working with a project using Rush. |
Any news about this issue ? A small repro is here: https://github.com/RDeluxe/rush-repro-955 Any project with rush + ts look like having the issue |
The repro provided has nothing to do with rush..
|
Hey ! sorry for bothering you @sheetalkamat :( I made a better repro here: https://github.com/totomakers/rush-example Maybe i can give you better information for helping you.
Normally, VScode provide me help for finding import, like the decorator But with rush nothing happen, i (hope) is because of the Issue was provide in Rush, VSCode and Typescript Maybe i miss the point :'( sorry and thanks for help |
Hello @sheetalkamat ! Thanks for your time. To be thorough I updated my repo https://github.com/RDeluxe/rush-repro-955 The README should explain everything. There are 3 projects there:
|
@RDeluxe Thank you for repro. we will investigate this. |
Very excited for this to get some attention @sheetalkamat! This issue has been open for a lonnnng time. |
Good news.. i have fix prototype available.. need to polish it and write some tests.. But i will get PR up soon. |
…r that isnt node_modules Most of the monorepo like scenarios are like this so looking at symlink to decide if file can be imported is essential Fixes #28689
…er that isnt node_modules (#37387) * Add tests that fail because of symlink to non common directory node_modules * When the imported module is through node_modules and symlink to folder that isnt node_modules Most of the monorepo like scenarios are like this so looking at symlink to decide if file can be imported is essential Fixes #28689
still have this problem...
|
I set |
For those still searching, make sure that you have vscode installed with the WSL plugin. You must be sure that the WSL backend is used! |
When trying to auto-import node modules within a
rush
project, intellisense will not locate dependencies. This also results in the editor crashing or re-initializing the TS server somewhat often.This is likely due to rush's unique node module structure, and it's unclear if this ever worked as I have only adopted rush somewhat recently.
Steps to Reproduce:
You can replicate this issue within the
web-build-tools
repo itself.Colors
in the top level (from the colors module). You shouldn't be able to resolve the symbol.eg.
This also seems to correlate with the editor's TS server crashing intermittently, but its not clear how to reproduce.
Does this issue occur when all extensions are disabled?: Yes
Versions tested:
Workaround
If you execute the below script to replace symlinks with real files:
Everything should work as expected.
There seems to be an issue with symlinks.
Related: microsoft/rushstack#955
The text was updated successfully, but these errors were encountered: