Skip to content

types: add lockfileVersion 3 and the object form of overrides to BunLockFile - #38868

Open
robobun wants to merge 2 commits into
mainfrom
farm/de7835e8/bun-lock-types-v3
Open

types: add lockfileVersion 3 and the object form of overrides to BunLockFile#38868
robobun wants to merge 2 commits into
mainfrom
farm/de7835e8/bun-lock-types-v3

Conversation

@robobun

@robobun robobun commented Aug 15, 2026

Copy link
Copy Markdown
Collaborator

Problem

Fix

  • lockfileVersion becomes 0 | 1 | 2 | 3, with a JSDoc line per version. The versions are the Version enum in src/install/lockfile/bun.lock.rs (V3 is Version::CURRENT).
  • overrides values become string | BunLockFileScopedOverrides, a new Record<string, string> alias whose JSDoc explains the name / name@range keys and the "." entry. This is exactly what the writer emits (write_override_rules in bun.lock.rs) and what the parser accepts (the overrides loop in parse_into_binary_lockfile: a value is a string, or an object whose values are all strings), so the alias is as wide as the format and no wider. The object's values are plain string like the sibling Record<string, string> fields, so Object.entries on one keeps yielding strings.
  • The object form is deliberately not tied to lockfileVersion === 3 (no discriminated union): Stringifier::version_to_write keeps a lockfile at version 1 when one of its packages fails the version 2 checks and still writes the override objects, and the parser reads the objects at every version. A union keyed on the version would reject lockfiles Bun itself writes.
  • Out of scope, tracked elsewhere: the BunLockFilePackageArray tuples also lag the writer (a tarball entry may carry a trailing integrity string, a git or github entry may carry one as a fourth element), so a lockfile with such a dependency still needs the tuple changes in Document bun.lock format and complete BunLockFile tuple types #34204 to type-check as a whole. That PR rewrites the tuple union; this one only touches the two fields above and adds BunLockFileScopedOverrides, so the two do not overlap.
  • Verified with test/integration/bun-types/bun-types.test.ts, new case "BunLockFile accepts lockfileVersion 3 and the object form of overrides". It spawns tsc over a file that assigns the shapes above to BunLockFile, compares lockfileVersion === 3 on both a BunLockFile value and a ./bun.lock import, narrows an override value to the object form, and pins with @ts-expect-error that two-level nesting and a version outside the union are still rejected. Without the bun.d.ts change it fails with 9 diagnostics (the two quoted above plus TS2305 for the missing alias); with it the file type-checks cleanly.
    • USE_SYSTEM_BUN=1 bun test test/integration/bun-types/bun-types.test.ts: 16 pass (the fixture-based cases under every lib configuration still pass with the widened type). This is also what the bun-types GitHub workflow runs; that check is green on this PR.
    • bun bd test test/integration/bun-types/bun-types.test.ts: 4 pass, 12 skipped (the skipped ones are the release-only LanguageService cases; the new case runs on debug builds too).
  • The case is added as a sibling of the existing Bun.mmap spawned-tsc case rather than by extracting a shared helper: several open types PRs already rewrite that block in different ways, so this PR leaves it untouched.

Background

  • bun.lock is a JSONC file whose first key, lockfileVersion, gates compatibility: a Bun release that does not know the number refuses the file. Versions 1, 2 and 3 share the same content layout; 2 only turns on stricter parse checks, and 3 marks that overrides carries scoped rules.
  • Overrides normally map a package name to the specifier every dependency on it must use. Nested ("parent": { "child": "..." }, "parent>child") and version-scoped ("pkg@1") rules apply to one parent's dependency, or only to dependents declaring a matching range. In bun.lock every such rule is normalized to one object per selector (name or name@range); inside it, "." is the rule for the selected package itself and any other key is the rule for that dependency of it. Bun supports one level of nesting, so the object's values are always strings.
  • bun-types.test.ts is excluded from the BuildKite test lanes and runs in the bun-types GitHub workflow instead (triggered by changes under packages/bun-types and this test directory). Its fixture-based cases drive the TypeScript LanguageService in-process and are skipped on debug builds; spawning tsc over one file is the pattern the file already uses for a case that also has to run there.
Earlier revision

The first push extracted the Bun.mmap case's tsc spawning into a helper shared with the new case. That was dropped in favor of an additive block (see the last Fix bullet); the bun.d.ts change is unchanged.

…ockFile

bun install writes "lockfileVersion": 3 and object-valued overrides entries
for nested and version-scoped override rules. BunLockFile still declared
lockfileVersion as 0 | 1 | 2 and overrides as Record<string, string>, so
code that reads bun.lock through this type fails to compile on either.

Widen lockfileVersion to 0 | 1 | 2 | 3, type overrides values as
string | BunLockFileScopedOverrides (a one-level Record<string, string>
whose "." key is the selected package itself), and document the versions.
The object form is not tied to lockfileVersion 3 because the writer keeps a
lockfile at version 1 in some cases while still writing the objects, and
the parser accepts them at every version.

The test spawns tsc like the existing Bun.mmap case so it also runs on
debug builds, where the LanguageService-based fixture checks are skipped.
@robobun
robobun requested a review from alii as a code owner August 15, 2026 04:23
@coderabbitai

coderabbitai Bot commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 5 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 7aa1698a-4b2c-4ded-817f-f16c5f605002

📥 Commits

Reviewing files that changed from the base of the PR and between 3b38ea2 and de5c9f8.

📒 Files selected for processing (2)
  • packages/bun-types/bun.d.ts
  • test/integration/bun-types/bun-types.test.ts

Comment @coderabbitai help to get the list of available commands.

@robobun

robobun commented Aug 15, 2026

Copy link
Copy Markdown
Collaborator Author

Status: ready for review.

  • Reproduced by type-checking a file that types a bun.lock with "lockfileVersion": 3 and object-valued overrides as BunLockFile against the current bun.d.ts: TS2367 on lockfileVersion === 3, TS2322 on each object value.
  • The new case in test/integration/bun-types/bun-types.test.ts fails with those diagnostics without the bun.d.ts change and passes with it, under both USE_SYSTEM_BUN=1 bun test (16 pass) and bun bd test (4 pass, the release-only cases skipped). Re-verified after the second push, which only made the test additive (the Bun.mmap case is back to its original form).
  • Scope: lockfileVersion and overrides only. The package tuple shapes (trailing integrity on tarball and git/github entries) are Document bun.lock format and complete BunLockFile tuple types #34204's change and do not overlap with this one.
  • Types only; no runtime change. Reviews so far: one automated review, no changes requested; no open review threads.

@claude claude Bot 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.

LGTM — types-only update bringing BunLockFile in line with what bun install now writes.

What was reviewed:

  • Verified lockfileVersion: 0 | 1 | 2 | 3 matches the Version enum in src/install/lockfile/bun.lock.rs (CURRENT = V3).
  • Confirmed the */bun.lock module declaration in extensions.d.ts re-exports BunLockFile, so the test's import imported from "./bun.lock" is typed via the declaration (no file needed on disk).
  • Checked the tsc() helper extraction preserves the existing Bun.mmap test's assertions (trim moved into the helper, exitCode check unchanged).
Extended reasoning...

Overview

This PR widens two fields on the BunLockFile type in packages/bun-types/bun.d.ts: lockfileVersion gains | 3 and overrides values gain | BunLockFileScopedOverrides (a new Record<string, string> alias). It adds JSDoc for both, and adds a test case in test/integration/bun-types/bun-types.test.ts that spawns tsc over a fixture asserting the new shapes type-check and that version 4 / two-level nesting are still rejected via @ts-expect-error. The existing Bun.mmap test's tsc-spawn setup is extracted into a shared tsc() helper.

Security risks

None. This is a pure .d.ts change plus a test; no runtime code is touched.

Level of scrutiny

Low. Type declarations catching up to runtime reality that already shipped in #38333. I verified against the source of truth: src/install/lockfile/bun.lock.rs defines Version::{V0,V1,V2,V3} with CURRENT = V3, and the PR description's account of Stringifier::version_to_write (object overrides can appear at versions below 3, so no discriminated union) matches the code. The new BunLockFileScopedOverrides = Record<string, string> is exactly as wide as what the writer emits and the parser accepts.

Other factors

  • The test refactor is behavior-preserving: the mmap test's three assertions (stderr, stdout, exitCode) are unchanged, with .trim() simply moved into the helper's return.
  • The new test pins the negative contract (@ts-expect-error on lockfileVersion: 4 and on nested-object override values), so future accidental widening will fail CI.
  • The await using proc inside the extracted helper disposes at helper-scope exit, after proc.exited has already resolved — no lifetime issue.
  • No prior reviews from me on this PR; no outstanding reviewer comments.

@robobun

robobun commented Aug 15, 2026

Copy link
Copy Markdown
Collaborator Author
Updated 8:44 AM PT - Aug 15th, 2026

@robobun, your commit de5c9f87c57d48bd529aedcb229583dfd7e746c9 passed in Build #97804! 🎉


🧪   To try this PR locally:

bunx bun-pr 38868

That installs a local version of the PR into your bun-38868 executable, so you can run:

bun-38868 --bun

Leaves the existing spawned-tsc block as it is and adds the new case as a
sibling block, so this change stays additive in bun-types.test.ts.

@claude claude Bot 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.

I reviewed this and didn't find any bugs — the type widening matches Version::V3 = CURRENT and the object-valued override shape in src/install/lockfile/bun.lock.rs, and the new test pins both the accepted shapes and the rejected ones with @ts-expect-error. Deferring only because packages/bun-types/ is CODEOWNER-gated.

Checked that extensions.d.ts's */bun.lock module declaration flows through to the fixture's import imported from "./bun.lock".
Checked that BunLockFileScopedOverrides = Record<string, string> matches the one-level nesting the writer emits and the parser accepts.
Noted: the PR description mentions extracting a shared tsc() helper, but the final commit keeps the two cases separate (which is fine — just a stale description line).

Extended reasoning...

Overview

Types-only change in packages/bun-types/bun.d.ts: widens BunLockFile.lockfileVersion from 0 | 1 | 2 to 0 | 1 | 2 | 3 and widens overrides values from string to string | BunLockFileScopedOverrides (a new Record<string, string> alias). Adds JSDoc for each version and the scoped-overrides shape. A new test case in test/integration/bun-types/bun-types.test.ts spawns tsc over a fixture that exercises the new shapes and pins the negative cases (two-level nesting, version 4) with @ts-expect-error.

Security risks

None. This is a .d.ts change with no runtime effect.

Level of scrutiny

Low on correctness — I verified against src/install/lockfile/bun.lock.rs that Version::V3 is CURRENT and that write_override_rules emits the object form described. The test follows the exact pattern of the existing Bun.mmap case in the same file (spawn tsc, drain both pipes, assert empty output then exit code). The @ts-expect-error assertions guard against over-widening.

Other factors

packages/bun-types/ is listed in .github/CODEOWNERS, so I'm deferring for the owner's sign-off rather than approving. The robobun CI comment flags a failure on an earlier commit (9aedc80); the head commit de5c9f8 was pushed after that and only reworks the test, so worth confirming CI is green on head before merge. The PR description mentions extracting a shared tsc() helper that the final diff does not contain — the second commit message ("add the BunLockFile tsc case without touching the Bun.mmap case") explains the helper was dropped, so the code is consistent even if the description is slightly stale.

@robobun

robobun commented Aug 15, 2026

Copy link
Copy Markdown
Collaborator Author

The description now matches the final diff: the new case is a sibling of the existing Bun.mmap case, and the dropped helper is only mentioned under "Earlier revision". The bun.d.ts change is the same in both pushes; the TypeScript types check is green on de5c9f8 and the BuildKite build for it is in progress.

Sign up for free to 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.

1 participant