Skip to content

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

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
+64 −3
Closed

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

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
34 changes: 31 additions & 3 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,7 @@
return result;
}

JSC_DEFINE_HOST_FUNCTION(jsMockFunctionCall, (JSGlobalObject * lexicalGlobalObject, CallFrame* callframe))
static JSC::EncodedJSValue jsMockFunctionCallImpl(JSGlobalObject* lexicalGlobalObject, CallFrame* callframe, JSValue thisValue)
{
Zig::GlobalObject* globalObject = uncheckedDowncast<Zig::GlobalObject>(lexicalGlobalObject);
auto& vm = JSC::getVM(globalObject);
Expand All @@ -838,7 +839,6 @@
}

JSC::ArgList args = JSC::ArgList(callframe);
JSValue thisValue = callframe->thisValue();
JSC::JSArray* argumentsArray = nullptr;
{
JSC::ObjectInitializationScope object(vm);
Expand Down Expand Up @@ -981,6 +981,34 @@
return JSValue::encode(jsUndefined());
}

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

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

// [[Construct]] must return an object: create `this` from new.target's prototype,
// run the mock, and return the mock's result only if it is an object.
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 = JSC::constructEmptyObject(vm, structure);

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

View check run for this annotation

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
Comment on lines +996 to +1001

@claude claude Bot May 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.

🟡 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 .prototype object (as Jest's plain-JS mockConstructor gets automatically) so the default-new.target case wires up the prototype chain.

Extended reasoning...

What the bug is

jsMockFunctionConstruct derives the new instance's structure via InternalFunction::createSubclassStructure(lexicalGlobalObject, newTarget, functionGlobalObject->objectStructureForObjectConstructor()). When newTarget is the mock function itself (the ordinary new fn() case), createSubclassStructure reads newTarget.prototype. But JSMockFunction is an InternalFunction subclass whose create() only calls Base::finishCreation(vm) and sets m_originalName — it never does putDirectWithoutTransition(vm, vm.propertyNames->prototype, ...) the way other Bun InternalFunction constructors (e.g. JSBufferList, NodeVMScript, JSX509Certificate) do, and InternalFunction::finishCreation does not auto-create one. So fn.prototype is undefined.

Code path that triggers it

const fn = jest.fn();
const inst = new fn();
Object.getPrototypeOf(inst) === Object.prototype;   // true in Bun, false in Jest
inst instanceof fn;                                 // TypeError in Bun, true in Jest
expect(inst).toBeInstanceOf(fn);                    // throws in Bun, passes in Jest

Step-by-step:

  1. new fn() enters jsMockFunctionConstruct with callframe->newTarget() === fn.
  2. createSubclassStructure does newTarget->get(..., vm.propertyNames->prototype) → undefined (no own .prototype, nothing on JSMockFunctionPrototype / Function.prototype).
  3. Since the result is not an object, it returns the supplied base structure, objectStructureForObjectConstructor(), whose prototype is Object.prototype.
  4. constructEmptyObject produces thisObject with [[Prototype]] = Object.prototype.
  5. Later, inst instanceof fn runs OrdinaryHasInstance: step 4 reads fn.prototype → undefined; step 5 ("If P is not an Object, throw a TypeError") throws.

Why existing code doesn't prevent it

The construct callback correctly implements OrdinaryCreateFromConstructor (fall back to %Object.prototype% when newTarget.prototype is not an object), so it is spec-compliant given its inputs. The gap is upstream: unlike a plain JS function mockConstructor() {} — which is what Jest uses and which gets an auto-created .prototype object — JSMockFunction never installs one. The new test only checks typeof instance === "object" and Reflect.construct(fn5, [], Base) (where Base supplies the prototype), so the default-new.target prototype chain is never asserted.

Impact

The PR description motivates this change with "Jest's class-mocking pattern (const instance = new MockedClass())", but the canonical assertion for that pattern — expect(instance).toBeInstanceOf(MockedClass) — throws in Bun while passing in Jest/Vitest. This is a real Jest-compat gap that becomes reachable for the first time now that new fn() returns an object instead of undefined.

That said, the root cause is pre-existing: jest.fn().prototype was already undefined before this PR, and ({}) instanceof jest.fn() already threw TypeError. This PR strictly improves behavior (assertion crash / undefined → an object), and the construct callback itself is correct.

How to fix

Give each JSMockFunction a fresh .prototype object in JSMockFunction::create (e.g. function->putDirect(vm, vm.propertyNames->prototype, constructEmptyObject(globalObject), PropertyAttribute::DontEnum)), mirroring what JSC does for ordinary JS functions and what other Bun InternalFunction constructors do explicitly. With that in place, createSubclassStructure will pick up fn.prototype, Object.getPrototypeOf(new fn()) === fn.prototype, and instanceof / toBeInstanceOf work as in Jest. This is somewhat orthogonal to the construct callback added here, so it's reasonable as a follow-up rather than a blocker for this crash fix.


JSValue returnValue = JSValue::decode(jsMockFunctionCallImpl(lexicalGlobalObject, callframe, thisObject));
RETURN_IF_EXCEPTION(scope, {});
if (returnValue && returnValue.isObject()) {
return JSValue::encode(returnValue);
}

return JSValue::encode(thisObject);

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

View check run for this annotation

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.
Comment on lines +1001 to +1009

@claude claude Bot May 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.

🟡 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.

Extended reasoning...

What's missing

JSMockFunction declares an instances array (exposed as fn.mock.instances), lazily initializes it in getInstances(), clears it in clear(), and visits it for GC — but nothing in the file ever pushes to it. jsMockFunctionCallImpl pushes to calls, contexts, invocationCallOrder, and returnValues, but not instances. The new jsMockFunctionConstruct creates thisObject and runs the mock implementation against it, but likewise never appends thisObject to fn->instances.

How Jest behaves

In Jest, mockConstructor is a plain JS function that pushes this to mock.instances on every invocation (call or construct). The Jest docs describe mock.instances as "an array that contains all the object instances that have been instantiated from this mock function using new". So after const inst = new fn(), Jest guarantees fn.mock.instances[0] === inst. This is the primary documented use case for mock.instances.

Why it surfaces with this PR

This is strictly a pre-existing gap — on main, mock.instances is always an empty array regardless of how the mock is invoked. However, before this PR, new fn() either asserted in debug builds or returned undefined in release, so the construct path was effectively unusable and nobody could meaningfully reach for mock.instances. This PR makes new fn() return a real object for the first time and explicitly states it "matches how Jest mocks behave when constructed", which makes the empty mock.instances array much more visible.

Step-by-step example

With this PR applied:

const fn = jest.fn(function () { this.x = 42; });
const inst = new fn();
  1. jsMockFunctionConstruct runs, allocates thisObject from new.target.prototype.
  2. It calls jsMockFunctionCallImpl(..., thisObject), which pushes [ ] to fn->calls, thisObject to fn->contexts, an id to invocationCallOrder, and a result object to returnValues — but never touches fn->instances.
  3. The implementation returns undefined (not an object), so jsMockFunctionConstruct returns thisObject.
  4. inst is the constructed object, fn.mock.contexts[0] === inst ✅, but fn.mock.instances is still [] ❌. In Jest, fn.mock.instances[0] === inst.

Impact

Not a regression and not a crash — mock.contexts[0] already captures the same value, and the PR's stated goal (fixing the isCell() assertion) is fully achieved. But users who follow Jest's docs for testing class mocks (expect(fn.mock.instances[0]).toBe(instance)) will find an empty array.

Suggested fix

In jsMockFunctionConstruct, after creating thisObject (or after the call returns), push it to fn->instances the same way jsMockFunctionCallImpl pushes thisValue to fn->contexts. For full Jest parity, jsMockFunctionCallImpl would also push thisValue to instances on every call (Jest pushes this unconditionally), but at minimum the construct path should populate it. Fine as a follow-up if you'd rather keep this PR scoped to the assertion fix.

}

void JSMockFunctionPrototype::finishCreation(JSC::VM& vm, JSC::JSGlobalObject* globalObject)
{
Base::finishCreation(vm);
Expand Down
33 changes: 33 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 @@ -238,6 +238,39 @@ describe("mock()", () => {
const obj = { fn };
expect(obj.fn()).toBe(obj);
});
test("can be constructed with new", () => {
const fn = jest.fn();
const instance = new fn();
expect(typeof instance).toBe("object");
expect(instance).not.toBe(null);
expect(fn).toHaveBeenCalledTimes(1);
expect(fn.mock.results[0]).toEqual({
type: "return",
value: undefined,
});

// the implementation is called with `this` set to the newly created object
const fn2 = jest.fn(function () {
this.x = 42;
});
const instance2 = new fn2();
expect(instance2.x).toBe(42);
expect(fn2.mock.contexts[0]).toBe(instance2);

// a primitive return value from the implementation is ignored, like a plain constructor
const fn3 = jest.fn(() => 123);
expect(typeof new fn3()).toBe("object");

// an object return value becomes the result of `new`
const result = { a: 1 };
const fn4 = jest.fn(() => result);
expect(new fn4()).toBe(result);

// Reflect.construct with a different new.target respects its prototype
class Base {}
const fn5 = jest.fn();
expect(Reflect.construct(fn5, [], Base)).toBeInstanceOf(Base);
});
if (isBun) {
test("jest.fn(10) return value shorthand", () => {
expect(jest.fn(10)()).toBe(10);
Expand Down
Loading