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

Chained module resolution during npm install #6424

Closed
erykwarren opened this issue Jan 9, 2016 · 6 comments
Closed

Chained module resolution during npm install #6424

erykwarren opened this issue Jan 9, 2016 · 6 comments

Comments

@erykwarren
Copy link

(related to #2338)

Problem

Install a TS module A, itself depending on another TS module B. npm flattens the dependency structure under node_modules and therefore the resolution algorithm implemented by #2338 is broken when module A is built.

How to repro

  1. clone https://github.com/erykwarren/ts-6424-repro
  2. npm install

The project includes dependency on module ts-lib2, which itself depends on ts-lib1. If you clone ts-lib2 by itself, and run npm install on it, it works. It's only when there's a chained dependency (with npm flattening the modules) that the problem occurs.

Assumptions

Each project is built as an npm postinstall step. I couldn't find out if there is a recommended way to accomplish this differently.

@vladima
Copy link
Contributor

vladima commented Jan 9, 2016

it looks like this issue is fixed in out latest bits, can you give a try to our nightly build (typescript@next) to see if you still can see this problem.

@erykwarren
Copy link
Author

It works very nicely. Thanks @vladima.

@erykwarren
Copy link
Author

@vladima Is there anyway to port this fix in 1.7? If we have to wait for 1.8, what's the workaround?

@erykwarren erykwarren reopened this Jan 11, 2016
@vladima
Copy link
Contributor

vladima commented Jan 11, 2016

this issue was fixed in two PRs: #5275 and #5461. I've ported them into release-1.7 in #6441, pinging @mhegazy for his decision if it is safe to take these changes.

@erykwarren
Copy link
Author

@vladima Thanks for pushing this. However, I tried building a custom TS with the patch in 6441, and it didn't solve the issue....

@erykwarren
Copy link
Author

@vladima Forget what I wrote, my test was wrong. It works! Thanks a lot.

@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
None yet
Projects
None yet
Development

No branches or pull requests

2 participants