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

update: presence of npm-shrinkwrap.json blocks update #7423

Closed
wants to merge 1 commit into from

Conversation

smikes
Copy link
Contributor

@smikes smikes commented Feb 22, 2015

Candidate fix for issue #6566

Maybe also fix for #6048

This seemed hard:

The way @iarna and I want this to work is npm update --save will update both npm-shrinkwrap.json and package.json, but npm update alone in the presence of npm-shrinkwrap.json should be a no-op / WARN that shrinkwrapping is blocking updates and you should use npm update --save.

So I did the simpler thing first; this change just stops update from running in the presence of an npm-shrinkwrap.json file. My proposed workflow for updating a shrinkwrap file, after this patch, is:

rm -f npm-shrinkwrap.json
npm update --save
npm shrinkwrap

This way npm-shrinkwrap.json functions as a lock, preventing npm update from running. To remove the lock, you must remove the shrinkwrap file. To restore the lock, you must run npm shrinkwrap again.

In #6566, the proposal was that npm update --save would remove the lock and reapply it. I think it's better for the npm commands to run at a more granular level, and allow users to evolve scripts around them as needed, than to have a shrinkwrap-related workflow embedded in npm update --save.

Comments/bikeshedding/etc v welcome.

@smikes
Copy link
Contributor Author

smikes commented Feb 25, 2015

In #6566, the proposal was that npm update --save would remove the lock and reapply it. I think it's better for the npm commands to run at a more granular level, and allow users to evolve scripts around them as needed, than to have a shrinkwrap-related workflow embedded in npm update --save.

Since that's not the medium or long-term plan, this PR is not ready to patch yet. I will mark it as such in the title.

@smikes smikes changed the title update: presence of npm-shrinkwrap.json blocks update NOT READY: update: presence of npm-shrinkwrap.json blocks update Feb 25, 2015
@smikes smikes changed the title NOT READY: update: presence of npm-shrinkwrap.json blocks update update: presence of npm-shrinkwrap.json blocks update Feb 28, 2015
@smikes
Copy link
Contributor Author

smikes commented Feb 28, 2015

Reading this spec again: #6566 (comment)

The way @iarna and I want this to work is npm update --save will update both npm-shrinkwrap.json and package.json, but npm update alone in the presence of npm-shrinkwrap.json should be a no-op / WARN that shrinkwrapping is blocking updates and you should use npm update --save.

This PR implements the second half; npm update in the presence of a shrinkwrap file. It does not document the shrinkwrap-updating behavior that npm update --save should give.

Now, this could go in as-is (with some documentation added), or it could be held over for a more complete solution.

Three ways I could take this:

  1. polish this up, fixing formatting and adding documentation, for inclusion now, with the understanding that npm@3 will implement the complete desired behavior ;
  2. close the PR, since npm@3 is taking this in a different direction
  3. actually solve the "update shrinkwrap.json" file, but not as elegantly as npm@3 will. So, for example, move npm-shrinkwrap.json away, run the update command, run npm shrinkwrap, and then display the diff between old and new shrinkwrap files.

My preferences are towards option 1 or 2. I will put this on the 'needs comment' tab.

@smikes
Copy link
Contributor Author

smikes commented Mar 2, 2015

Withdrawing this for now. Maybe revisit after npm@3

@smikes smikes closed this Mar 2, 2015
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.

None yet

1 participant