Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Added support for install --development #4791

Open
wants to merge 2 commits into from

5 participants

@davglass

This basically installs the packages that npm prune --production would remove.

Use case:

In a CI environment, I need to:

  • install production dependencies
  • analyze them and their size
  • display any warnings or helpful information
  • install the devDependencies
  • run their tests
  • prune --production
  • package for deployment

Now, I could do a --production install, then install without but I actually want to "switch" registries in between these. I only want "production" dependencies installed from a private local registry, but devDependencies can come from anywhere as they will be deleted with a prune. If a production dependency isn't in my local registry, it should 404 but doing a two pass install without scoping it would let it install anyway and then prune wouldn't work as I would expect.

I couldn't get all the tests to run on any of my dev machines from master or this branch, but I did add 7 passing tests for this functionality:

 ./node_modules/.bin/tap ./test/tap/install-development.js ./test/tap/install-production.js 
ok ./test/tap/install-development.js .................... 4/4
ok ./test/tap/install-production.js ..................... 3/3
total ................................................... 7/7
ok
@domenic
Collaborator

This seems very niche, and adding a --development flag that is different from the --dev flag seems bad. So, I am -1...

@davglass

Well, --dev does way more than it actually should.

I went with --development to simulate NODE_ENV and it effectively "undoes" what npm prune --production does. That, and there is currently no other way to install prod deps and dev deps separately.

I talked to @isaacs about this today and he mentioned that he would possibly rather just fix --dev to only go one level down.

@davglass

Ping @isaacs, would you rather do this or have me fix --dev to only do one level down?

@isaacs
Owner

We should definitely not have --dev and --development, since that's confusing, but I'm not happy with what --dev does anyway, and neither are most users. (Though I know that @raynos and some others like it, so removal isn't trivial or an obvious win.)

Why doesn't this work?

rm -rf node_modules # start fresh
npm install --production --registry=https://npm.yahoo.com/
# analyze etc
npm install # grab missing dev deps
npm test
npm prune --production
@davglass

The issue here is my whitelist, if a production dep is not in the whitelist it will 404 on the first install. When the second install happens it will install anyway, then prune --production won't remove it since it's already a installed and it's in the tree already.

What if we added a depth to --dev? If it's missing it's assumed it's top level only.

@domenic
Collaborator

What if we added a depth to --dev? If it's missing it's assumed it's top level only.

This sounds nice to me although I have not thought out how it composes with other uses of --depth etc. throughout the CLI.

@davglass

I tried simply adding a --depth 1 and it get's pretty hairy.

The other issue with that, is adding --dev still installs the the regular dependencies too. Which defeats the purpose of only installing the devDependencies.

@othiym23 othiym23 self-assigned this
@othiym23
Owner

I'm +1 on this, and part of the reason I like it is because it allows us to continue deprecating the giant footgun that is npm install --dev.

@othiym23
Owner

Needs to run top-level lifecycle scripts as well.

@othiym23
Owner

This needs to happen soonish for @davglass-related reasons.

@othiym23 othiym23 added this to the 2.2.0 milestone
@othiym23 othiym23 added the next-minor label
@othiym23 othiym23 removed this from the 2.2.0 milestone
@bengl

How about --only [dependencies|devDependencies|...]?

(Yeah it duplicates --production, but that could be deprecated in the future.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
6 lib/install.js
@@ -249,6 +249,12 @@ function readDependencies (context, where, opts, cb) {
if (er && er.code === "ENOENT") er.code = "ENOPACKAGEJSON"
if (er) return cb(er)
+ if (npm.config.get("development")) {
+ data.dependencies = {}
+ //Reset after the first pass so devDependencies have dependencies
+ npm.config.set("development", false)
+ }
+
if (opts && opts.dev) {
if (!data.dependencies) data.dependencies = {}
Object.keys(data.devDependencies || {}).forEach(function (k) {
View
39 test/tap/install-development.js
@@ -0,0 +1,39 @@
+var common = require('../common-tap.js')
+var test = require('tap').test
+var npm = require('../../')
+var osenv = require('osenv')
+var path = require('path')
+var fs = require('fs')
+var rimraf = require('rimraf')
+var mkdirp = require('mkdirp')
+var pkg = path.join(__dirname, 'install-development')
+//shared with ./install-production.js
+
+test("setup", function (t) {
+ rimraf.sync(path.resolve(pkg, 'node_modules'))
+ process.chdir(pkg)
+ t.end()
+})
+
+test('"npm install . --development" should install only devDependencies', function(t) {
+ npm.load(function() {
+ npm.config.set("development", true)
+ npm.config.set("production", false)
+ npm.commands.install(['.'], function(err) {
+ var p = path.resolve(pkg, 'node_modules/format-package-json/package.json')
+ t.ok(JSON.parse(fs.readFileSync(p, 'utf8')), "format-package-json was installed")
+ p = path.resolve(pkg, 'node_modules/format-package-json/node_modules/detect-indent/package.json')
+ t.ok(JSON.parse(fs.readFileSync(p, 'utf8')), "format-package-json/node_modules/detect-indent was installed")
+ p = path.resolve(pkg, 'node_modules/foo/package.json')
+ t.notOk(fs.existsSync(p), "foo was not installed")
+ t.end()
+ })
+ })
+})
+
+test('cleanup', function(t) {
+ process.chdir(__dirname)
+ rimraf.sync(path.resolve(pkg, 'node_modules'))
+ t.end()
+})
+
View
11 test/tap/install-development/package.json
@@ -0,0 +1,11 @@
+{
+ "name": "install-development",
+ "version": "0.0.0",
+ "description": "only install dev deps",
+ "dependencies": {
+ "foo": "*"
+ },
+ "devDependencies": {
+ "format-package-json": "*"
+ }
+}
View
38 test/tap/install-production.js
@@ -0,0 +1,38 @@
+var common = require('../common-tap.js')
+var test = require('tap').test
+var npm = require('../../')
+var osenv = require('osenv')
+var path = require('path')
+var fs = require('fs')
+var rimraf = require('rimraf')
+var mkdirp = require('mkdirp')
+var pkg = path.join(__dirname, 'install-development')
+//shared with ./install-development.js
+
+test("setup", function (t) {
+ rimraf.sync(path.resolve(pkg, 'node_modules'))
+ process.chdir(pkg)
+ t.end()
+})
+
+test('"npm install . --production" should install dependencies', function(t) {
+ npm.load(function() {
+ npm.config.set("development", false)
+ npm.config.set("production", true)
+ npm.commands.install(['.'], function(err) {
+ var p = path.resolve(pkg, 'node_modules/foo/package.json')
+ t.ok(JSON.parse(fs.readFileSync(p, 'utf8')), "foo was installed")
+ p = path.resolve(pkg, 'node_modules/format-package-json/package.json')
+ t.notOk(fs.existsSync(p), "format-package-json was not installed")
+ t.end()
+ })
+ })
+})
+
+test('cleanup', function(t) {
+ process.chdir(__dirname)
+ rimraf.sync(path.resolve(pkg, 'node_modules'))
+ t.end()
+})
+
+
Something went wrong with that request. Please try again.