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

Commit lockfile for consistent CI and installs #3100

Merged
merged 1 commit into from
Aug 30, 2024

Conversation

uncenter
Copy link
Contributor

@uncenter uncenter commented Nov 11, 2023

Lockfiles exist to guarantee consistency across installs, especially in CI tests! This is a fresh lockfile from a fresh clone and install, and all tests are passing.

@zachleat
Copy link
Member

What are the drawbacks to checking in the lockfile?

  • Minor updates in dependencies must be opted into at the project level, right?
  • Increase in source conflicts

Others?

@uncenter
Copy link
Contributor Author

Minor updates in dependencies must be opted into at the project level, right?

Can you elaborate?

Increase in source conflicts

Sure, but that's easily resolved and doesn't outweigh the benefits of consistent package versioning in different environments.

@Aankhen
Copy link

Aankhen commented Nov 14, 2023

What are the drawbacks to checking in the lockfile?

  • Minor updates in dependencies must be opted into at the project level, right?

  • Increase in source conflicts

Others?

That seems like a fair summary. I’d like to emphasize for clarity that all changes to dependencies must be explicit, whether minor or major, upgrade or downgrade, etc. I agree with uncenter that conflicts in lockfiles are generally trivial to resolve, especially outside a monorepo, but that’s certainly a non-zero cost that should be considered.

(As for the more general question, I’m entirely on the side of committing it.)

@zachleat
Copy link
Member

My primary concern is (unless I’m mistaken) that checking in a lockfile makes Eleventy the bottleneck when dependencies issue updates (especially for security reasons). Numerous times over the years we’ve had deps issue updates and they are subsequently automatically picked up via minor releases without any action on my part.

This is a huge benefit.

@zachleat zachleat added the needs-discussion Please leave your opinion! This request is open for feedback from devs. label Jun 10, 2024
@zachleat
Copy link
Member

#3293 (comment) is one such recent issue

@Aankhen
Copy link

Aankhen commented Jun 11, 2024

Numerous times over the years we’ve had deps issue updates and they are subsequently automatically picked up via minor releases without any action on my part.

I agree that that’s an advantage, but conversely, apart from introducing non-determinism, it adds a new vector for security issues in the form of malicious updates from the developers of those dependencies. That may not be a major concern, of course, since malicious developers are exceedingly unlikely compared to security issues that are fixed by later releases.

@zachleat
Copy link
Member

After a bit more research I learned that I am misinformed about one small but important point: the package-lock.json file is never part of the published package code so “Minor updates in dependencies must be opted into at the application level” is not accurate.

More accurately, minor updates in dependencies would need to be opted into in this repo only, which makes a lot more sense now that we have dependabot enabled here.

So, let’s roll with this for a bit and see how annoying it is 😅

@zachleat zachleat merged commit e20f58f into 11ty:main Aug 30, 2024
@zachleat
Copy link
Member

Shipping with 3.0.0-beta.2 and 3.0.0-alpha.19

@zachleat zachleat added this to the Eleventy 3.0.0 milestone Aug 30, 2024
@zachleat zachleat added enhancement and removed needs-discussion Please leave your opinion! This request is open for feedback from devs. labels Aug 30, 2024
@uncenter uncenter deleted the remove-lockfile-gitignore branch August 30, 2024 23:52
@paulshryock
Copy link
Contributor

paulshryock commented Sep 8, 2024

Yay for dependency hygiene.

The next step is to also pin (at least dev) dependencies, and then upgrade those pinned dependencies regularly on purpose (instead of by accident).

https://docs.renovatebot.com/dependency-pinning/#pinning-dependencies-and-lock-files

@uncenter
Copy link
Contributor Author

uncenter commented Sep 8, 2024

Big fan of Renovate myself! GitHub's Dependabot is the other option, though I found it cumbersome at times.

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

Successfully merging this pull request may close these issues.

4 participants