Skip to content

Fix crash when constructing a mock function whose implementation returns a primitive#31375

Closed
robobun wants to merge 2 commits into
mainfrom
farm/4e0236f3/mock-construct-return-object
Closed

Fix crash when constructing a mock function whose implementation returns a primitive#31375
robobun wants to merge 2 commits into
mainfrom
farm/4e0236f3/mock-construct-return-object

Use jest.fn() in cross-runner mock construct test

85f9f8b
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed May 25, 2026 in 10m 50s

Code review found 1 potential issue

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

Details

Severity Count
🔴 Important 0
🟡 Nit 0
🟣 Pre-existing 1
Severity File:Line Issue
🟣 Pre-existing src/jsc/bindings/JSMockFunction.cpp:1008-1010 mock.instances not populated when constructing a mock

Annotations

Check notice on line 1010 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

Not a regression, but worth noting while we're here: `mock.instances` is still never populated — `fn->instances` is declared, cleared, GC-visited and exposed via `getInstances()`, but nothing ever pushes to it. In Jest, `new mockFn()` pushes the constructed `this` onto `mock.instances` (it's the canonical API for tracking instances), and the new construct path now has `thisObject` in hand, so this would be the natural place to wire it up. The new test asserts on `noImpl.mock.contexts[0]` rather