npm prune --production and shared deps #4509

Closed
davglass opened this Issue Jan 17, 2014 · 9 comments

Comments

Projects
None yet
5 participants
Contributor

davglass commented Jan 17, 2014

Take this package.json as an example:

{
    "name": "foo",
    "version": "1.0.0",
    "dependencies": {
        "facebook-test-users": "*"
    },
    "devDependencies": {
        "request": "2.21.0"
    }
}
$ npm install
$ npm ls

foo@1.0.0 /tmp
├── facebook-test-users@0.0.4
└─┬ request@2.21.0
  ├── aws-sign@0.3.0
  ├── cookie-jar@0.3.0
  ├── forever-agent@0.5.0
  ├─┬ form-data@0.0.8
  │ ├── async@0.2.9
  │ └─┬ combined-stream@0.0.4
  │   └── delayed-stream@0.0.5
  ├─┬ hawk@0.13.1
  │ ├─┬ boom@0.4.2
  │ │ └── hoek@0.9.1
  │ ├── cryptiles@0.2.2
  │ ├── hoek@0.8.5
  │ └─┬ sntp@0.2.4
  │   └── hoek@0.9.1
  ├─┬ http-signature@0.9.11
  │ ├── asn1@0.1.11
  │ ├── assert-plus@0.1.2
  │ └── ctype@0.5.2
  ├── json-stringify-safe@4.0.0
  ├── mime@1.2.11
  ├── node-uuid@1.4.1
  ├── oauth-sign@0.3.0
  ├── qs@0.6.6
  └── tunnel-agent@0.3.0


$ npm prune --production
unbuild request@2.21.0

$npm ls
foo@1.0.0 /tmp
└─┬ facebook-test-users@0.0.4
  └── UNMET DEPENDENCY request ~2.21.0

npm ERR! missing: request@~2.21.0, required by facebook-test-users@0.0.4
npm ERR! not ok code 0

The logic here doesn't seem right to me, I know it's removing devDeps but since that devDep was also a real dep it was installed higher in the tree. Now pruning breaks my deps.

Owner

isaacs commented Jan 19, 2014

Yes, this is a bug. It should not be removing anything that's depended on anywhere.

@isaacs isaacs added a commit that referenced this issue Jan 19, 2014

@isaacs isaacs Revert "prune: --production option to unbuild devDependencies"
This reverts commit f65da7d.

See #4509
1101b6a
Owner

isaacs commented Jan 19, 2014

Bug introduced in f65da7d. Sorry about that. It's doing the wrong thing. Rather than assume that any devDependency is necessarily extraneous at the end of the walk, it needs to remove them from the dependencies hash at the start of the walk.

dev/production needs to be passed to read-package-json so that it knows whether or not to merge devDependencies into dependencies in the first place. Reverted.

Member

domenic commented Jan 19, 2014

To be fair, npm prune --production didn't even exist before that commit :P.

Owner

isaacs commented Jan 20, 2014

It did though.

npm config set production=true, and then npm prune would leave devDeps around, yes, which sucks. But it wouldn't remove devdeps that were actual deps of some other thing deeper in the tree.

chadly commented Jan 29, 2014

So is npm prune --production coming back or is this feature gone for good?

Contributor

davglass commented Feb 3, 2014

+1 for getting this back, but working :)

Contributor

davglass commented Feb 12, 2014

After investigating this a but more (I really, really need this back) I think I have the way to fix this, but need @izs to verify.

I "think" the "right" way to fix this is in read-installed and having a flag on it to ignore devDependencies.

If I manually remove these two lines:
https://github.com/npm/read-installed/blob/master/read-installed.js#L189
https://github.com/npm/read-installed/blob/master/read-installed.js#L201

Then have this package.json

{
    "name": "foo",
    "version": "1.0.0",
    "dependencies": {
        "facebook-test-users": "*"
    },
    "devDependencies": {
        "request": "2.21.0",
        "vows": "*",
        "istanbul": "*"
    }
}

Then call npm prune, it does this:

node ../npm/bin/npm-cli.js prune
npm info it worked if it ends with ok
unbuild vows@0.7.0
unbuild istanbul@0.2.4
npm info ok 

So, making devDependencies being optional in read-installed they are basically considered extraneous and prune will drop them.

This appears to be the proper functionality, thoughts?

Owner

isaacs commented Feb 12, 2014

@davglass Yeah, I think those are the two lines that need to be removed, but only conditionally when you pass some kind of {production: true} option, or whatever.

fjacq commented Feb 12, 2014

The easiest fix I found since npm prune --production don't work (it works like npm prune) in npm v1.3.24 is the following:

rm -rf node_modules
npm install --production

@davglass davglass added a commit to davglass/npm that referenced this issue Feb 15, 2014

@davglass davglass prune: Added back --production support - fixes #4509 6b0db5a

isaacs closed this in acc4d02 Feb 15, 2014

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