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

Fix #677, possible approach for transitive deps during hoist #692

Closed

Conversation

jppurcell9
Copy link

Here's a possible approach to fixing #677. Instead of installing the leaf dependencies directly, we install them in a "shadow" directory package-name/node_modules/_node_modules. Once all the leaves for package-name are installed, we go back and move each dep from package-name/node_modules/_node_modules/dep to package-name/node_modules/dep and symlink the shadow modules back into dep (package-name/node_modules/dep/node_modules -> package-name/node_modules/_node_modules). This approach should work for both npm and yarn clients.

This isn't ready to be merged - tests are failing and more complex cases need to be addressed (version collisions in the flattened shadow dir, for example). Just wanted to get some feedback on the approach to see if this is worth pursuing.

@evocateur
Copy link
Member

I appreciate the proof-of-concept!

However, it looks suuuuper-complicated, and I'd rather not engage in too much hackery (aside from our current yarn bait-n-switch) with npm clients while work is underway to specify "native" behaviour that should solve this (yarn is coordinating with npm folks, too).

In the mean time, I'd be much happier with a solution that simply used npm install --global-style (regardless of npmClient option) to deal with non-hoisted package dependencies. It hits the sweet spot of "already tested elsewhere" and "acceptable compromise of functionality", for me.

@jppurcell9
Copy link
Author

Agreed - and getting this right (not just this proof of concept) is going to be even more complicated. I'll close this and try again with --global-style.

@jppurcell9 jppurcell9 closed this Mar 20, 2017
@jppurcell9 jppurcell9 deleted the feature/hoist-vs-transitive-deps branch March 20, 2017 17:46
@lock
Copy link

lock bot commented Dec 27, 2018

This thread has been automatically locked because there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Dec 27, 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

Successfully merging this pull request may close these issues.

2 participants