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

deps: upgrade npm to 4.0.2 #9848

Closed
wants to merge 1 commit into from
Closed

deps: upgrade npm to 4.0.2 #9848

wants to merge 1 commit into from

Conversation

iarna
Copy link
Member

@iarna iarna commented Nov 29, 2016

Checklist
  • make -j8 test (UNIX), or vcbuild test nosign (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)
  • deps
Description of change

Breaking changes:

  • Partial shrinkwraps are no longer supported. npm-shrinkwrap.json is
    considered a complete installation manifest except for devDependencies.
  • npm shrinkwrap includes development dependencies by default. You can avoid doing this with npm shrinkwrap --only=prod. Likewise if your shrinkwrap contains dev dependencies you can avoid installing them with npm install --only=prod or setting NODE_ENV to production.
  • prepublish has been deprecated, replaced by prepare. A prepublishOnly
    script has been temporarily added, which will only run on npm publish.
  • npm search rewritten to stream results, and no longer supports sorting.
  • npm scripts no longer prepend the path of the node executable used to run
    npm before running scripts. A --scripts-prepend-node-path option has been
    added to configure this behavior.
  • npat has been removed.
  • npm outdated exits with exit code 1 if it finds any outdated packages.
  • npm tag has been removed after a deprecation cycle. Use npm dist-tag.

Other notable changes:

  • npm will now send Npm-In-CI and Npm-Scope headers to registries when fetching modules. The former will allow registries to more accurately distinguish between CI and user traffic. The latter will allow registries to implement features on the basis of the scope of the project (rather than the module actually being installed).

Changelogs

r: @jasnell
r: @Fishrock123

@iarna iarna added dont-land-on-v4.x npm Issues and PRs related to the npm client dependency or the npm registry. labels Nov 29, 2016
@nodejs-github-bot nodejs-github-bot added the npm Issues and PRs related to the npm client dependency or the npm registry. label Nov 29, 2016
@addaleax addaleax added the semver-major PRs that contain breaking changes and should be released in the next major version. label Nov 30, 2016
@addaleax addaleax added this to the 8.0.0 milestone Nov 30, 2016
@addaleax
Copy link
Member

I guess this supersedes #9284 so I’ll be closing that one. :)

@addaleax addaleax mentioned this pull request Nov 30, 2016
4 tasks
@iarna
Copy link
Member Author

iarna commented Nov 30, 2016

@addaleax Oh! Yes indeed, thank you for that! =)

@Fishrock123
Copy link
Contributor

Tests pass, I suppose this is fine to land on master? Let's re-discuss at the next CTC meeting.

@jasnell
Copy link
Member

jasnell commented Dec 5, 2016

PR needs to be rebased and updated to resolve conflicts

@rvagg
Copy link
Member

rvagg commented Dec 14, 2016

@nodejs/ctc be warned: this is going to a vote at next week's meeting so we can resolve it without going in circles. It's going to be merged into master so the question is whether it goes into v7.

We held back v3 because of the large delta and high chance of breaking users' workflows. This change set is much smaller and contains less risk for tripping up users. It also contains changes (prepublish deprecation) that are going through a deprecation cycle which we may want to hand off to users in preparation for npm@5.

So @nodejs/ctc, please look over the breaking changes listed in OP and try and form an opinion about whether to consider this a breaking change for Node or not. Please drop your vote in here unless you're sure you'll be at next week's meeting

@MylesBorins
Copy link
Contributor

I'm +1 on this change landing on Master.
If citgm can work cleanly with this change I feel fairly positive about it running on node 7. I would be very intersted in hearing statistics of what % of people on 7 have updgraded npm to 4

@addaleax
Copy link
Member

addaleax commented Dec 14, 2016

/cc @nodejs/npm for all of the above (and if you have any strong opinions, it’s probably the right time to voice them)

Barring any shocking new insights I’d be +1 on landing on master and v7.x.

@zkat
Copy link
Contributor

zkat commented Dec 15, 2016

....how are there any conflicts on this? Unless folks have been messing with deps/npm? This should be straight-up cherry-pickable.

That said, y'all can merge this, but npm@4.0.5 is becoming latest tomorrow (and probably downstreamed friday/monday-ish). But it should be fine to just grab this one in the meantime. (really, though, wtf@conflicts)

@MylesBorins
Copy link
Contributor

@zkat I think the diff for this commit was built off a version of master that didn't have the latest v3.x

Likely that is where conflicts come from

@targos
Copy link
Member

targos commented Dec 15, 2016

For those who were wondering if the prepublish deprecation prints something, the answer is yes. Here is the output:

npm WARN prepublish-on-install As of npm@5, `prepublish` scripts will run only for `npm publish`.
npm WARN prepublish-on-install (In npm@4 and previous versions, it also runs for `npm install`.)
npm WARN prepublish-on-install See the deprecation note in `npm help scripts` for more information.

I think that if we decide not to land npm 4 in node 7, npm 3 should be updated to support prepare scripts.
Otherwise, people who don't upgrade to npm 4 will experience a major breakage in their workflows when node 8 goes out and they cannot prepare their projects for it in advance.

@Fishrock123
Copy link
Contributor

@zkat I merged the npm@3 update to master because that is where it best belonged at the time. :P

@addaleax
Copy link
Member

I’m closing this and moving the ctc-agenda label as it’s superseded by #10330 (“deps: upgrade npm to 4.0.5”)

@addaleax addaleax closed this Dec 19, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
npm Issues and PRs related to the npm client dependency or the npm registry. semver-major PRs that contain breaking changes and should be released in the next major version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants