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

[BUG] --production installs devDependencies if the devDependency has a shrinkwrap #1113

Closed
marcosloic opened this issue Apr 6, 2020 · 8 comments
Labels
Bug thing that needs fixing

Comments

@marcosloic
Copy link

What / Why

If a project has a devDependency that itself has a shrinkwrap, then the devDependency's dependencies will be installed

When

  • n/a

How

npm install --only=prod

Current Behavior

devDependencies get installed

Steps to Reproduce

  • npm init
  • npm install polymer-cli --save-dev
  • rm -rf node_modules
  • npm install --only=prod

Expected Behavior

polymer-cli is a devDependency, as such nothing should be installed

This issue has been spotted because polymer-cli has vulnerable dependencies spotted by retirejs. The --production (or --only=prod) seems to work if the devDependency does not have a shrinkwrap file. npm ci --production works as expected and bypasses the devDependencies entirely

@bmacher
Copy link

bmacher commented Apr 9, 2020

I am facing the same problem with eslint-plugin-html. As a workaround im using uninstill to install the dependencies.

npm uninstall eslint-plugin-html --production

@mhart
Copy link
Contributor

mhart commented Oct 2, 2020

I'm seeing similar behavior though I don't think it's been tracked down to a shrinkwrap file. Eg, just installing mocha as a dev dependency:

$ rm -rf *.json node_modules && \
  npm init -y && \
  npm install -D mocha --no-audit && \
  rm -rf node_modules && \
  npm install --production --no-audit && \
  ls node_modules | wc -l

14

I can see that package-lock.json does indeed omit "dev": true for all of the modules that appear in node_modules.

However, if I manually specify one of mocha's dependencies, es-abstract, to be installed as a dev dependency, and then uninstall it, it works:

$ rm -rf *.json node_modules && \
  npm init -y && \
  npm install -D mocha es-abstract --no-audit && \
  npm rm es-abstract --no-audit && \
  rm -rf node_modules && \
  npm install --production --no-audit && \
  ls node_modules | wc -l

ls: node_modules: No such file or directory
       0

@boneskull
Copy link
Contributor

boneskull commented Oct 12, 2020

ref: mochajs/mocha#4474

there's an example repo; in the mocha-2020-10-bug folder there's a package.json containing only mocha as a devDependency. upon npm install --production, I expect node_modules to be empty. but...

$ npm i --production
npm WARN [email protected] No description
npm WARN [email protected] No repository field.

added 16 packages from 12 contributors and audited 135 packages in 1.128s

12 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities

$ ls node_modules
define-properties          has                        is-regex                   string.prototype.trimend
es-abstract                has-symbols                is-symbol                  string.prototype.trimstart
es-to-primitive            is-callable                object-inspect
function-bind              is-date-object             object-keys

This is weird indeed. I note that the package-lock.json file in this directory looks to be missing dev: true for each of these entries.

Running yarn install --production results in an empty node_modules, as expected.

Also note that blasting package-lock.json and running npm install --production has no effect; the package-lock.json file is recreated the same way.

@boneskull
Copy link
Contributor

@ruyadorno have you seen this before?

@ljharb
Copy link
Contributor

ljharb commented Oct 12, 2020

I believe all of those are deps of es-abstract, so the implication is that that's what's pulling them all in. It seems like the lockfile does not annotate it as a dev dep.

@boneskull
Copy link
Contributor

es-abstract likely arrives through Mocha's inclusion of promise.allsettled, which is a production dependency of Mocha.

@ruyadorno
Copy link
Contributor

Thanks for the heads up @boneskull and no 😅 never seen that before! I just gave it a try locally to the example repo you linked above and I can also confirm it does not work as expect in npm6 (indeed it installs devDeps when using --production) but the good news is that the problem is fixed in npm7! (also tried it with the same example repo)

I don't believe we are going to patch npm6 moving forward other than providing security releases and small fixes from the community contributions that happens to have a very active champion. With that in mind and given that [email protected] fixes both the mocha issue described and the original issue posted by @marcosloic (also tested the polimer-cli repro and I can confirm in npm7 it works as expected when using --only=prod or --production) I would advise upgrading to npm7 as the correct solution to this problem 😊 :

npm install -g npm@7

Thanks @marcosloic for the original report and @mhart and @boneskull for the help debugging it, let me know if there's anything else I can help with 😄

@darcyclarke darcyclarke added the Bug thing that needs fixing label Oct 30, 2020
@darcyclarke
Copy link
Contributor

npm v6 is no longer in active development; We will continue to push security releases to v6 at our team's discretion as-per our Support Policy.

If your bug is preproducible on v7, please re-file this issue using our new issue template.

If your issue was a feature request, please consider opening a new RRFC or RFC. If your issue was a question or other idea that was not CLI-specific, consider opening a discussion on our feedback repo

Closing: This is an automated message.

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

No branches or pull requests

7 participants