Skip to content

Shared external module causes compiler to lockup #5695

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

Closed
david-driscoll opened this issue Nov 16, 2015 · 10 comments
Closed

Shared external module causes compiler to lockup #5695

david-driscoll opened this issue Nov 16, 2015 · 10 comments
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue

Comments

@david-driscoll
Copy link

I have a very strange issue that I've finally figured out the cause of, as it's really very strange.

It appears that if there is a dependency, that exists in two locations, and it defines typings as external, then tsc gets confused and cannot reason about them both.

Setup

  • Using atom-typescript as my editor, though the issue shows up when trying to compile with tsc as well.
  • I'm on a windows machine.

My folder structure looks like:

  • c:\Dev\Omnisharp\omnisharp-atom
    • \node_modules\omnisharp-client symlink to c:\Dev\Omnisharp\omnisharp-node-client
    • \node_modules\@reactive\rxjs symlink to c:\Dev\reactivex\RxJS
  • c:\Dev\Omnisharp\omnisharp-node-client
    • \node_modules\@reactive\rxjs symlink to c:\Dev\reactivex\RxJS
  • c:\Dev\reactivex\RxJS

Everything is typically working great, I'm working on typings for RxJS 5.0, and updating omnisharp-client without issue.

When I got to omnisharp-atom an issue revealed itself, if I define Observable<T> as a return type for a method, with the above setup, then tsc get's confused and fails to compile. It gets into some sort of infinite loop.

Working

If I update things so that they look like:

  • c:\Dev\Omnisharp\omnisharp-atom
    • \node_modules\omnisharp-client (no symlink)
      • \node_modules\@reactive\rxjs (removed)
    • \node_modules\@reactive\rxjs symlink to c:\Dev\reactivex\RxJS

Then everything is happy and works as expected.

Without symlinks

If I update things so that they look like:

  • c:\Dev\Omnisharp\omnisharp-atom
    • \node_modules\omnisharp-client (no symlink)
      • \node_modules\@reactive\rxjs (symlink or non-symlink)
    • \node_modules\@reactive\rxjs symlink to c:\Dev\reactivex\RxJS

Then tsc fails to operate and will sit and consume CPU forever until it is killed.

@mhegazy
Copy link
Contributor

mhegazy commented Nov 16, 2015

@vladima can you take a look

@vladima
Copy link
Contributor

vladima commented Nov 17, 2015

@david-driscoll I've tried to repro it but with no luck however this sounds like a serious issue. Can you please share a self contained repro so we can address it properly?

@david-driscoll
Copy link
Author

Sure, I can recreate it on demand with my current solution, so I'll try zipping it up and double checking it still fails, then post a link.

@vladima
Copy link
Contributor

vladima commented Nov 17, 2015

thanks!

@david-driscoll
Copy link
Author

So creating a zip failed to extract if you're on windows (like myself) because of long paths.

I've created a batch file that will setup the scenario, of course feel free to review it for an malicious code. It's just pulling down a few git repos, running npm install for those repos and setting up the same symlinks.

https://onedrive.live.com/redir?resid=B65E6B05AE4EE402!237452&authkey=!AG6bNj8i96iBJJQ&ithint=file%2czip

@vladima
Copy link
Contributor

vladima commented Nov 17, 2015

I can see what is going on. Good news is that is not not related to module resolution - all files are resolved without any issues. The issue is that from compiler perspective it sees two different set of typings for RxJS: one in omnisharp-atom folder and another in omnisharp-node-client and all the time pretty much spent trying to relate types from these typing sets

@david-driscoll
Copy link
Author

@vladima any traction on this issue, still seems to be a problem. Any proposed work arounds perhaps?

@RyanCavanaugh RyanCavanaugh added the Bug A bug in TypeScript label Feb 16, 2016
@mhegazy mhegazy added this to the TypeScript 1.8.2 milestone Feb 16, 2016
@vladima
Copy link
Contributor

vladima commented Feb 16, 2016

@david-driscoll TypeScript compiler currently does not follow symlinks so in your scenario it views RxJS in omnisharp-node-client\node_modules and omnisharp-atom\node_modules as different set of files and this leads to the worst possible option during type relationship check - comparing two identical structures. We have an issue to handle symlinks properly (#6365), once it is done - thinks should look better. As a workaround in your case can you push RxJS dependency higher in the directory hierarchy so compiler will use the same package for both omnisharp-node-client and omnisharp-atom?

├───node_modules
│   └───rx (symlink from rx-js)
├───omnisharp-atom
├───omnisharp-node-client
└───rx-js

@kruncher
Copy link

kruncher commented Mar 8, 2016

I am having the exact same issue with rxjs and use of symlinks.

It doesn't like it when I symlink the core package into my plugin package because rxjs occurs in both paths standard-generator/node_modules/core-package/node_modules/rxjs and standard-generator/node_modules/rxjs.

To me it seems that TypeScript could just look in standard-generator/node_modules for rxjs first, before attempting to look inside the deeper node_modules folders.

@vladima
Copy link
Contributor

vladima commented May 6, 2016

#8486 have added ability to follow symlinks during module resolution . Script this change should already be available in nightly builds, managed part and version of tsc.exe that support symlinks will be included in the next release

@vladima vladima closed this as completed May 6, 2016
@vladima vladima added the Fixed A PR has been merged for this issue label May 6, 2016
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue
Projects
None yet
Development

No branches or pull requests

5 participants