Renovate: package-lock.json changes after merging a Renovate PR and running npm install

[felixfbecker]

What Renovate type are you using?
Renovate GitHub App

Describe the bug
After merging a Renovate PR and running npm install locally, package-lock.json changes.

This is unfortunate, because it means that the automation still requires a manual npm install, commit and push, at which point I could have just updated the dependency manually in the first place.

I am running npm v6.2.0 (latest). What version is Renovate using?

Diff

diff --git a/web/package-lock.json b/web/package-lock.json
index 350caa888a..d8d390e406 100644
--- a/web/package-lock.json
+++ b/web/package-lock.json
@@ -6906,8 +6906,7 @@
         "ansi-regex": {
           "version": "2.1.1",
           "bundled": true,
-          "dev": true,
-          "optional": true
+          "dev": true
         },
         "aproba": {
           "version": "1.2.0",
@@ -6928,14 +6927,12 @@
         "balanced-match": {
           "version": "1.0.0",
           "bundled": true,
-          "dev": true,
-          "optional": true
+          "dev": true
         },
         "brace-expansion": {
           "version": "1.1.11",
           "bundled": true,
           "dev": true,
-          "optional": true,
           "requires": {
             "balanced-match": "^1.0.0",
             "concat-map": "0.0.1"
@@ -6950,20 +6947,17 @@
         "code-point-at": {
           "version": "1.1.0",
           "bundled": true,
-          "dev": true,
-          "optional": true
+          "dev": true
         },
         "concat-map": {
           "version": "0.0.1",
           "bundled": true,
-          "dev": true,
-          "optional": true
+          "dev": true
         },
         "console-control-strings": {
           "version": "1.1.0",
           "bundled": true,
-          "dev": true,
-          "optional": true
+          "dev": true
         },
         "core-util-is": {
           "version": "1.0.2",
@@ -7080,8 +7074,7 @@
         "inherits": {
           "version": "2.0.3",
           "bundled": true,
-          "dev": true,
-          "optional": true
+          "dev": true
         },
         "ini": {
           "version": "1.3.5",
@@ -7093,7 +7086,6 @@
           "version": "1.0.0",
           "bundled": true,
           "dev": true,
-          "optional": true,
           "requires": {
             "number-is-nan": "^1.0.0"
           }
@@ -7108,7 +7100,6 @@
           "version": "3.0.4",
           "bundled": true,
           "dev": true,
-          "optional": true,
           "requires": {
             "brace-expansion": "^1.1.7"
           }
@@ -7116,14 +7107,12 @@
         "minimist": {
           "version": "0.0.8",
           "bundled": true,
-          "dev": true,
-          "optional": true
+          "dev": true
         },
         "minipass": {
           "version": "2.2.4",
           "bundled": true,
           "dev": true,
-          "optional": true,
           "requires": {
             "safe-buffer": "^5.1.1",
             "yallist": "^3.0.0"
@@ -7142,7 +7131,6 @@
           "version": "0.5.1",
           "bundled": true,
           "dev": true,
-          "optional": true,
           "requires": {
             "minimist": "0.0.8"
           }
@@ -7223,8 +7211,7 @@
         "number-is-nan": {
           "version": "1.0.1",
           "bundled": true,
-          "dev": true,
-          "optional": true
+          "dev": true
         },
         "object-assign": {
           "version": "4.1.1",
@@ -7236,7 +7223,6 @@
           "version": "1.4.0",
           "bundled": true,
           "dev": true,
-          "optional": true,
           "requires": {
             "wrappy": "1"
           }
@@ -7322,8 +7308,7 @@
         "safe-buffer": {
           "version": "5.1.1",
           "bundled": true,
-          "dev": true,
-          "optional": true
+          "dev": true
         },
         "safer-buffer": {
           "version": "2.1.2",
@@ -7359,7 +7344,6 @@
           "version": "1.0.2",
           "bundled": true,
           "dev": true,
-          "optional": true,
           "requires": {
             "code-point-at": "^1.0.0",
             "is-fullwidth-code-point": "^1.0.0",
@@ -7379,7 +7363,6 @@
           "version": "3.0.1",
           "bundled": true,
           "dev": true,
-          "optional": true,
           "requires": {
             "ansi-regex": "^2.0.0"
           }
@@ -7423,14 +7406,12 @@
         "wrappy": {
           "version": "1.0.2",
           "bundled": true,
-          "dev": true,
-          "optional": true
+          "dev": true
         },
         "yallist": {
           "version": "3.0.2",
           "bundled": true,
-          "dev": true,
-          "optional": true
+          "dev": true
         }
       }
     },
@@ -11277,7 +11258,6 @@
           "version": "0.1.4",
           "bundled": true,
           "dev": true,
-          "optional": true,
           "requires": {
             "kind-of": "^3.0.2",
             "longest": "^1.0.1",
@@ -12243,8 +12223,7 @@
         "longest": {
           "version": "1.0.1",
           "bundled": true,
-          "dev": true,
-          "optional": true
+          "dev": true
         },
         "lru-cache": {
           "version": "4.1.3",

[hutson]

@felixfbecker Renovate depends on npm version 6.2.0.

Maybe it has to do with the options passed to the npm command by Renovate?

Do you have a repository that can be pulled down to replicate the outdated dependency that Renovate is upgrading?

Basically I would like to see if the options passed to npm are causing a different package-lock.json result than just npm install.

It could also just be that's how npm works.

[rarkins]

Another related npm issue from the now-archived repo: https://github.com/npm/npm/issues/17722

Here is an attempt to move that issue into the new npm.community: https://npm.community/t/package-lock-json-and-optional-packages/692

When I previously pinged the npm team about adding an option like npm install --from-lock-file command they said to use npm ci in dev and on CI instead.

[rarkins]

I was browsing other npm issues and see that @felixfbecker has experience with npm’s optional dependency handling before: https://github.com/npm/npm/issues/18202#issuecomment-333679234 :)

[felixfbecker]

I wanted to stress again that this is probably the number one friction point with Renovate and really takes the automation benefits away, as I always have to manually commit a new lockfile again. Since we also have some packages configured to automerge, I constantly get questions like "why does my lockfile always change? I tried reinstalling npm and clearing the cache but it still changes". Maybe we can express to npm that this is a really important issue for tools like Renovate, and if npm does not fix it soon, find a workaround in Renovate (e.g. updating lockfiles manually)

[felixfbecker]

This seems to be the latest bug tracking this in npm: https://npm.community/t/package-lock-json-keeps-changing-between-platforms-and-runs/1129

[rarkins]

The recommendation from npm appears to be that your team should be running npm ci and not npm install. If this is their position then it explains somewhat why they don’t seem to be treating this with high priority.

[felixfbecker]

I don't understand what you/they mean by this - should I run npm ci locally? The name of the command to me indicates that it is intended to be run only in CI.
I understand that it is the equivalent for a npm install --from-lock-file, as it will never change the lockfile, if Renovate needs that functionality. But Renovate _needs_ to update the lockfile, so it doesn't seem like a solution at all.

[rarkins]

I have pinged the npm team again just now to confirm, but I believe they intend for you and your team to run “npm ci” instead of “npm install” unless you are adding dependencies or modifying versions manually. I think you need to ignore the unintuitiveness of the command name and adopt their recommendation, assuming they confirm it.

Meanwhile yes Renovate will run install because thats it’s job to update lock files. If it’s your job to just install packages from the lock file then you will run “npm ci”.

I hope though the npm team will help solve the problem either by:

• not modifying the lock file when you run “npm install” unless the lock file is invalid. Ie stop flapping optional lines between platforms
• making changes under the hood so that “npm install” just calls the “npm ci” logic of a lockfile is present and valid

[rarkins]

Can you confirm one thing: is Renovate adding the “optional” lines in your lockfile and removing them? And which platform are you and developers running “npm install” on when the file changes back?

[felixfbecker]

npm ci nukes node_modules before installation and generates it from scratch - it makes sense and is fast in CI, where you don't have node_modules yet so you just want to recreate from the lockfile. But in development, where you already have node_modules, you want npm to not reinstall everything - if nothing changed, npm should do nothing. For example, we run npm install as part of our dev script, which takes only a second if nothing changed. npm ci on the other hand would take >10s.

Can you confirm one thing: is Renovate adding the “optional” lines in your lockfile and removing them? And which platform are you and developers running “npm install” on when the file changes back?

99% of our team runs on macOS, while I assume Renovate runs on Linux.
The sequence of events is:

• Renovate PR adds optional: true in many places in the package-lock
• Renovate PR is merged
• Team member on macOS runs npm install on master
• npm removes the optional: true fields in all those places where Renovate had added them (see diff in the original post)

[rarkins]

Thanks for the data points. I think the npm cli team assumes the performance of npm ci is fast enough for most/all situations but in your case it’s evidently adding an unnecessary 10s. Although it does wipe out node modules each time, npm ci should usually reinstall from cache so removing that directory is not actually as “inefficient” as it sounds.

[felixfbecker]

My understanding was that npm ci is only for CI (and that's why it's named that way). There are other disadvantages too, for example some packages like fsevents, node-sass, Puppeteer or Cypress will download additional files in postinstall, which adds to npm ci from-scratch install time. npm linked packages also get overridden. I think npm ci also does not automatically run npm audit.

Some hard numbers:

npm ci: 55.66s

> time npm ci
npm WARN prepare removing existing node_modules/ before installation

> @gql2ts/[email protected] postinstall /Users/felix/src/github.com/sourcegraph/sourcegraph/web/node_modules/@gql2ts/language-typescript
> node postinstall.js

Message from @gql2ts/language-typescript:
  To enable pretty-printing of your code, please install prettier from npm/yarn


> [email protected] install /Users/felix/src/github.com/sourcegraph/sourcegraph/web/node_modules/fsevents
> node install

[fsevents] Success: "/Users/felix/src/github.com/sourcegraph/sourcegraph/web/node_modules/fsevents/lib/binding/Release/node-v64-darwin-x64/fse.node" already installed
Pass --update-binary to reinstall or --build-from-source to recompile

> [email protected] install /Users/felix/src/github.com/sourcegraph/sourcegraph/web/node_modules/puppeteer
> node install.js

Downloading Chromium r575458 - 80.4 Mb [====================] 100% 0.0s
Chromium downloaded to /Users/felix/src/github.com/sourcegraph/sourcegraph/web/node_modules/puppeteer/.local-chromium/mac-575458

> [email protected] install /Users/felix/src/github.com/sourcegraph/sourcegraph/web/node_modules/node-sass
> node scripts/install.js

Cached binary found at /Users/felix/.npm/node-sass/4.9.0/darwin-x64-64_binding.node

> [email protected] postinstall /Users/felix/src/github.com/sourcegraph/sourcegraph/web/node_modules/node-sass
> node scripts/build.js

Binary found at /Users/felix/src/github.com/sourcegraph/sourcegraph/web/node_modules/node-sass/vendor/darwin-x64-64/binding.node
Testing binary
Binary is fine
added 2042 packages in 55.016s
       55.66 real        14.34 user        23.21 sys

npm i (noop) 13.59s

> time npm i
npm WARN [email protected] requires a peer of ajv@^6.0.0 but none is installed. You must install peer dependencies yourself.
npm WARN [email protected] requires a peer of [email protected] - 3 but none is installed. You must install peer dependencies yourself.
npm WARN [email protected] requires a peer of typescript@^2.1.0 but none is installed. You must install peer dependencies yourself.
npm WARN [email protected] requires a peer of typescript@^2.3.0 but none is installed. You must install peer dependencies yourself.
npm WARN [email protected] requires a peer of typescript@>= 2.3.1 < 3 but none is installed. You must install peer dependencies yourself.

audited 21083 packages in 13.094s
found 130 vulnerabilities (114 low, 11 moderate, 5 high)
  run `npm audit fix` to fix them, or `npm audit` for details
       13.59 real        13.40 user         1.38 sys

[rarkins]

So if Renovate were to “work around” this, we’d need to do a diff/compare of the before and after lock files and strip out all newly-added “optional”: true lines. It’s not foolproof though because we’d need to strip it from newly-added dependencies too, and in theory here could be one that’s optional on both Linux and OSX and could get added again once npm install is run on OSX (i.e. edge case)

[felixfbecker]

I wonder if these packages are actually optional. If they truly are, then the Linux behaviour seems right (having an optional: true field). In that case I would also be okay with a postinstall script I put in package.json that reads package-lock and does some kind of "normalization" where it adds the optional fields back on macOS by finding out which packages are actually optional.

[rarkins]

Yes, I think probably the Linux command is doing the right thing and the OSX one is problematic. Do you think a post install script could diff/grep/reset so that it doesn’t ultimately modify the lock file unnecessarily when run on OSX? I think you could probably “trust” that any optional: true line is meant to be there.

[felixfbecker]

It's not clear to me how a diff/reset would work in practice. The solution I had in my head was to walk the tree to check which dependencies are optional or not. But that would probably require reverse-engineering npm's algorithm to a point where we could just fix the bug in npm itself.

[martijnwalraven]

I seem to be running into a related issue. I recently changed our repo to use npm ci during CI, relying on package-lock.json. Afterwards, Renovate opened a PR to pin dependencies (I had changed them to caret versions, not realizing Renovate wanted them pinned). The PR adds optional: true to various packages in package-lock.json, similar to what is mentioned above.

In addition, it also seems to add integrity: null to entries with local specifiers like "version": "file:packages/apollo-tracing".

These changes to the package-lock.json lead npm ci to fail while installing packages:

npm ERR! path /home/circleci/project/node_modules/apollo-tracing/node_modules/graphql-extensions/node_modules/apollo-server-env
npm ERR! code EEXIST
npm ERR! errno -17
npm ERR! syscall mkdir
npm ERR! EEXIST: file already exists, mkdir '/home/circleci/project/node_modules/apollo-tracing/node_modules/graphql-extensions/node_modules/apollo-server-env'
npm ERR! File exists: /home/circleci/project/node_modules/apollo-tracing/node_modules/graphql-extensions/node_modules/apollo-server-env
npm ERR! Move it away, and try again.

I receive the same error when checking out the PR locally and running npm ci.

It seems this may be related to this npm issue. We are using file: specifiers to resolve local dependencies in our monorepo, but this seems to be working fine otherwise (and is now recommended by the author of Lerna).

If I check out the Renovate PR locally and run npm install to recreate package-lock.json, that removes the optional: true and integrity: null entries, and npm ci runs without errors.

I pushed the updated package-lock.json to the PR, and CI also passes now. But it seems these changes to package-lock.json are likely to stay problematic, requiring us to run npm install manually on every Renovate PR.

Any idea what could be going on and how this could be fixed?

[rarkins]

@martijnwalraven I suspect that the npm ci failure is due to the integrity fields and not optional ones.

By the way you are welcome to leave dependencies in ranges and configure Renovate to leave them alone, although usually I'd ask people "why?" first, especially for devDependencies.

[rarkins]

@martijnwalraven I think there might be more going on - let's move yours to a separate issue. When I check out that repo and run npm i --package-lock-only then I already see some significant changes being made:

[rarkins]

Confirmed that @martijnwalraven's problem is another npm bug that seems to be a combination of (a) npm i --package-lock-only and (b) file: references. I have created #2388 in this repo to track it instead, and raised a bug report with npm here: https://npm.community/t/npm-i-package-lock-only-changes-lock-file-incorrectly-when-file-references-used-in-dependencies/1412

[rarkins]

@felixfbecker I was checking back through this issue and I don't think that we have any steps or example repository to reproduce this in yet?

[abernix]

@rarkins Thanks very much for taking a look at the problem @martijnwalraven (who is out on holiday at the moment) identified above — _and_ for moving it to a new issue, _and_ for reporting the (ultimate) cause of the bug to npm.community. ⭐️🙇⭐️

[felixfbecker]

I set up a repro case: https://npm.community/t/package-lock-json-keeps-changing-between-platforms-and-runs/1129/7?u=felixfbecker

[rarkins]

Here is a second npm.community post on the topic: https://npm.community/t/package-lock-json-changes-from-one-npm-install-to-the-next/1454/5

[prymitive]

I've also run into the same issue, would it be possible to tell renovate which nodejs/npm version to use when bumping packages rather than trying to work around each specific case like those optional: true lines being injected? npm had a similiar issues in the past (on 5.x) so there might be future regressions with some other keys.

[felixfbecker]

This happens even if the versions match exactly. It's the OS differences.

[prymitive]

Yes, but it's my understanding that not every npm version behaves that way.
npm verision before 5.6 (can't remember which one exactly) had the issue of producing different lock files on different platforms, then it was fixed (works fine with 5.6.1), then it was broken again with 6.1 (based on reports I've seen).
Right now I've pinned the version of nodejs I use on travis to 8.11.4, which included npm 5.6.1 and that fixed most problems with running npm install on CI. That's because when I've upgraded to 8.12 (with npm 6.4) I started getting different lock file on CI after npm install, and I have a check that fails if there are differences.
Being able to pin nodejs/npm version used by renovate would (at least for me) solve most of the pain.

[rarkins]

I'm hoping to support configurable npm versions in maybe a month's time, although as @felixfbecker points out, that's definitely not a cure-all for this particular issue. It's certainly a good idea regardless and may reduce the symptoms or prevent similar problems.

[stefee]

@rarkins
Do you know if skipInstalls works as a work-around for this optional:true issue? It wasn't totally clear from reading this thread and #2388.

[rarkins]

It means you won’t get any package lock updates at all so probably not a good option. It’s disappointing that this hasn’t been prioritised by npm yet. For anyone in this scenario, lockfiles are essentially “broken”

[rarkins]

By the way this one is difficult for Renovate to work around because it’s really the OSX client that’s misbehaving here and stripping out optional statements it shouldn’t. Ie not the Linux client that Renovate uses, which is correctly adding optional lines where they should be.

But faced with no fix, I’m considering trying. I think this logic might cover the majority of cases:

• diff the lockfile for changes after Renovate updates it using npm
• remove any added optional: true lines

There’s surely going to be some cases when it causes some other problem, but if it’s made opt-in (config-wise) then could be worth trying.

[stefee]

I feel like that fix might just flip the problem on its head by removing optional: true lines that end up getting re-added when running npm install locally? In some cases (maybe most cases) it would have a worse outcome, certainly if you're running Linux and you're on a team who are all running macOS.

I also imagine it might make troubleshooting the npm issue more problematic if we're modifying the lockfile programatically. Perhaps a config option just adds complexity to the problem without guaranteeing to solve it? I think it's worth some investigation though.

[felixfbecker]

For the record, we switched to yarn because of this and broken npm link.

[stefee]

@felixfbecker
Not a bad idea...

P.S. I ❤️ npm though

[stefee]

Discussion re: this particular optional:true npm issue moved to here:
https://npm.community/t/package-lock-json-is-not-complete-on-first-run-for-some-modules/1817

[stefee]

This fix in npm should resolve our optional:true issue: https://github.com/npm/cli/pull/76 🎉

[stefee]

This fix has been released in npm 6.6.0.

@rarkins will latest npm be available to renovate immediately?

[rarkins]

@srilq it will be merged in #3094 soon

[stefee]

@rarkins Yay, thanks

[felixfbecker]

@srilq I don't see it mentioned in the changelog: https://github.com/npm/cli/releases/tag/v6.6.0
and the bug report is still open: https://npm.community/t/package-lock-json-keeps-changing-between-platforms-and-runs/1129

Could you link to the fix?

[stefee]

@felixfbecker
134207174 npm.community#2569 Fix checking for optional dependencies. (@larsgw)
https://github.com/npm/cli/commit/134207174652e1eb6d7b0f44fd9858a0b6a0cd6c
https://github.com/npm/cli/pull/76

[stefee]

It looks like this fix hasn't changed the Linux release that renovate uses, but instead changes how the macOS release works to be in line with the Linux version (which makes sense, see this comment: https://github.com/renovatebot/renovate/issues/2294#issuecomment-410518993).

I think this issue can be resolved, but end-users need to make sure they are running npm>=6.6.

There is also still an npm registry issue to keep in mind which causes the https: –> http: diffs, but I don't think this will be fixed by a patch to the client. It's best to just ignore these if possible, and rm -rf node_modules && npm cache clean --force seems to help.

Update: this npm registry issue is fixed! (Source: https://npm.community/t/some-packages-have-dist-tarball-as-http-and-not-https/285/50). If you see the lockfile problem locally, you just need to run this and it should work as a permanent fix: git checkout package-lock.json && rm -rf node_modules && npm cache clean --force && npm install

[rarkins]

That sounds like the right fix to me. The problem wasn't adding the optional: true lines, it was removing them. So if the Mac npm no longer removes them then it should stop the flapping once they are all now added back.

If you see Renovate adding in http:// URLs then let me know. I'd hoped that by following the approaches recommended by npm (e.g. no node_modules, etc) then we should be set, even if developers themselves still need to take action.

[westonruter]

After updating locally to npm 6.7 the issue with optional is fixed, but I'm seeing something what may be similar where resolved is getting toggled between false and the URL for the package:

--- a/package-lock.json
+++ b/package-lock.json
@@ -3878,28 +3878,28 @@
       "dependencies": {
         "abbrev": {
           "version": "1.1.1",
-          "resolved": "https://registry.npmjs.org/abbrev/-/abbrev-1.1.1.tgz",
+          "resolved": false,
           "integrity": "sha512-nne9/IiQ/hzIhY6pdDnbBtz7DjPTKrY00P/zvPSm5pOFkl6xuGrGnXn/VtTNNfNtAfZ9/1RtehkszU9qcTii0Q==",
           "dev": true,
           "optional": true
         },
         "ansi-regex": {
           "version": "2.1.1",
-          "resolved": "https://registry.npmjs.org/ansi-regex/-/ansi-regex-2.1.1.tgz",
+          "resolved": false,
           "integrity": "sha1-w7M6te42DYbg5ijwRorn7yfWVN8=",
           "dev": true,
           "optional": true
         },
         "aproba": {
           "version": "1.2.0",
-          "resolved": "https://registry.npmjs.org/aproba/-/aproba-1.2.0.tgz",
+          "resolved": false,
           "integrity": "sha512-Y9J6ZjXtoYh8RnXVCMOU/ttDmk1aBjunq9vO0ta5x85WDQiQfUF9sIPBITdbiiIVcBo03Hi3jMxigBtsddlXRw==",
           "dev": true,
           "optional": true
         },
         "are-we-there-yet": {
           "version": "1.1.5",
-          "resolved": "https://registry.npmjs.org/are-we-there-yet/-/are-we-there-yet-1.1.5.tgz",
+          "resolved": false,
           "integrity": "sha512-5hYdAkZlcG8tOLujVDTgCT+uPX0VnpAH28gWsLfzpXYm7wP6mp5Q/gYyR7YQ0cKVJcXJnl3j2kpBan13PtQf6w==",
           "dev": true,
           "optional": true,

The resolved is populated with URLs by renovate, but locally when doing npm install they are set to false.

[rarkins]

@westonruter could you copy paste this to a new issue? Btw Renovate is running npm 6.9.0 but soon switching to a mode where it will try to detect which version the project is using, and also allow config override of it too.