Bundled dependencies breaking shrinkwrap #313

Closed
eknkc opened this Issue Aug 30, 2012 · 11 comments

Comments

Projects
None yet
5 participants

eknkc commented Aug 30, 2012

Hi,

As far as I can tell, version 2.11.0 includes some "bundleDependencies" within the packages.json file. With this config, a clean installation of request causes invalid / extraneous dependencies. At this state, it's not possible to shrinkwrap the dependency list.

Can we just use regular dependencies?

BTW, there are sublime text project files in form-data folder, contains somewhat personal information of the owner.

Output:

$ mkdir requestTest
$ cd requestTest
$ npm install request

npm http GET https://registry.npmjs.org/request
npm http 304 https://registry.npmjs.org/request
request@2.11.0 node_modules/request

$ npm ls
npm WARN unmet dependency /path/node_modules/request/node_modules/form-data requires mime@'1.2.2' but will load
npm WARN unmet dependency /path/node_modules/request/node_modules/mime,
npm WARN unmet dependency which is version 1.2.7
/path
└─┬ request@2.11.0
  ├─┬ form-data@0.0.3 extraneous
  │ ├── async@0.1.9
  │ └─┬ combined-stream@0.0.3
  │   └── delayed-stream@0.0.5
  └── mime@1.2.7 invalid extraneous

zzen commented Aug 31, 2012

Same here. Filled out some more info at 1f21b17#commitcomment-1794229

chakrit commented Sep 1, 2012

I took the following steps and were able to shrinkwrap again:

cd node_modules/request
rm -Rf node_modules
cd ../..
// add latest form-data and mime to package.json
npm install
npm shrinkwrap // now succeeds

...

Owner

mikeal commented Sep 3, 2012

this is an npm bug. @isaacs

@mikeal mikeal closed this Sep 3, 2012

eknkc commented Sep 3, 2012

It is.

Just curious, what is the particular reason that bundling dependencies is better than regular npm dependencies here?

It might be that this way the package does not rely on npm registry at all. I could clone the repo without npm. Was that the goal?

zzen commented Sep 4, 2012

It is an npm bug @mikeal - and it's also been open for 7months - with no sign of a speedy fix. :(

I wonder what's so important about using bundleDependencies instead of regular dependencies to break the build for everybody. I now have to refer to a fork just to get my own package to build properly.

chakrit commented Sep 4, 2012

@zzen I think you can read more on his blog http://www.mikealrogers.com/posts/nodemodules-in-git.html

BTW, you can also lock version using npm shrinkwrap which doesn't cause the bug.

Much better approach IMO

zzen commented Sep 4, 2012

Thanks @chakrit both for the tip and the blog post. It was interesting and the problem is indeed very real.

Contributor

isaacs commented Sep 4, 2012

@mikeal You should reopen this issue.

There are three issues here. The first is a request issue, the second is an npm issue (which might be best left as-is), and the third is a form-data issue.

  1. Bundled dependencies still need to be regular dependencies (or optional, or dev), or else npm ls will see them as extraneous. Request's package.json file should declare which versions of form-data and mime are acceptable.
  2. I could of course add some logic to have bundledDependencies get added as whatever@* in the dependencies list if they're not found, but the reason for this is to nudge people towards actually adding them. This would put a bandaid over the issue, but I haven't worked out all the implications. At the very least, a npm dedupe might end up providing request with a version of mime that it doesn't actually work properly with, so it's hazardous to do so.
  3. form-data is ludicrously strict in its dependency declaration. See felixge/node-form-data#10

In other words "bundled dependencies breaking shrinkwrap" is working exactly as designed in this case. There are packages in node_modules that you and felix have told npm are extraneous and invalid, so it's correctly refusing to shrinkwrap the spoiled contents.

isaacs added a commit to isaacs/request that referenced this issue Sep 4, 2012

Owner

mikeal commented Sep 4, 2012

can we make those changes you made a pull request?

i think it's kind of funny that while people are wondering "why are you bundling" we're having to make changes to the deps that get bundled with our version :)

@mikeal mikeal reopened this Sep 4, 2012

Contributor

isaacs commented Sep 4, 2012

Yeah, the changes in that pull req should fix request's part in this little snafu. But really, form-data should be more liberal, and I'm not very happy with npm's handling of this situation. The naive approach won't work for various reasons, but it ought to just accept that if you've bundled something, you know what you doing.

mikeal added a commit that referenced this issue Sep 4, 2012

Owner

mikeal commented Aug 27, 2014

Is this still an issue?

This is so old I'm closing, if it is actually still an issue just let me know and I'll re-open.

@mikeal mikeal closed this Aug 27, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment