Skip to content

chore(deps): use package-lock - #650

Merged
avivkeller merged 1 commit into
mainfrom
package-lock
Mar 8, 2026
Merged

chore(deps): use package-lock#650
avivkeller merged 1 commit into
mainfrom
package-lock

Conversation

@avivkeller

@avivkelleravivkeller commented Mar 6, 2026

Copy link
Copy Markdown
Member

CopilotAI review requested due to automatic review settings March 6, 2026 20:46
@avivkeller
avivkeller requested a review from a team as a code ownerMarch 6, 2026 20:46
@vercel

vercelBot commented Mar 6, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

ProjectDeploymentActionsUpdated (UTC)
api-docs-toolingReadyReadyPreviewMar 6, 2026 8:46pm

Request Review

@codecov

codecovBot commented Mar 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.90%. Comparing base (57a7359) to head (409c37c).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #650 +/- ##
=======================================
Coverage 75.90% 75.90% =======================================
Files 145 145 Lines 13735 13735 Branches 992 992 =======================================
Hits 10426 10426 Misses 3303 3303 Partials 6 6 

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the project’s npm lockfile strategy by aligning the repo metadata/versioning with package-lock.json (vs. shrinkwrap-style behavior) to improve install determinism and avoid cross-platform install failures from platform-specific optional native bindings.

Changes:

  • Bump package version from 1.0.0 to 1.0.1.
  • Update package-lock.json to match the new version and refreshed lock metadata (including peer/optional markers).

Reviewed changes

Copilot reviewed 1 out of 2 changed files in this pull request and generated no comments.

FileDescription
package.jsonBumps package version to 1.0.1.
package-lock.jsonUpdates lockfile version fields and package entries/metadata to match 1.0.1 and current npm lock output.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@MattIPv4MattIPv4 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we do a review of all our dependencies and check we're comfortable with their semver ranges? This big change here, as you noted, is going to be that usage of this tooling is no longer pinned to the exact dependency versions we define, so we must be comfortable that the semver ranges we define for our dependencies will be safe and aren't going to cause issues for consumers of this tooling.

@avivkeller

Copy link
Copy Markdown
MemberAuthor

Can we do a review of all our dependencies and check we're comfortable with their semver ranges? This big change here, as you noted, is going to be that usage of this tooling is no longer pinned to the exact dependency versions we define, so we must be comfortable that the semver ranges we define for our dependencies will be safe and aren't going to cause issues for consumers of this tooling.

We should be fine as is. The only semver issue was Preact, which we fixed a few days ago.

@ovflowdovflowd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm stamping with an approval... for the sake of continuity. I won't even check what changed on package-lock as I'm scared.

This is on you Aviv to please verify this won't blow up the universe 😭

@avivkeller

Copy link
Copy Markdown
MemberAuthor

The package-lock.json had a few line changes, which were only:

  • Version bump to 1.0.1
  • A few deps changed to peer dependencies

@ovflowd

Copy link
Copy Markdown
Member

Just do it Aviv, and if you do it, do it fast 😆

@ovflowd

Copy link
Copy Markdown
Member

Like now

@ovflowd

Copy link
Copy Markdown
Member

Or maybe yesterday.

@MattIPv4MattIPv4 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm also okay with this, but we should be prepared to revert or fix-forward if we run into issues with dependencies not being pinned as we need them.

@MattIPv4

MattIPv4 commented Mar 6, 2026

Copy link
Copy Markdown
Member

Like now
Or maybe yesterday.

Smh, no respect for the 48hr PR grace period.

@MattIPv4

MattIPv4 commented Mar 6, 2026

Copy link
Copy Markdown
Member

We should be fine as is. The only semver issue was Preact, which we fixed a few days ago.

As Claudio said, trusting you on this. The only one that stood out to me was that we're on an rc version of rolldown.

@ovflowd

Copy link
Copy Markdown
Member

Like now

Or maybe yesterday.

Smh, no respect for the 48hr PR grace period.

So disrespectful right? 🤧😮‍💨

@ovflowd

Copy link
Copy Markdown
Member

We should be fine as is. The only semver issue was Preact, which we fixed a few days ago.

As Claudio said, trusting you on this. The only one that stood out to me was that we're on an rc version of rolldown.

On that I can share that rolldown is alpha an their rc's is their effort to become stable/get out of experimental. But agreed we should honestly speaking apply our dependency pinning strategy on our package.json as I said before.

@avivkeller would you mind first doing a PR that does that and then conversion PR?

@avivkeller

avivkeller commented Mar 7, 2026

Copy link
Copy Markdown
MemberAuthor

Sure! After my Slack discussion with @wraithgar, it's important to note that there is / might be an npm bug somewhere in the mix here, so we should also keep an eye out on that

@avivkeller

Copy link
Copy Markdown
MemberAuthor

I'm going to land this first, since there's not 100% consensus on how pinning should work, and this is blocking some work in nodejs/node.

@avivkeller
avivkeller merged commit 34c1e89 into mainMar 8, 2026
24 checks passed
@avivkeller
avivkeller deleted the package-lock branch March 8, 2026 13:43
@aduh95

Copy link
Copy Markdown
Contributor

@avivkeller added the blocked label on Mar 7

@avivkeller commented on Mar 8

I'm going to land this first, since there's not 100% consensus on how pinning should work, and this is blocking some work in nodejs/node.

The timeline here is hard to follow. What was it blocking exactly? My takeaway from the Slack thread is that the issue was not caused by shrinkwrap, was there some other context I'm missing? Also if there was no consensus on how pinning should work, shouldn't that mean we should have stick with shrinkwrap until a consensus has emerged?

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@avivkeller@ovflowd@MattIPv4@aduh95