Conversation
WalkthroughAdds ChangesLegacy Jest Alias Deprecation
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
|
Updated 9:24 PM PT - Jun 30th, 2026
❌ @robobun, your commit 84ce8e3 has 1 failures in
🧪 To try this PR locally: bunx bun-pr 32905That installs a local version of the PR into your bun-32905 --bun |
|
Found 1 issue this PR may fix:
🤖 Generated with Claude Code |
|
This PR may be a duplicate of:
🤖 Generated with Claude Code |
There was a problem hiding this comment.
No issues found — this is a clean docs/types/tests-only change with no runtime impact, but since it adds @deprecated tags to public matcher types in packages/bun-types/ (CODEOWNER'd), I'll leave the final call to a human.
Extended reasoning...
Overview
This PR marks nine legacy Jest matcher aliases (toThrowError, toBeCalled, toBeCalledTimes, toBeCalledWith, lastCalledWith, nthCalledWith, toReturn, lastReturnedWith, nthReturnedWith) as @deprecated in packages/bun-types/test.d.ts, adds the three return-value aliases that were missing from the type definitions but already wired up at runtime (verified in src/runtime/test_runner/jest.classes.ts:342-366), fixes three incorrect @alias JSDoc tags, updates JSDoc examples to use canonical matcher names, adds a "Legacy Jest Aliases" table to docs/test/writing-tests.mdx, and adds a small test block in test/js/bun/test/expect.test.js exercising all nine aliases (correctly gated behind test_skipIf(isJest), both of which are already defined at the top of that file). There are no runtime code changes.
Security risks
None. The diff is confined to JSDoc comments, ambient type declarations, a markdown docs table, and additive test assertions. Nothing touches auth, crypto, network, filesystem, or any executable runtime path.
Level of scrutiny
Low-to-moderate. The mechanical content (JSDoc tags, docs table, tests) is straightforward and I verified the new type declarations match existing runtime aliases. However, packages/bun-types/ is CODEOWNER'd by alii per .github/CODEOWNERS, and adding @deprecated to widely-used matchers is a user-facing API signal (IDE strikethrough for everyone using toThrowError etc.). That's a reasonable product decision given Jest 30 dropped these, but it's the kind of public-API surface change the types owner should explicitly sign off on rather than have auto-approved.
Other factors
The bug-hunting system found no issues. The new tests follow existing patterns in the file (test_skipIf(isJest) is used elsewhere at lines 1340, 3566, 3782). The PR description notes overlap with #32335 which the author has already accounted for. Verification commands in the description show the type integration tests and expect tests passing.
8fdf9a1 to
aa3831f
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/js/bun/test/expect.test.js`:
- Around line 4903-4932: The new matcher alias coverage in expect.test.js only
checks runtime behavior, but the added bun-types declarations for aliases like
toReturn, lastReturnedWith, and nthReturnedWith are not covered by a type
fixture. Add a test/integration/bun-types fixture that exercises these
declarations through the public types contract so CI catches regressions or
renames. Use the existing bun-types integration setup to validate the matcher
aliases end-to-end alongside the relevant expect/Jest matcher symbols.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3a623b45-7853-4b56-be86-1ec93b403103
📒 Files selected for processing (3)
docs/test/writing-tests.mdxpackages/bun-types/test.d.tstest/js/bun/test/expect.test.js
There was a problem hiding this comment.
Changes look correct and my earlier docs nit was addressed, but packages/bun-types/ is CODEOWNER-protected so I'll leave the approval to a human.
Extended reasoning...
Overview
This PR adds @deprecated JSDoc tags to nine legacy Jest matcher aliases in packages/bun-types/test.d.ts, declares three previously-missing aliases (toReturn, lastReturnedWith, nthReturnedWith) that already exist at runtime in src/runtime/test_runner/jest.classes.ts, fixes a few incorrect @alias tags and JSDoc examples, adds a Legacy Jest Aliases table to docs/test/writing-tests.mdx, and adds runtime + type-fixture coverage. No runtime code changes.
Security risks
None. This is JSDoc, type declarations, documentation, and test additions only.
Level of scrutiny
The diff itself is mechanical and I verified the three newly-declared aliases match the runtime wiring in jest.classes.ts. However, packages/bun-types/ and *.d.ts are explicitly listed in .github/CODEOWNERS, and this PR changes the public bun-types API surface (adding three new matcher declarations and @deprecated tags that produce IDE strikethrough for end users). Per my guidelines I don't approve CODEOWNER-protected paths.
Other factors
Both review comments on this PR have been addressed: the bun-types integration fixture was added in c7f6248 (and verified load-bearing via a revert check), and my docs lead-in nit about scoping the alias table to Bun's supported set was fixed in dc4e2f3. The bug-hunting system found no issues. The one CI failure (sql-mysql.auth.test.ts) is unrelated to this diff. There's also overlap with #32335 noted in the description.
Jest 30 removed the alias matchers (toThrowError, toBeCalled, toBeCalledTimes, toBeCalledWith, lastCalledWith, nthCalledWith, toReturn, lastReturnedWith, nthReturnedWith). Bun still accepts them at runtime, so a suite using them passes under bun test but is a hard TypeError under Jest 30. Runtime support is kept (removal would be a breaking change and Vitest still accepts them), but each alias is now marked @deprecated in the bun:test type definitions with a pointer to the canonical matcher, and the matcher docs gain a table that lists the aliases explicitly as a deliberate superset of Jest 30. Also declares the three return-value aliases that were already wired up at runtime but missing from the types, fixes three copy/paste errors in existing @alias tags, and updates JSDoc examples that used toBeCalledWith to the canonical name. A describe block in expect.test.js exercises all nine aliases (skipped when the file is run under Jest).
The runtime test in expect.test.js proves the aliases work at runtime, but it type-checks against the in-repo module, not the packed bun-types tarball that the bun-types integration test validates. Add a fixture block so that removing or renaming any of the alias declarations (in particular toReturn, lastReturnedWith, and nthReturnedWith, which were newly declared in this PR) fails the type integration test. Before the test.d.ts change this fixture fails with: TS2339: Property 'toReturn' does not exist on type 'Matchers<...>' TS2339: Property 'lastReturnedWith' does not exist on type 'Matchers<...>' TS2551: Property 'nthReturnedWith' does not exist on type 'Matchers<...>'
The lead-in read as if the 9-row table were the complete list of aliases Jest 30 removed. Jest 30 removed 11; Bun does not implement toReturnTimes or toReturnWith, so reword the sentence to describe the aliases Bun supports rather than Jest's removal set.
8dc4cf8 to
84ce8e3
Compare
There was a problem hiding this comment.
Changes look correct and my earlier docs nit was addressed — deferring since packages/bun-types/ is CODEOWNER'd and adding @deprecated to nine public matchers is a types-surface decision worth an owner sign-off.
Extended reasoning...
Overview
This PR touches four files with no runtime code: packages/bun-types/test.d.ts (adds @deprecated/@alias JSDoc to six existing matcher aliases, adds three missing declarations for toReturn/lastReturnedWith/nthReturnedWith that are already wired at runtime in src/runtime/test_runner/jest.classes.ts, and swaps JSDoc examples from toBeCalledWith → toHaveBeenCalledWith), docs/test/writing-tests.mdx (new Legacy Jest Aliases table), test/js/bun/test/expect.test.js (runtime coverage using the file's existing test_skipIf(isJest) pattern), and test/integration/bun-types/fixture/test.ts (type-fixture coverage). Both prior review threads (my docs-wording nit and CodeRabbit's fixture request) are resolved.
Security risks
None. The diff is ambient .d.ts declarations, MDX prose, and test additions — nothing compiles into the binary and nothing touches auth, crypto, network, or filesystem paths.
Level of scrutiny
Correctness-wise this is low risk: the three new declarations match the runtime aliases in jest.classes.ts:342-366, the @deprecated tags are pure JSDoc hints (no tsc errors), and the tests follow established file conventions. However, packages/bun-types/ and *.d.ts are explicitly CODEOWNER'd in .github/CODEOWNERS, and this change takes a public API stance — soft-deprecating nine matchers will surface as IDE strikethroughs for every Bun user who uses them. That's the kind of types-surface decision the owner should sign off on rather than a bot.
Other factors
The bug-hunting pass found nothing. Test coverage is solid (both runtime and type-fixture, with the fixture verified load-bearing per the PR description's revert check). The PR description's CI analysis is thorough and the failing lanes are unrelated flake. I'd approve on correctness alone if not for the CODEOWNERS gate.
|
Closing as part of a cleanup of the overlapping bun-types PRs. The part of this PR that fixes #32334 (declaring |
Problem
Jest 30 removed the short-form matcher aliases (
toThrowError,toBeCalled,toBeCalledTimes,toBeCalledWith,lastCalledWith,nthCalledWith,toReturn,lastReturnedWith,nthReturnedWith). Bun still accepts them at runtime, so a suite using them passes underbun testbut fails withTypeError: expect(...).toThrowError is not a functionunder Jest 30. The Bun docs advertise Jest compatibility without saying which Jest's matcher set that means.Approach
Keep the aliases working at runtime: removing them would break existing Bun test suites, and Vitest still accepts them as well. Instead, make the divergence explicit.
packages/bun-types/test.d.ts: each alias is tagged@deprecatedwithUse {@link canonical} instead. Jest removed this alias in Jest 30; Bun keeps it for backward compatibility.This surfaces as IDE strikethrough and steers users to the name that works under Bun, Jest 30 and Vitest alike.packages/bun-types/test.d.ts: declarestoReturn,lastReturnedWith,nthReturnedWith, which were wired up at runtime injest.classes.tsbut missing from the type definitions (they now follow the same@deprecatedpattern). Also updates theexpect.any/expect.anything/arrayContainingJSDoc examples to stop showcasingtoBeCalledWith. (An earlier revision of this PR also fixed three mistargeted@aliastags onlastCalledWith/nthCalledWith;mainhas since fixed those independently in docs: editorial pass over docs/ and bun-types JSDoc #33112, so they are no longer part of this diff.)docs/test/writing-tests.mdx: adds a "Legacy Jest Aliases" table listing each alias with its canonical replacement. The lead-in is scoped to the aliases Bun supports rather than Jest's removal set, since Jest 30 removed 11 aliases and Bun implements neithertoReturnTimesnortoReturnWith.test/js/bun/test/expect.test.js: adds alegacy Jest matcher aliases (removed in Jest 30)describe block exercising all nine aliases at runtime, gated behindtest_skipIf(isJest)since the file is also runnable under Jest and Vitest.test/integration/bun-types/fixture/test.ts: exercises all nine aliases through the packedbun-typescontract so the type integration test fails if a declaration is removed or renamed.@deprecatedis a JSDoc hint rather than a tsc error, so using the aliases in the fixture compiles while still catching a regression.No runtime change.
Fixes #32334 (the three return-value aliases now have type declarations).
Overlap with #32335
#32335 adds plain declarations for the three return-value aliases. This PR adds the same three declarations plus the
@deprecatedtag and the other six aliases; whichever lands first, the other reduces to a trivial rebase.Verification
The bun-types fixture is load-bearing: with
packages/bun-types/test.d.tsreverted tomain, the type integration test fails with exactlyand passes again with the declarations restored.
Rebase
Rebased onto
mainafter #33112, an editorial pass overdocs/and thebun-typesJSDoc, landed and conflicted with this PR inpackages/bun-types/test.d.ts(six hunks). Resolved by keepingmain's editorial wording and reapplying this PR's changes on top: the canonical matcher name in the three asymmetric-matcher prose lines, the newtoReturndeclaration, and the@deprecatedtags. #33112 independently made the same fix to the three mistargeted@aliastags this PR had also fixed, so that part is no longer in the diff.docs/test/writing-tests.mdx,test/js/bun/test/expect.test.js, and the bun-types fixture merged cleanly. Re-verified after the rebase: the fullexpect.test.jspasses underbun bd test(409 pass, 0 fail, which now includes the testsmainadded to that file) and the bun-types integration type check passes.CI note
This PR changes four files, none of which is an input to the
bunbinary: a docs page, the ambientpackages/bun-types/test.d.ts, and two test files. The binary built from this branch is therefore identical in behavior to the onemainbuilds at the base commit (52a1ddf), so no runtime test behavior can differ frommain's.On the rebased base, build 67416 finished with 282 of 287 jobs passed. None of the failures names a file this PR touches:
test/js/node/test/parallel/test-net-connect-memleak.jsfailed on the two alpine x64 lanes. The same test is failing right now on two other unrelated PR builds (67413, 67410), so it is a property of currentmainon musl, not of this diff.test/napi/napi.test.tshas failed on every one of this PR's four CI runs, across three different trees and two bases, and also fails on other PRs' builds.test/js/bun/webview/webview-chrome.test.tssimilarly recurs across this PR's builds and other PRs'.test/js/bun/s3/s3.test.tsfailed with the S3 backend's ownInternalError: "We encountered an internal connectivity issue"response. The remaining entries are each a single test file on a single lane with an automatic retry.darwin 26 aarch64test shards failed with no failing test annotated at all; that same lane failed identically on this PR's three earlier builds, which ran different trees.Earlier, on the pre-rebase base, the same tree was run twice (65756, then 65816 via an empty retrigger commit) and produced different failure sets (8 failing files, then 6, with 3 in common): the signature of nondeterministic tests, since a regression caused by a commit fails the same files on every run of that commit.
Two GitHub Actions checks (
TypeScript types,bun-plugin-svelte) also fail onmainHEAD and are unrelated: the former is@typescript/native-previewnot resolving in the tsgo variant (thebasic type checksvariant, the one that exercisestest.d.tsand the new fixture, passes); the latter is a bundler output-format assertion.The diff itself is green and ready to merge.