Skip to content

Fix assertion failure constructing jest.fn() via Reflect.construct #32667

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 assertion failure constructing jest.fn() via Reflect.construct #32667

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
25 changes: 24 additions & 1 deletion 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 @@ -981,6 +982,28 @@
return JSValue::encode(jsUndefined());
}

JSC_DEFINE_HOST_FUNCTION(jsMockFunctionConstruct, (JSGlobalObject * lexicalGlobalObject, CallFrame* callframe))
{
auto& vm = JSC::getVM(lexicalGlobalObject);
auto scope = DECLARE_THROW_SCOPE(vm);

JSObject* newTarget = asObject(callframe->newTarget());
JSGlobalObject* functionGlobalObject = getFunctionRealm(lexicalGlobalObject, newTarget);
RETURN_IF_EXCEPTION(scope, {});
Structure* structure = InternalFunction::createSubclassStructure(lexicalGlobalObject, newTarget, functionGlobalObject->objectStructureForObjectConstructor());
RETURN_IF_EXCEPTION(scope, {});
JSObject* thisObject = constructEmptyObject(vm, structure);

Check notice on line 995 in src/jsc/bindings/JSMockFunction.cpp

View check run for this annotation

Claude / Claude Code Review

Constructed mock instance prototype diverges from Jest 

Note (pre-existing, not blocking): `JSMockFunction` never installs an own `.prototype` property, so when `newTarget` is the mock itself `createSubclassStructure` reads `fn.prototype` as `undefined` and falls back to `Object.prototype`. The constructed instance therefore has `Object.getPrototypeOf(new fn()) === Object.prototype` (and `new fn() instanceof fn` throws), diverging from Jest where mocks are real JS functions. A follow-up adding a per-instance `.prototype` in `JSMockFunction::create()`
Comment on lines +993 to +995

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

🟣 Note (pre-existing, not blocking): JSMockFunction never installs an own .prototype property, so when newTarget is the mock itself createSubclassStructure reads fn.prototype as undefined and falls back to Object.prototype. The constructed instance therefore has Object.getPrototypeOf(new fn()) === Object.prototype (and new fn() instanceof fn throws), diverging from Jest where mocks are real JS functions. A follow-up adding a per-instance .prototype in JSMockFunction::create() would close the gap; the construct logic here would then work unchanged.

Extended reasoning...

Summary

jsMockFunctionConstruct derives the new instance's structure via InternalFunction::createSubclassStructure(lexicalGlobalObject, newTarget, functionGlobalObject->objectStructureForObjectConstructor()). That helper reads newTarget.prototype and only falls back to the supplied base structure when the property is not an object. JSMockFunction is an InternalFunction and never defines an own .prototype data property — there is no putDirect(vm.propertyNames->prototype, ...) anywhere in this file, no entry for it in JSMockFunctionPrototypeTableValues, and InternalFunction::finishCreation does not auto-create one (only JSFunction does that lazily). So in the common new fn() case where newTarget === fn, fn.prototype is undefined, the fallback path is taken, and the freshly allocated thisObject has %Object.prototype% as its [[Prototype]].

Step-by-step

  1. const fn = jest.fn(); — creates a JSMockFunction. No own prototype is installed, so fn.prototype === undefined.
  2. new fn() enters jsMockFunctionConstruct with newTarget = fn.
  3. createSubclassStructure calls newTarget->get(globalObject, vm.propertyNames->prototype) → undefined. Since that's not an object, it returns the base structure (objectStructureForObjectConstructor()).
  4. constructEmptyObject(vm, structure) produces a plain object whose [[Prototype]] is Object.prototype.
  5. The mock has no implementation, so jsMockFunctionCall returns undefined, result.isObject() is false, and thisObject is returned.
  6. Result: Object.getPrototypeOf(new fn()) === Object.prototype, and (new fn()) instanceof fn throws TypeError: instanceof called on an object with an invalid prototype property because fn.prototype is not an object.

In Jest, mock functions are ordinary JS functions, so each mock has its own .prototype object, Object.getPrototypeOf(new fn()) === fn.prototype, and (new fn()) instanceof fn === true. The new test in this PR only asserts typeof === "object", so it does not exercise this divergence.

Why this is pre-existing / a nit

The construct code added here is spec-correct: OrdinaryCreateFromConstructor mandates falling back to the realm's %Object.prototype% when newTarget.prototype is not an object. The root cause is the pre-existing lack of a per-instance .prototype on JSMockFunction — e.g. ({}) instanceof jest.fn() already threw TypeError before this PR, and jest.fn().prototype was already undefined. This PR simply makes the constructed instance observable for the first time (previously construction asserted/returned undefined), so the prototype-chain divergence is now newly visible, but it was not introduced here. The Reflect.construct(fn, [], SomeClass) case where newTarget is a real class with a .prototype already works correctly with this implementation.

Suggested follow-up

In JSMockFunction::create(), allocate a fresh prototype object and install it:

JSObject* prototype = constructEmptyObject(globalObject);
prototype->putDirect(vm, vm.propertyNames->constructor, function, PropertyAttribute::DontEnum);
function->putDirect(vm, vm.propertyNames->prototype, prototype, PropertyAttribute::DontEnum | PropertyAttribute::DontDelete);

Once that exists, createSubclassStructure will pick it up automatically and jsMockFunctionConstruct needs no changes. This is a Jest-compat improvement worth tracking separately; it should not block this crash fix.

callframe->setThisValue(thisObject);

EncodedJSValue encodedResult = jsMockFunctionCall(lexicalGlobalObject, callframe);
RETURN_IF_EXCEPTION(scope, {});
Comment on lines +995 to +999

@coderabbitai coderabbitai Bot Jun 24, 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.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Record constructed mocks in mock.instances.

The new construct path creates thisObject and records it as the call context via jsMockFunctionCall, but never appends it to fn->instances, so new mock() still leaves mock.mock.instances stale. Add the instance update before delegating to the shared call logic.

🤖 Prompt for 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.

In `@src/jsc/bindings/JSMockFunction.cpp` around lines 995 - 999, The construct
path in JSMockFunction::construct currently creates thisObject and passes it
through jsMockFunctionCall, but does not record the constructed instance in
fn->instances, leaving mock.mock.instances stale for new mock() calls. Update
the construct flow to append thisObject to the mock instances collection before
delegating to the shared call logic, keeping the existing jsMockFunctionCall and
callframe setup intact.


JSValue result = JSValue::decode(encodedResult);
if (result.isObject())
return encodedResult;
return JSValue::encode(thisObject);
}

