Skip to content

Fix crash when constructing a jest.fn() mock #30641

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
robobun wants to merge 1 commit into from
Closed

Fix crash when constructing a jest.fn() mock #30641

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 50 additions & 5 deletions src/jsc/bindings/JSMockFunction.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@
}

JSC_DECLARE_HOST_FUNCTION(jsMockFunctionCall);
JSC_DECLARE_HOST_FUNCTION(jsMockFunctionConstruct);
JSC_DECLARE_CUSTOM_GETTER(jsMockFunctionGetter_protoImpl);
JSC_DECLARE_CUSTOM_GETTER(jsMockFunctionGetter_mock);
JSC_DECLARE_HOST_FUNCTION(jsMockFunctionGetter_mockGetLastCall);
Expand Down Expand Up @@ -462,7 +463,7 @@
}

JSMockFunction(JSC::VM& vm, JSC::Structure* structure, CallbackKind wrapKind)
: Base(vm, structure, jsMockFunctionCall, jsMockFunctionCall)
: Base(vm, structure, jsMockFunctionCall, jsMockFunctionConstruct)
{
initMock();
}
Expand Down Expand Up @@ -826,7 +827,8 @@
return result;
}

JSC_DEFINE_HOST_FUNCTION(jsMockFunctionCall, (JSGlobalObject * lexicalGlobalObject, CallFrame* callframe))
template<bool isConstructCall>
static JSC::EncodedJSValue jsMockFunctionCallImpl(JSGlobalObject* lexicalGlobalObject, CallFrame* callframe)
{
Zig::GlobalObject* globalObject = uncheckedDowncast<Zig::GlobalObject>(lexicalGlobalObject);
auto& vm = JSC::getVM(globalObject);
Expand All @@ -839,6 +841,39 @@

JSC::ArgList args = JSC::ArgList(callframe);
JSValue thisValue = callframe->thisValue();

if constexpr (isConstructCall) {
JSObject* newTargetObject = asObject(callframe->newTarget());
JSGlobalObject* functionGlobalObject = getFunctionRealm(globalObject, newTargetObject);
RETURN_IF_EXCEPTION(scope, {});
Structure* baseStructure = functionGlobalObject->objectStructureForObjectConstructor();
Structure* structure = newTargetObject == fn ? baseStructure : InternalFunction::createSubclassStructure(globalObject, newTargetObject, baseStructure);

Check failure on line 850 in src/jsc/bindings/JSMockFunction.cpp

View check run for this annotation

Claude / Claude Code Review

new fn() ignores user-assigned fn.prototype when newTarget === fn 

The `newTargetObject == fn` fast-path skips `createSubclassStructure` and hard-codes `objectStructureForObjectConstructor()` (i.e. `Object.prototype`), so a user-assigned `fn.prototype` is ignored when constructing directly via `new fn()` — yet honored via `Reflect.construct(fn, [], Other)`. Since `createSubclassStructure` already falls back to `baseStructure` when `newTarget.prototype` isn't an object, you can drop the ternary and call it unconditionally.

@claude claude Bot May 13, 2026

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.

🔴 The newTargetObject == fn fast-path skips createSubclassStructure and hard-codes objectStructureForObjectConstructor() (i.e. Object.prototype), so a user-assigned fn.prototype is ignored when constructing directly via new fn() — yet honored via Reflect.construct(fn, [], Other). Since createSubclassStructure already falls back to baseStructure when newTarget.prototype isn't an object, you can drop the ternary and call it unconditionally.

Extended reasoning...

What the bug is

Line 850 uses the standard JSC fast-path idiom:

Structure* structure = newTargetObject == fn
    ? baseStructure
    : InternalFunction::createSubclassStructure(globalObject, newTargetObject, baseStructure);

This pattern is correct for built-in constructors like Array or Map, where it relies on two invariants: (a) the constructor's .prototype is non-writable/non-configurable, and (b) baseStructure already has that .prototype baked in as its stored prototype. Neither invariant holds for JSMockFunction. As an InternalFunction, it has no default .prototype own-property (so users can freely assign one), and baseStructure here is functionGlobalObject->objectStructureForObjectConstructor(), whose stored prototype is Object.prototype — not anything the user set.

How it manifests

Assigning .prototype to a mock is a documented Jest pattern for mocking ES6 classes (see Jest's ES6 class mocks docs). In Jest, mocks are plain JS functions, so new fn() follows OrdinaryCreateFromConstructor and honors fn.prototype. With this PR's fast-path, Bun ignores it.

Why nothing prevents it

InternalFunction::createSubclassStructure does the right thing — it performs newTarget->get(vm.propertyNames->prototype) and uses the result as the instance's [[Prototype]], falling back to baseStructure only when .prototype is not an object. The bug is precisely that the newTargetObject == fn branch skips this lookup. The new test at mock-fn.test.js:842 ("Reflect.construct respects new.target prototype") only exercises the newTarget !== fn branch, so it passes; there's no test that sets fn.prototype and constructs directly.

Step-by-step proof

const fn = jest.fn();
fn.prototype = { greet() { return "hi"; } };
const inst = new fn();
  1. new fn() enters jsMockFunctionCallImpl<true> with callframe->newTarget() === fn.
  2. newTargetObject == fn is true, so structure = baseStructure = objectStructureForObjectConstructor().
  3. thisValue = constructEmptyObject(vm, structure) → an object whose [[Prototype]] is Object.prototype.
  4. No implementation returns an object, so constructorResult returns thisValue.
  5. Result: Object.getPrototypeOf(inst) === Object.prototype, and inst.greet is undefined.

Contrast with Reflect.construct(fn, [], class C {}): newTargetObject !== fn, so createSubclassStructure reads C.prototype and the instance correctly inherits from it — the asymmetry confirmed by the new test.

Impact

Users mocking classes by assigning Mock.prototype.method = jest.fn() and then doing new Mock() will get instances that don't inherit those methods, diverging from Jest. instanceof checks against fn will also fail (inst instanceof fn → false even though fn.prototype is set).

Fix

Drop the ternary:

Structure* structure = InternalFunction::createSubclassStructure(globalObject, newTargetObject, baseStructure);

createSubclassStructure already returns baseStructure when newTarget.prototype is not an object (the default case for a fresh jest.fn()), so this is both a correctness fix and a simplification with negligible cost.

RETURN_IF_EXCEPTION(scope, {});
thisValue = constructEmptyObject(vm, structure);

JSC::JSArray* instancesArray = fn->instances.get();
if (instancesArray) {
instancesArray->push(globalObject, thisValue);
RETURN_IF_EXCEPTION(scope, {});
} else {
JSC::ObjectInitializationScope object(vm);
instancesArray = JSC::JSArray::tryCreateUninitializedRestricted(
object,
globalObject->arrayStructureForIndexingTypeDuringAllocation(JSC::ArrayWithContiguous),
1);
instancesArray->initializeIndex(object, 0, thisValue);
fn->instances.set(vm, fn, instancesArray);
}

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

View check run for this annotation

Claude / Claude Code Review

mock.instances not populated on non-construct calls (Jest divergence) 

In Jest, `mock.instances` is pushed on **every** invocation (in lockstep with `mock.calls`/`mock.contexts`), not only on `[[Construct]]` — see `jest-mock`'s wrapper, which runs `mockState.instances.push(this)` unconditionally. With this change, mixing `new fn()` and `fn()` produces arrays where `instances[i]` no longer corresponds to `calls[i]`, and the new test "calling as a function does not populate instances" will fail when this file is run under Jest (the file header says it's meant to be c
Comment on lines +854 to +866

@claude claude Bot May 13, 2026

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.

🟡 In Jest, mock.instances is pushed on every invocation (in lockstep with mock.calls/mock.contexts), not only on [[Construct]] — see jest-mock's wrapper, which runs mockState.instances.push(this) unconditionally. With this change, mixing new fn() and fn() produces arrays where instances[i] no longer corresponds to calls[i], and the new test "calling as a function does not populate instances" will fail when this file is run under Jest (the file header says it's meant to be cross-runtime). Consider pushing thisValue to instances unconditionally to match jest-mock, or at minimum gate that last test with if (isBun).

Extended reasoning...

What the bug is

Jest's mock-function wrapper (in packages/jest-mock/src/index.ts) executes, on every invocation:

mockState.instances.push(this);
mockState.contexts.push(this);
mockState.calls.push(args);

All three arrays therefore grow in lockstep and fn.mock.instances.length === fn.mock.calls.length === fn.mock.contexts.length always holds. After this PR, Bun pushes to instances only inside the if constexpr (isConstructCall) block in jsMockFunctionCallImpl, while calls and contexts are still pushed unconditionally. So a regular (non-construct) call grows calls/contexts but leaves instances untouched.

How it manifests / step-by-step proof

const fn = jest.fn();
new fn();   // call 0: construct
fn();       // call 1: regular
new fn();   // call 2: construct

Jest:

  • fn.mock.calls.length === 3
  • fn.mock.contexts.length === 3
  • fn.mock.instances.length === 3 → [obj0, undefined, obj2]
  • fn.mock.instances[2] corresponds to fn.mock.calls[2].

Bun after this PR:

  • fn.mock.calls.length === 3
  • fn.mock.contexts.length === 3
  • fn.mock.instances.length === 2 → [obj0, obj2]
  • fn.mock.instances[2] is undefined and fn.mock.instances[1] is the instance from call 2, not call 1.

Any Jest-compatible test that indexes these arrays in parallel (e.g. expect(fn.mock.instances[i]).toBe(fn.mock.contexts[i])) will see misaligned data.

Why existing code doesn't prevent it

The instances push is guarded by if constexpr (isConstructCall) (JSMockFunction.cpp:845–867), whereas the contexts push a few lines below runs in both branches. Nothing else in the function adds an entry to instances for non-construct calls, so the arrays diverge by design.

Impact

  1. Index misalignment between mock.instances and mock.calls/mock.contexts when construct and regular calls are mixed — a behavioral divergence from Jest that can break ported test suites.
  2. The new test breaks the file's cross-runtime contract. mock-fn.test.js opens with "This file is meant to be runnable in Jest, Vitest, and Bun", but the new test "calling as a function does not populate instances" asserts expect(fn.mock.instances).toHaveLength(0) after a regular call. Under real Jest that array has length 1 (containing undefined), so running this file in Jest will fail.

Note that pre-PR, instances was always empty, which was also divergent from Jest — so the C++ change here is a strict improvement. The issue is that it goes only halfway, and the accompanying test codifies the remaining divergence as expected behavior.

How to fix

Either:

  • Match jest-mock fully: move the instances push out of the if constexpr (isConstructCall) block and push thisValue unconditionally (just like contexts). Then change the last test to expect toHaveLength(1) / [undefined].
  • Or, keep the current C++ but fix the test file: wrap "calling as a function does not populate instances" in if (isBun) { … } so the file remains green when run under Jest/Vitest, consistent with how other Bun-specific behavior in this file is gated.

}

auto constructorResult = [&](JSValue returnValue) -> JSValue {
if constexpr (isConstructCall) {
if (!returnValue.isObject())
return thisValue;
}
return returnValue;
};

JSC::JSArray* argumentsArray = nullptr;
{
JSC::ObjectInitializationScope object(vm);
Expand Down Expand Up @@ -954,12 +989,12 @@
fn->returnValues.set(vm, fn, returnValuesArray);
}

return JSValue::encode(returnValue);
return JSValue::encode(constructorResult(returnValue));
}
case JSMockImplementation::Kind::ReturnValue: {
JSValue returnValue = impl->underlyingValue.get();
setReturnValue(createMockResult(vm, globalObject, "return"_s, returnValue));
return JSValue::encode(returnValue);
return JSValue::encode(constructorResult(returnValue));
}
case JSMockImplementation::Kind::ReturnThis: {
setReturnValue(createMockResult(vm, globalObject, "return"_s, thisValue));
Expand All @@ -978,7 +1013,17 @@
}

