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

[Discussion] How do we handle package-lock changes? #1864

Closed
RunDevelopment opened this issue Apr 23, 2019 · 10 comments
Closed

[Discussion] How do we handle package-lock changes? #1864

RunDevelopment opened this issue Apr 23, 2019 · 10 comments

Comments

@RunDevelopment
Copy link
Member

RunDevelopment commented Apr 23, 2019

In #1862 it came up that the package-lock changed even though package.json didn't.

After a minute of googling, I found this. Basically: npm install will rewrite package-lock.json if newer versions of dependencies (matching the version pattern) are available.
This means that PR will modify the package-lock every time one of Prism's dependencies releases a new minor version.

So how do we want to handle this?

  • Just merge the new package-lock.
  • Don't commit package-lock.json into PRs.
  • Just use npm ci instead of npm install.
  • Fixed dependencies + regular updates.

@LeaVerou @mAAdhaTTah @Golmote


This only concerns the case where package-lock.json changes without changes to package.json.

@Golmote
Copy link
Contributor

Golmote commented Apr 23, 2019

How about using fixed versions for the dependencies? This would require us to manually check for updates regularly tho.

@mAAdhaTTah
Copy link
Member

I use Dependabot to keep the dependencies up to date on other projects. Could help here. If we keep everything up to date, then we're unlikely to get PRs with lock changes in the first place.

@RunDevelopment
Copy link
Member Author

Fixed version plus regular updates won't work, unfortunately.
If one of our dependencies doesn't use fixed versions, that means that the lock file will still change after one of the dependencies of our dependencies gets a new minor version.

@Golmote
Copy link
Contributor

Golmote commented Apr 23, 2019

What I meant was set the explicit version (up to the patch digit) on all our dependencies, without any prefix. This should seal the versions of the deps until we manually change them. Or am I missing something?

@mAAdhaTTah
Copy link
Member

Technically, tertiary dependencies aren't subject to the restriction you're suggesting, but the lock file should keep them in place. Which honestly, the lock file should keep everything in place, so I'm not entirely sure what the issue is here.

@Golmote
Copy link
Contributor

Golmote commented Apr 23, 2019

Apparently there's been a change in the behavior, which would explain why it didn't happen before. https://stackoverflow.com/a/45566871
If our dependencies don't change, their dependencies shouldn't change either I assume?

@mAAdhaTTah
Copy link
Member

@Golmote that was my understanding of the intended behavior. It took some time for them to settle on it and get it right. If contributors are using the latest npm version, we shouldn't see lock file changes. We should, if we're not, use npm ci in... ci.

@RunDevelopment
Copy link
Member Author

The reason why I think that the tertiary dependencies are updated even when using fixed versions is that in the package lock here none of our direct dependencies changed.
This is why I expect the same result for fixed versions.

@ExE-Boss
Copy link
Contributor

ExE-Boss commented May 31, 2019

The reason npm does this is because it produces a flat node_modules and has to keep package‑lock.json in sync with said node_modules.

We could switch to pnpm, which uses symlinks, hardlinks and isolated trees and therefore only updates pnpm‑lock.yaml when dependencies change and only the ones that actually changed.

@RunDevelopment
Copy link
Member Author

We hadn't had any PRs with package-lock.json changes for a while now, so I will close this issue.

Should this ever become relevant again, we can reopen this or open a new issue.

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

No branches or pull requests

4 participants