void JSMockFunctionPrototype::finishCreation(JSC::VM& vm, JSC::JSGlobalObject* globalObject)
{
Base::finishCreation(vm);
Expand Down
22 changes: 22 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,28 @@ describe("mock()", () => {

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

it("Reflect.construct returns an object when the implementation does not", () => {
const noImpl = jest.fn();
expect(typeof Reflect.construct(noImpl, [])).toBe("object");
expect(typeof new noImpl()).toBe("object");

const primitiveImpl = jest.fn(() => 42);
expect(typeof Reflect.construct(primitiveImpl, [])).toBe("object");
expect(typeof new primitiveImpl()).toBe("object");

const returnsPrimitive = jest.fn().mockReturnValue(42);
expect(typeof Reflect.construct(returnsPrimitive, [])).toBe("object");
expect(typeof new returnsPrimitive()).toBe("object");
Comment on lines +800 to +809

@coderabbitai coderabbitai Bot Jun 24, 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.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Assert a non-null constructed object.

typeof ... === "object" also passes for null, which is still an invalid construct result. Capture each constructed value and assert it is not null before checking the object shape/prototype.

🤖 Prompt for 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.

In `@test/js/bun/test/mock-fn.test.js` around lines 800 - 809, The construction
checks in the mock function tests only verify typeof on the results from
Reflect.construct and new calls, which still passes for null. Update the
assertions around noImpl, primitiveImpl, and returnsPrimitive to first capture
the constructed values, then assert each value is not null before confirming it
is an object/has the expected prototype shape; use the existing jest.fn mock
names to locate the affected cases.


const sentinel = {};
const objectImpl = jest.fn(() => sentinel);
expect(Reflect.construct(objectImpl, [])).toBe(sentinel);
expect(new objectImpl()).toBe(sentinel);

expect(noImpl()).toBeUndefined();
expect(primitiveImpl()).toBe(42);
});
});

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