setReturnValue(createMockResult(vm, globalObject, "return"_s, jsUndefined()));
return JSValue::encode(jsUndefined());
return JSValue::encode(constructorResult(jsUndefined()));
}

JSC_DEFINE_HOST_FUNCTION(jsMockFunctionCall, (JSGlobalObject * lexicalGlobalObject, CallFrame* callframe))
{
return jsMockFunctionCallImpl<false>(lexicalGlobalObject, callframe);
}

JSC_DEFINE_HOST_FUNCTION(jsMockFunctionConstruct, (JSGlobalObject * lexicalGlobalObject, CallFrame* callframe))
{
return jsMockFunctionCallImpl<true>(lexicalGlobalObject, callframe);
}

void JSMockFunctionPrototype::finishCreation(JSC::VM& vm, JSC::JSGlobalObject* globalObject)
Expand Down
57 changes: 57 additions & 0 deletions test/js/bun/test/mock-fn.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -794,6 +794,63 @@ describe("mock()", () => {

expect(bar()()).toBe(true);
});

describe("as a constructor", () => {
test("Reflect.construct on jest.fn() with no implementation", () => {
const fn = jest.fn();
const inst = Reflect.construct(fn, []);
expect(typeof inst).toBe("object");
expect(inst).not.toBeNull();
expect(fn.mock.instances).toHaveLength(1);
expect(fn.mock.instances[0]).toBe(inst);
});

test("Reflect.construct on jest.fn() with mockReturnValue(primitive)", () => {
const fn = jest.fn().mockReturnValue("hello");
const inst = Reflect.construct(fn, []);
expect(typeof inst).toBe("object");
expect(fn()).toBe("hello");
});

test("Reflect.construct on jest.fn() with mockReturnValue(object)", () => {
const obj = { a: 1 };
const fn = jest.fn().mockReturnValue(obj);
const inst = Reflect.construct(fn, []);
expect(inst).toBe(obj);
});

test("Reflect.construct on jest.fn() with implementation returning a primitive", () => {
const fn = jest.fn(() => 42);
const inst = Reflect.construct(fn, []);
expect(typeof inst).toBe("object");
expect(fn()).toBe(42);
});

test("new on jest.fn() returns an object and records the instance", () => {
const fn = jest.fn(function () {
this.x = 123;
});
const inst = new fn();
expect(typeof inst).toBe("object");
expect(inst.x).toBe(123);
expect(fn.mock.instances).toHaveLength(1);
expect(fn.mock.instances[0]).toBe(inst);
expect(fn.mock.contexts[0]).toBe(inst);
});

test("Reflect.construct respects new.target prototype", () => {
class MyClass {}
const fn = jest.fn();
const inst = Reflect.construct(fn, [], MyClass);
expect(inst instanceof MyClass).toBe(true);
});

test("calling as a function does not populate instances", () => {
const fn = jest.fn().mockReturnValue(1);
expect(fn()).toBe(1);
expect(fn.mock.instances).toHaveLength(0);
});
});
});

describe("spyOn", () => {
Expand Down
Loading