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

install: Add support for --development, installing devDependencies only #9024

Closed
wants to merge 9 commits into from

Conversation

bengl
Copy link
Contributor

@bengl bengl commented Jul 22, 2015

This is a re-do of #4791 for v3.x.

The intention here is to have a CLI flag that complements --production. We have several use cases for this, including switching between registries for both sets of dependencies. A test similar to the one for --production is included.

The discussion on #4791 seemed to be all about the naming of the flag, so here are some alternatives, in case --development isn't acceptable:

  • --not-production
  • --only=[production,development]

@othiym23
Copy link
Contributor

Thanks for doing this! I closed #4791 in favor of this PR.

My objections to the name of the flag have nothing to do with its fitness for purpose (I think naming the flags --production and --development make intuitive sense), but are instead just pointing out that --development already means something for install, and therefore requires a deprecation cycle (see #6200 for my original thoughts on this).

Given that npm i --development currently does something that nobody expects or enjoys, it can probably be a pretty abbreviated deprecation cycle, but we do need to add a deprecation to npm@3 indicating that the behavior of npm install --development is going to change in the next major version, and then release this patch as part of npm@4, which could be relatively soon, as we're moving npm towards a more iterative release process and keep working on keeping our versioning true to the spirit of semver (i.e. not waiting another year before we release npm@4).

If you want to put together a patch that resolves #6200, you'd be pretty much guaranteeing this PR's path to landing. The CLI team will get to it eventually if you don't, but we have a lot of high-priority things contending for our attention right now, so it might take us a little while.

@iarna
Copy link
Contributor

iarna commented Jul 22, 2015

I'm kind of fond of --only={prod[uction]|dev[elopment}--production has lots of other side effects (eg setting NODE_ENV=production in lifecycle scripts) and it'd be nice to have a version that just impacts the modules being installed.

So I'd favor landing this as --only along with a deprecation to --dev.

But regardless, I think this needs a few more things before we can land it:

  1. Documentation updates
  2. Integration with npm ls, and maybe npm outdated and npm prune.

@bengl
Copy link
Contributor Author

bengl commented Jul 23, 2015

Sure, I'm happy to proceed and do a --dev deprecation and clean this up. With a few questions:

  1. In cases where --dev currently behaves in ways other than how it does in install (e.g. shrinkwrap, ls, etc.), should --dev also be deprecated in favor of --development, using the existing behavior?
  2. Is there a agreement on --only?

I think the way I'd like to go about it is:

  • Go with --only.
  • install --production operates as before, while install --only=production doesn't set NODE_ENV or other side-effects.
  • Add deprecation warning for --dev in all uses, preferring the use of --only=development instead, which would have the following behaviors:
    • For install, use new logic as per this PR.
    • For all other commands, operate as --dev did before.
  • Documents!

How does that sound?

@othiym23
Copy link
Contributor

  1. In cases where --dev currently behaves in ways other than how it does in install (e.g. shrinkwrap, ls, etc.), should --dev also be deprecated in favor of --development, using the existing behavior?

--dev and --development are synonyms, so we're not trying to deprecate one in favor of the other. The deprecation is of npm install --dev / npm install --development's current unhelpful behavior. The deprecation cycle doesn't have to be super long-lived, but the current behavior is working as intended (but why?).

  1. Is there an agreement on --only?

I like @iarna's case for using a switch that's less overloaded, so yes, let's call that a consensus. That also has the nice side effect of decoupling the deprecation of --dev / --development from shipping this feature, because npm install --only=dev / npm install --only=production isn't a backwards-incompatible change.

That said, I agree with @iarna that it would be really good to see the changes to --development and --production flow out into the other commands where they're used right now. We can keep talking about whether we want to reuse the existing switches or move to using --only in those contexts -- shipping just the npm install bits seems like the appropriate level of work to start, and lets you get the piece you really need for your day job sooner.

  • install --production operates as before, while install --only=production doesn't set NODE_ENV or other side-effects.

This sounds good, but I think maybe deprecating --production in favor of --only=production in most contexts is a good idea.

  • Add deprecation warning for --dev in all uses, preferring the use of --only=development instead, which would have the following behaviors:
    • For install, use new logic as per this PR.
    • For all other commands, operate as --dev did before.

With the caveat that preserving dev and development as interchangeable synonyms is a good thing from an ergonomics point of view, this all sounds good.

  • Documents!

Yes, please!

Thanks for picking this up, @bengl!

@bengl
Copy link
Contributor Author

bengl commented Jul 24, 2015

OK cool, switched it over to --only.

More clarification questions, about specific non-install commands:

  1. outdated, updated and shrinkwrap: All use --dev to mean "and also devDependencies". If that's deprecated in favor of --only=dev, that's a bit weird. (prune also shares that meaning, but it seems safer there since it's the default.) Should something like --also={prod[uction]|dev[elopment]} be used for these ones? Or perhaps it makes sense to keep --dev around for these ones?
  2. ls: (not a question, just here for completion's sake for commands currently using --dev) This one's easy as --only makes total sense here.

@othiym23
Copy link
Contributor

outdated, updated and shrinkwrap: All use --dev to mean "and also devDependencies". If that's deprecated in favor of --only=dev, that's a bit weird. (prune also shares that meaning, but it seems safer there since it's the default.) Should something like --also={prod[uction]|dev[elopment]} be used for these ones?

I like the idea of --also=dev as a dual to --only=dev. 👍

Or perhaps it makes sense to keep --dev around for these ones?

Why not have both?

@iarna
Copy link
Contributor

iarna commented Jul 28, 2015

I like also as well. I approve of separating these use cases. Things that mean different things should look different.

@bengl
Copy link
Contributor Author

bengl commented Aug 1, 2015

OK, I went through and added --also and --only wherever it made sense, plus docs, etc.

I only put the deprecation warning for install, because for the others:

"Why not have both?" -- @othiym23

So I think that's it and this is ready. The only failing test on the non-0.8 travis builds appears to be unrelated to my changes.

If I've missed something, or you'd like me to squash commits or something, I'm happy to fix.

@othiym23 othiym23 added this to the 3.x-next-next milestone Aug 4, 2015
@othiym23
Copy link
Contributor

othiym23 commented Aug 4, 2015

Hey Ben, @iarna is going to take a look at this and we're going to try to get it in shape to land in next week's npm@3 release – should we plan on backporting this to npm@2? Thanks for your patience and all the work you've done so far!

@othiym23 othiym23 added the review label Aug 4, 2015
@bengl
Copy link
Contributor Author

bengl commented Aug 4, 2015

I'm not Ben, I'm Bryan 😄 (no worries, common mistake!).

Excellent news on potentially having it in npm@3.x-next-next! Backporting it to npm@2 would require a different implementation (likely using @davglass's patch), but could probably re-use much of the same config, doc and test changes. If I find some spare time over the next week or so I'll give it a shot.

@othiym23
Copy link
Contributor

othiym23 commented Aug 5, 2015

Sorry, Bryan! I know who you are! I believe in you! I also believe you exist because I've met you! This is what I get for trying to bang out that comment quickly in the middle of a meeting, while also thinking about my esteemed colleague Ben!

Backporting it to npm@2 would require a different implementation (likely using @davglass's patch), but could probably re-use much of the same config, doc and test changes. If I find some spare time over the next week or so I'll give it a shot.

My actual question is: is this worth the effort? I.e. how long is your esteemed employer going to take to upgrade to npm@3 once it's generally released? We're probably going to support npm@2 for ~6mo-1yr because npm@3 is such a big change (and also to properly support the new Node LTS initiative), but as much as possible, I'd like to restrict changes to npm@2 to bug fixes and keep feature on npm@3. That said, I'm willing to make an exception in this case if it's a big driver for your team and the teams you support.

@bengl
Copy link
Contributor Author

bengl commented Aug 5, 2015

Haha, again, no worries. My username makes it far too easy.

From my esteemed employer's perspective, I don't think we're too concerned with npm@2, so if you'd like to minimize new features there, I'm all for it 👍 .

@davglass
Copy link
Contributor

davglass commented Aug 6, 2015

👍 only for npm@3

iarna pushed a commit that referenced this pull request Aug 12, 2015
iarna pushed a commit that referenced this pull request Aug 12, 2015
iarna pushed a commit that referenced this pull request Aug 12, 2015
iarna pushed a commit that referenced this pull request Aug 12, 2015
iarna pushed a commit that referenced this pull request Aug 12, 2015
iarna pushed a commit that referenced this pull request Aug 12, 2015
iarna pushed a commit that referenced this pull request Aug 12, 2015
@iarna
Copy link
Contributor

iarna commented Aug 14, 2015

Hey! Good news! This is out and live in 3.3.0!

@bengl
Copy link
Contributor Author

bengl commented Aug 19, 2015

🎉 thanks!

@jcollum
Copy link

jcollum commented Aug 31, 2015

I'm reading through this thread and now I'm fairly confused. I don't think this is the right place to put a discussion of how to build/deploy node projects though, so can someone point me to a discussion or a best practices? I'm trying to automate pushing a node library to a local npm via a CI server. Thought that:

  1. npm install --dev
  2. transpile
  3. run the mocha tests
  4. npm pack (if tests pass)
  5. deploy new lib tgz file to local npm repo

Would do the trick. Should I just be doing npm install instead? But that will install a lot of stuff I don't need to actually run/build the npm module... about 15 deps that are unrelated to the actual building/testing of the module. npm install --dev seems really straightforward (except the part about not working 😄)

Suggestions? (currently using npm 2.11.3, node 0.12.7)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants