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

npm 'optional' fights #197

Closed
Floppy opened this issue Sep 27, 2018 · 13 comments
Closed

npm 'optional' fights #197

Floppy opened this issue Sep 27, 2018 · 13 comments

Comments

@Floppy
Copy link

Floppy commented Sep 27, 2018

I've noticed that we get a lot of change in our package-lock.json files around optional settings for various packages (see https://github.com/apolitical/styleguide/pull/122/files for an example).

I think it's caused by npm/npm#17722, which is that the file will be different if it's generated on MacOS or Linux, because of system dependencies of those packages.

We're a mixed-OS development team anyway, so we need to solve this for ourselves, but I wondered if you had any thought on somehow making Dependabot somehow ignore or respect the setting that's already in there.

/cc @huwd @Gisleburt @thethomaseffect

@prymitive
Copy link

Current ticket tracking this on npm side is https://npm.community/t/package-lock-json-keeps-changing-between-platforms-and-runs/1129 I think.
For me the solution was to pin npm version used in my projects to 5.6 (or rather nodejs to 8.11.4 since it bundles npm 5.6) - that way there's no diff between npm running on my laptop and on Travis CI. But since dependabot seems to be using 6.x it generates package-lock that includes all those extra changes.

@greysteil
Copy link
Contributor

Sorry for the slow reply on this one from me - I've been a bit rushed for the last couple of days.

This one is tricky. There might be something that Dependabot can do in post-processing lockfiles to avoid the npm bug, but it's dangerous for us to meddle with the lockfile too much (in case we introduce our own bugs).

I'm guessing there's a reason you're sticking to npm, rather than yarn?

@prymitive
Copy link

For me it was more like the lack of reason to use yarn, but I guess you have a point so I checked all the places I use npm and looks like I can just call yarn instead of npm everywhere (docker nodejs image & TravisCI), so the only cost is me installing brew so I can have yarn on macOS. Thanks!

@Floppy
Copy link
Author

Floppy commented Oct 5, 2018

Same, we've not needed to change yet, but perhaps it's sensible to do so.

@feelepxyz
Copy link
Contributor

@Floppy @prymitive just confirmed that this issue has been fixed in this pr: npm/cli#76
Discussed here: https://npm.community/t/package-lock-json-is-not-complete-on-first-run-for-some-modules/1817

If I understand this issue correctly, once this lands in npm optional: true will be added to all sub-dependencies that have an optional parent, currently it will only be set to optional if the parent is optional on the platform npm install runs on.

@Floppy
Copy link
Author

Floppy commented Oct 30, 2018

@feelepxyz superb! Thanks for the update :)

@feelepxyz
Copy link
Contributor

This should now be fixed in npm@6.6.0 🎉

You'll need to install the new version of npm to get consistent results from running npm install: npm install -g npm@6.6.0

The fix in npm: npm/cli@1342071

@roopakv
Copy link

roopakv commented Jan 20, 2020

We are recently seeing this issue, specifically when updating @storybook/react.

my local (mac) and my CI (linux) both are consistent, however dependabot is not. the diff i am seeing that dependabot is not adding id below

image

@feelepxyz
Copy link
Contributor

@roopakv this is usually because of differing npm versions and should be fixed in the latest version, which version are you running locally and on ci? To install the latest version: npm install -g npm@latest

@roopakv
Copy link

roopakv commented Jan 21, 2020

@feelepxyz Yeah I thought that was the case and it seems like we are running the same version everywhere. (atleast my CI and engineers' local) the problem is from dependabot

@roopakv
Copy link

roopakv commented Jan 27, 2020

@feelepxyz any update here. We are seeing this happen more frequently now.

@feelepxyz
Copy link
Contributor

@roopakv what version of npm are you running? I'm pretty sure it's caused by a version discrepancy as we worked on the patch in npm that fixed this problem by making it deterministic.

@roopakv
Copy link

roopakv commented Jan 28, 2020

@feelepxyz We use npm 6.13.6 everywhere.

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

No branches or pull requests

5 participants