include devDependencies in outdated and update #3262

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
Contributor

jdiamond commented Mar 17, 2013

For #2369 and #3250.

rawtaz commented Mar 18, 2013

+1 to this. I spent the last hour or so not believing that a package manager like npm cannot update dependencies (be it dev dependencies in this case) using npm update or npm update --dev or similar. This is needed! :)

+1 I had exactly the same problem as @rawtaz. To workaround it I've been doing this: npm cache clear <module name> && npm uninstall <module name> && npm install

Lovely! 👍

Baggz commented Apr 2, 2013

👍

rawtaz commented Apr 2, 2013

Has this made its way into core yet?

Contributor

jdiamond commented Apr 3, 2013

I don't think so. I assume Isaac will close the pull request when he decides one way or the other. I'm still using my local linked version of npm with this change so I hope he accepts it.

Contributor

edef1c commented Apr 3, 2013

I think that we should just include devDeps if they're already installed - a --dev doesn't make too much sense to me. (discussion welcome)

Whether update requires --dev or includes devDependencies by default is a minor implementation issue. Actually getting update to work for devDependencies, one way or another, is what really matters here. There have been open issues on this for a year with no resolution. Now that someone contributed a fix it'd be nice to have it merged.

Siyfion commented Apr 18, 2013

This is completely ridiculous. Why hasn't this, or some other strategy, been put in place to allow us to update devDependencies? Manually going through a list of 20 dependencies and calling npm install <pkg> is silly.

Contributor

jdiamond commented Apr 18, 2013

Sorry. =(

I agree that it's frustrating. My solution to cope (until finally deciding to patch npm) was to move my devDependencies into dependencies, npm up, and then move everything back. Maybe that would be less tedious than manually installing every package?

Contributor

edef1c commented Apr 19, 2013

@jdiamond Does this require npm update --dev or just npm update? I'd love to merge this.

Contributor

jdiamond commented Apr 19, 2013

Works just like install so no --dev. You can use --production to disable it.

Siyfion commented Apr 20, 2013

@jdiamond I thought install did need a --dev to install dev dependencies?

Contributor

jdiamond commented Apr 20, 2013

@Siyfion Oh, I was talking about when you run it as npm install and it reads your devDependencies from package.json without needing --dev. This patch makes npm update work the same way.

Owner

isaacs commented Apr 21, 2013

This doesn't work just like install, though. It'll install devDependencies at all levels, not just at the first level.

When you set opts.dev, the logic should be config.dev || (!config.production && at top level && !config.global)

Contributor

jdiamond commented Apr 21, 2013

Hi Isaac, thanks for the feedback.

This is how I'm testing:

  1. I have package.json with mocha as a dev dependency. mocha has should and coffee-script as dev dependencies.
  2. When I run npm install, mocha is installed, but should and coffee-script are not installed in my node_modules nor in mocha's.
  3. If I install an older version of mocha and then run npm update, it updates mocha and its non-dev dependencies, but it still doesn't install should or coffee-script.
  4. If I install an older version of mocha, cd into node_modules/mocha, and then run npm update, it does install should and coffee-script.

Is step 4 the behavior you want to avoid?

mourner commented Jun 26, 2013

👍

Member

robertkowalski commented Jul 1, 2013

Hey @jdiamond, can I help you with that PR that is getting old?

I could add a commit and afterwards you rebase our work into one commit.

Contributor

jdiamond commented Jul 2, 2013

Hi @robertkowalski, I just rebased my change onto isaacs/master without any conflicts so I'm not sure what work may need to be done. I'm really just waiting for @isaacs to find some time to look at this PR again and answer the questions I asked. It's been working great for me, but if you think you can improve it enough to satisfy him, please do so.

Member

robertkowalski commented Jul 3, 2013

@jdiamond please pull
robertkowalski/npm@3568dde into
your branch and and rebase the both commits from us into one. It
should do the job then. Strangly I were not able to do a pull request to
your repo, somehow GitHub fails here.

2013/7/2 Jason Diamond notifications@github.com

Hi @robertkowalski https://github.com/robertkowalski, I just rebased my
change onto isaacs/master without any conflicts so I'm not sure what work
may need to be done. I'm really just waiting for @isaacshttps://github.com/isaacsto find some time to look at this PR again and answer the questions I
asked. It's been working great for me, but if you think you can improve it
enough to satisfy him, please do so.


Reply to this email directly or view it on GitHubhttps://github.com/isaacs/npm/pull/3262#issuecomment-20369385
.

Member

robertkowalski commented Jul 3, 2013

@isaacs I think that is what you meant? Is it okay now?

2013/7/3 Robert Kowalski rok@kowalski.gd

@jdiamond please pull
robertkowalski/npm@3568dde your branch and and rebase the both commits from us into one. It
should do the job then. Strangly I were not able to do a pull request to
your repo, somehow GitHub fails here.

2013/7/2 Jason Diamond notifications@github.com

Hi @robertkowalski https://github.com/robertkowalski, I just rebased
my change onto isaacs/master without any conflicts so I'm not sure what
work may need to be done. I'm really just waiting for @isaacshttps://github.com/isaacsto find some time to look at this PR again and answer the questions I
asked. It's been working great for me, but if you think you can improve it
enough to satisfy him, please do so.


Reply to this email directly or view it on GitHubhttps://github.com/isaacs/npm/pull/3262#issuecomment-20369385
.

any progress on this?

+1 on getting this in, would be handy!

Member

domenic commented Aug 16, 2013

@robertkowalski want to create a fresh new PR for this?

Member

domenic commented Sep 8, 2013

Fixed by #3863.

domenic closed this Sep 8, 2013

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