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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Npm3 problems #28

Closed
SimenB opened this issue Feb 19, 2016 · 13 comments 路 Fixed by dependents/node-filing-cabinet#5
Closed

Npm3 problems #28

SimenB opened this issue Feb 19, 2016 · 13 comments 路 Fixed by dependents/node-filing-cabinet#5

Comments

@SimenB
Copy link

SimenB commented Feb 19, 2016

Bad title, but I think that's the reason. 馃槅

Let's see if I can explain what (I think) is happening:
We have a dependency on yargs, which depends on yargs-parser, which uses lodash.assgin@4. We also use babel-eslint, which uses lodash.assign@3. lodash.assign@4 has dependencies on lodash.rest and lodash.keys, while lodash.assign@3 has dependencies on lodash._baseassign, lodash._createassigner and lodash.keys.

npm@3 puts lodash.assign@3 and lodash.rest in my projects node_modules, but keeps lodash.assign@4 in yargs-parser's node_modules.

Now, the problem:
dependency-tree references the lodash.assign@3 in my project's node_modules, instead of the right one for yargs-parser.

$ npm ls lodash.assign -p
/home/testyo/repos/frontend-repo/code/node_modules/lodash.assign
/home/testyo/repos/frontend-repo/code/node_modules/yargs-parser/node_modules/lodash.assign

image

I think you have an optimization which says "oh, lodash.assign is already added, moving on", instead of handling the fact that multiple versions are installed.

Do you understand the error, or should I try to create a reproducible sample?

@mrjoelkemp
Copy link
Collaborator

Thanks for reporting. I don't recall having any optimizations in place for that dependency. Do you know what would fix this issue?

@SimenB
Copy link
Author

SimenB commented Feb 19, 2016

Well, yargs-parser depends on /home/testyo/repos/frontend-repo/code/node_modules/yargs-parser/node_modules/lodash.assign not on /home/testyo/repos/frontend-repo/code/node_modules/lodash.assign like it currently resolves to.

Where is the resolution logic, maybe I could take a look?

@SimenB
Copy link
Author

SimenB commented Feb 19, 2016

Maybe use https://github.com/substack/node-resolve which allows us to set a basedir? If you currently use require.resovle it might work

@mrjoelkemp
Copy link
Collaborator

The dependency-tree module doesn't use lodash.assign directly. I can't find its usage in any of the dependencies either.

Does your shrinkwrap fail? Or is it that the deduping isn't as good as it could be?

Also, how did you gather that dependency-tree is involved?

@SimenB
Copy link
Author

SimenB commented Feb 19, 2016

Yeah, that's it. Digging down into filing-cabinet, require.resolve is used, ignoring where the require comes from. So nested dependencies will never resolve correctly

@mrjoelkemp
Copy link
Collaborator

I'm open to using node-resolve instead if that'll fix the issue. Sadly, passing along a baseDir will involve having both dependency-tree and filing-cabinet support the option. I'm happy to review PRs for whatever is necessary, but I don't have the availability to fix this issue myself.

Is there an easy workaround?

@SimenB
Copy link
Author

SimenB commented Feb 19, 2016

@mrjoelkemp
Copy link
Collaborator

Try npm update dependency-tree to get the recent patch release for filing-cabinet and let me know if that works for you.

@mrjoelkemp
Copy link
Collaborator

@SimenB may I ask, what are you using dependency-tree for? Curious as to its applications outside of the stuff I've written.

@SimenB
Copy link
Author

SimenB commented Feb 19, 2016

It does work now, thanks!

@SimenB
Copy link
Author

SimenB commented Feb 19, 2016

To answer your question, we currently have a mock version of our Spring app running on express, which is used for front end development. But the same version is also bundled up for usage as a demo from a memory stick. When we bundle it, we want to avoid adding development dependencies to the tar ball, so we use dependency-tree to get only the actual dependencies and zipping it up in one file.

@mrjoelkemp
Copy link
Collaborator

@SimenB that sounds pretty cool. Might be interesting to have http://rollupjs.org/ use dependency-tree to support commonjs and amd apps.

@SimenB
Copy link
Author

SimenB commented Feb 19, 2016

We tried webpacking it, but native dependencies messed up when in a single file (logically, I suppose). But dependency-tree works good for us, just using archiver to add the deps we get, just deduping them and messing with the strings to get relative pathing, to a zip.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants