Skip to content
This repository has been archived by the owner on Aug 11, 2022. It is now read-only.

local private module dependencies #2442

Closed
tj opened this issue May 10, 2012 · 49 comments
Closed

local private module dependencies #2442

tj opened this issue May 10, 2012 · 49 comments
Milestone

Comments

@tj
Copy link
Contributor

tj commented May 10, 2012

hey hey, looking for your thoughts or suggestions on this issue. What I wanted ideally is to modularize our app(s) using vanilla node_modules, however they are not in any registry so they're checked into the repo. $ npm install see's that they're already installed and ignores them, even if their own dependencies might not be installed. I have bigger plans for this so ideally avoiding a shell script to iterate and $ cd node_modules/pkg && npm install if possible.

Any suggestions for this use-case? maybe if "private": true is specified (or similar) npm could check the deps.

@mmalecki
Copy link
Contributor

I think npm should just iterate over bundledDependencies and run npm install in them.

@tj
Copy link
Contributor Author

tj commented May 10, 2012

yeah that would be great, if it makes sense to isaac, I might be missing something

@Mithgol
Copy link

Mithgol commented May 11, 2012

Running npm install on dependencies would probably fail in a somewhat important use case where a bundled dependency contains a pre-compiled module so that npm install of parent module _just works_ (and does not require Python and Visual Studio on Windows to build that one dependency).

In any case a bundled dependency is probably bundled for the very reason of avoiding simple npm install — otherwise the author of the parent module would make it unbundled (and would let npm install it).

In your own use case you should have not only bundled your unpublished modules to the parent module's npm package — that's not enough; you should also have copied and added children's dependencies to the parent module's package.json. (I believe you may accomplish it unless their dependencies are conflicting.)

Then npm install will install those dependencies (to the parent's node_modules folder instead of children's), and then Node.js will scan parent's node_modules folder immediately after children's folders — and there it'll find all the dependencies necessary for its children.

Problem solved.

@aseemk
Copy link

aseemk commented May 11, 2012

Definitely +1 to this issue. FYI that this is probably covered through a combination of #1341 and #1558.

@Mithgol:

[…] you should also have copied and added children's dependencies to the parent module's package.json. (I believe you may accomplish it unless their dependencies are conflicting.)

I don't believe this is a recommended practice. It goes against the whole purpose of modularity -- you'll have to keep your own package.json in sync with the module's. And indeed, this won't support conflicting dependencies, which was the reasoning behind npm 1.0's move to local node_modules.

@tj
Copy link
Contributor Author

tj commented May 11, 2012

yeah.. that wont really help me here, that's like going back to git submodules

@tj
Copy link
Contributor Author

tj commented May 21, 2012

@isaacs this is the thing I was talking about at the conf. I'll still try the git repo route but it's a little limiting since we're using github to host the repos and we're already at our private repo limit

@aseemk
Copy link

aseemk commented May 21, 2012

Not sure what the git repo route is exactly, but if it's specifying your deps as git URLs, we tried that too and found that a bit inflexible to use -- e.g. in staging, you want to use the staging branch while in production you want to use the production branch.

@tj
Copy link
Contributor Author

tj commented May 28, 2012

the biggest problem for us with the git repo thing is just the amount of private repos we would need. The solution @mmalecki mentioned sounds reasonable to me but I know isaac mentioned those are strictly untouched by npm by design so maybe we need something else

@isaacs
Copy link
Contributor

isaacs commented Jun 4, 2012

npm should definitely do this.

At the moment, bundledDependencies are just ignored. It would be best if npm skipped the unpack step, but still traversed into the child folders to see what needs to be done.

It wouldn't be terribly hard to do.

@tj
Copy link
Contributor Author

tj commented Jul 23, 2012

haha this is our clean install time since it has to loop each dep:

real    3m45.342s
user    1m14.221s
sys 0m14.632s

@tj
Copy link
Contributor Author

tj commented Aug 14, 2012

with all the duplicates we have over 12,000 require()s now and booting takes about 5s hahaha :D

@isaacs
Copy link
Contributor

isaacs commented Aug 21, 2012

@visionmedia You should try running npm dedupe on your tree. Should make your startup time a bit faster.

@tj
Copy link
Contributor Author

tj commented Aug 22, 2012

k cool I will take a look at that

@therealplato
Copy link

+1

9 similar comments
@jerryorr
Copy link

+1

@mkuklis
Copy link

mkuklis commented Oct 3, 2012

+1

@isaacs
Copy link
Contributor

isaacs commented Oct 4, 2012

+1

@tommedema
Copy link

+1

@tglines
Copy link

tglines commented Oct 18, 2012

+1

@justinwalsh
Copy link

+1

@icodeforlove
Copy link

+1

@andrewluetgers
Copy link

+1

@isaacs
Copy link
Contributor

isaacs commented Feb 26, 2013

+1

@mercmobily
Copy link

+1. I just got bitten by this one too, and I just couldn't figure out why dependencies weren't getting installed...!

@sidwood
Copy link

sidwood commented Apr 18, 2013

+1

3 similar comments
@vjpr
Copy link

vjpr commented May 1, 2013

+1

@isaacs
Copy link
Contributor

isaacs commented May 2, 2013

+1

@robertjd
Copy link

robertjd commented May 2, 2013

+1

@robertkowalski
Copy link
Contributor

ok, just to make sure I am getting this right:

npm install should traverse the bundledDependencies-Array and based on that entries it then tries to enter the folders of this entries and does a npm install inside them?

@Kingdutch
Copy link

@robertkowalski That seems to be the desired behaviour yes. (It's also what I expected when I added a bundledDependency and gave it a package.json like a good boy scout would and ran npm install on my base module)

@sethkinast
Copy link

Just got bitten by this as well, +1

@mercmobily
Copy link

I am writing a module that has twenty-five (!) bundleDependencies, each one with different dependencies. At the moment, I am making sure that all dependencies of each bundled module are added to the main package.js file of the main project. But... it's ugly. So, I confirm:

npm install should traverse the bundledDependencies-Array and based on that entries it then tries to enter the folders of this entries and does a npm install inside them"

Yes, absolutely, please.

@mikermcneil
Copy link

Currently, we're doing this for copying deps (which looks for subdeps and copies them, similar to @mercmobily's solution).

I've been struggling with the best way to do this as well-- was thinking maybe a script that makes a one-time-use package.json for the bundleds+the normal dependencies from the main module's package json? Temporarily replaces the existing package.json file, npm installs, then writes the original package.json back in place when it's done. That would allow us to take advantage of the dependency deduping already built into npm, and not have to worry about keeping anything rerolled in sync.

In case this helps anyone, here's a similar script that looks at a testDependencies key in the package.json file and npm installs them (in case you have a situation where you don't want devDependencies to bring in everything at once).

MarcDiethelm pushed a commit to MarcDiethelm/xtc-cli that referenced this issue Apr 2, 2014
@Philzen
Copy link

Philzen commented Apr 5, 2014

bump (Ouch!) 👍

@mercmobily
Copy link

@mikermcneil Ugh I wish I had used a script as well. Right now I did the "checking" work manually, and live with the thought that when/if this bug is fixed, I will have to go through everything and take out the redundant dependences from my main package...

@thomasfr
Copy link

thomasfr commented May 2, 2014

+1 :D

@mixxen
Copy link

mixxen commented May 7, 2014

+1

@tjwebb
Copy link

tjwebb commented Jun 20, 2014

was this ever resolved? I could be misunderstanding, but it seems because of this issue the behavior of bundledDependencies doesn't align with the documentation

@nkbt
Copy link

nkbt commented Jun 20, 2014

I don't think this will be ever (or soon) resolved. See above. I personally cannot see any automated solution to prevent indefinite recursive libs fetching. What could be done - some additional config variable with list of dependencies that must be fetched even if they exist in parents.

Just in case here is my recent talk on the topic http://nkbt.github.io/nnj with failing example and couple of ways to solve the issue.

@thomasfr
Copy link

Thanks @nkbt for your link!
I recently also tried some variations to organize local libs / modules whatever one might call it. I tried for instance some variations with setting NODE_PATH. I also tried of adding a node_modules/lib dir so i could do require("lib/dothisandalsothat.js (without relative file paths.). It all worked but it all felt a bit cumbersome or not right if we have npm at our hands.

How do others manage its local private node.js modules and libraries?
cheers

@othiym23
Copy link
Contributor

We recently landed a new feature that allows you to npm install --save / depend on file paths (i.e. "foo" : "file:../../foo-local"), which is basically what this issue is requesting. Give that a shot, and let's iterate from there. It took us a while, but we eventually we got around to this. Thanks to everyone for the ideas and suggestions.

@mercmobily
Copy link

Hi,

The documentation says:

"This feature is helpful for local offline development and creating tests
that require npm installing where you don't want to hit an external server,
but should not be used when publishing packages to the public registry."

That's not quite my use case. In my use case, I have Hotplate which in turn
contains, in stock, a number of modules (36 of them to be exact), each
one with a specific task and -- potentially -- with different requirements
in terms of sub-modules.

I am not sure what you are saying actually closes this ticket...?

Merc.

On 16 September 2014 11:58, Forrest L Norvell notifications@github.com
wrote:

We recently landed
https://github.com/npm/npm/blob/b706d637d5965dbf8f7ce07dc5c4bc80887f30d8/doc/files/package.json.md#local-paths
a new feature that allows you to npm install --save / depend on file
paths (i.e. "foo" : "file:../../foo-local"), which is basically what this
issue is requesting. Give that a shot, and let's iterate from there. It
took us a while, but we eventually we got around to this. Thanks to
everyone for the ideas and suggestions.


Reply to this email directly or view it on GitHub
#2442 (comment).

@thomasfr
Copy link

@othiym23 Nice addition. I will give that a try - looks good! :)

@nathanbowser
Copy link

@othiym23 This is so amazing. Thank you!

@othiym23
Copy link
Contributor

@mercmobily

There are two parts (at least) to this issue -- one is being able to effectively us local dependencies (which is what the issue is titled after), and another is ensuring that dependencies (including bundledDependencies) are fully installed after npm install runs. We've got the local dependencies piece of it covered, even if it doesn't work exactly the same way as described by @visionmedia / using bundledDependencies. The rest is something that can be addressed by the multi-stage install work that @iarna is doing, so I'll draw her attention to this discussion and we'll make sure that bundledDependencies are part of the scope of her work. There are many, many issues open around this, so because the biggest piece of this use case has been addressed, I'm leaving it closed (for now).

@mercmobily
Copy link

Thanks Forrest. Which one is the ticket that @iarna is working on, about the multi-stage install? I
just want to make sure my use case is at least considered!

@othiym23 othiym23 added this to the multi-stage install milestone Sep 16, 2014
@othiym23
Copy link
Contributor

@mercmobily

There isn't a specific tracking issue for your particular use case, so can you open a new issue describing it (as concretely as you're able)? The multi-stage install project is meant to rework the installation process so that it figures out exactly which dependencies are missing, and then installing all of them in a single pass. It seems entirely in scope for bundledDependencies to be included as part of that dependency tree realization, but an explicit use case (or maybe even… a failing test 😍) would be very useful.

@mercmobily
Copy link

Just wrote out a ticket, and then realised that it already existed...!

#1341

And that the multi-stage install should really fix it:

#5919

So, all good -- thanks!

@npm npm locked and limited conversation to collaborators Jun 24, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests