Skip to content

bun:test: return an object when constructing a mock function - #31370

Closed
robobun wants to merge 1 commit into
mainfrom
farm/4e0236f3/fix-mock-construct
Closed

bun:test: return an object when constructing a mock function#31370
robobun wants to merge 1 commit into
mainfrom
farm/4e0236f3/fix-mock-construct

bun:test: return an object when constructing a mock function

50b36e2
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed May 24, 2026 in 16m 9s

Code review found 2 potential issues

Found 3 candidates, confirmed 2. See review comments for details.

Details

Severity Count
🔴 Important 0
🟡 Nit 2
🟣 Pre-existing 0
Severity File:Line Issue
🟡 Nit src/jsc/bindings/JSMockFunction.cpp:996-1001 new fn() instances get Object.prototype, so instanceof fn throws
🟡 Nit src/jsc/bindings/JSMockFunction.cpp:1001-1009 mock.instances not populated when constructing a mock

Annotations

Check warning on line 1001 in src/jsc/bindings/JSMockFunction.cpp

See this annotation in the file changed.

@claude claude / Claude Code Review

new fn() instances get Object.prototype, so instanceof fn throws

Now that `new fn()` returns an object, the common Jest assertion `expect(new fn()).toBeInstanceOf(fn)` is reachable — but it throws `TypeError` because `JSMockFunction` never installs a `.prototype` own property, so `createSubclassStructure` falls back to `Object.prototype` and `OrdinaryHasInstance` rejects the non-object `fn.prototype`. The missing `.prototype` is pre-existing and the construct callback itself is spec-correct, so this can be a follow-up: give each `JSMockFunction` a fresh `.pro

Check warning on line 1009 in src/jsc/bindings/JSMockFunction.cpp

See this annotation in the file changed.

@claude claude / Claude Code Review

mock.instances not populated when constructing a mock

Now that `new fn()` actually works, it might be worth pushing `thisObject` into `fn->instances` here so `fn.mock.instances[0]` is the constructed instance — that's the canonical use case for `mock.instances` in Jest. This is technically pre-existing (Bun has never populated `mock.instances` for any call), so feel free to defer to a follow-up, but flagging since the PR description says this matches how Jest mocks behave when constructed.