Fix crash when constructing jest.fn() via Reflect.construct #31164
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| import { describe, expect, jest, spyOn, test } from "bun:test"; | ||
|
Contributor
There was a problem hiding this comment. 🟡 nit: Per CLAUDE.md (root and Extended reasoning...The repo's contributor guidelines explicitly call out test placement. Root The existing canonical file is There's also a concrete benefit beyond convention: Step-by-step:
The only minor friction is that the new file uses TypeScript annotations ( Fix: Move the |
||
|
|
||
| describe("Reflect.construct on mock functions", () => { | ||
| test("jest.fn() with no implementation", () => { | ||
| const fn = jest.fn(); | ||
| const result = Reflect.construct(fn, []); | ||
| expect(typeof result).toBe("object"); | ||
| expect(result).not.toBe(fn); | ||
| }); | ||
|
|
||
| test("jest.fn() with implementation returning a primitive", () => { | ||
| const fn = jest.fn(() => 42); | ||
| const result = Reflect.construct(fn, []); | ||
| expect(typeof result).toBe("object"); | ||
| }); | ||
|
|
||
| test("jest.fn() with implementation returning an object", () => { | ||
| const obj = { a: 1 }; | ||
| const fn = jest.fn(() => obj); | ||
| const result = Reflect.construct(fn, []); | ||
| expect(result).toBe(obj); | ||
| }); | ||
|
|
||
| test("jest.fn().mockReturnValue(primitive)", () => { | ||
| const fn = jest.fn().mockReturnValue(42); | ||
| const result = Reflect.construct(fn, []); | ||
| expect(typeof result).toBe("object"); | ||
| }); | ||
|
|
||
| test("jest.fn().mockReturnThis()", () => { | ||
| const fn = jest.fn().mockReturnThis(); | ||
| const result = Reflect.construct(fn, []); | ||
| expect(typeof result).toBe("object"); | ||
| }); | ||
|
|
||
| test("spyOn an undefined property", () => { | ||
| const obj: Record<string, unknown> = {}; | ||
| const spy = spyOn(obj, "x" as never); | ||
| const result = Reflect.construct(spy, []); | ||
| expect(typeof result).toBe("object"); | ||
| }); | ||
|
|
||
| test("with explicit newTarget", () => { | ||
| const fn = jest.fn(); | ||
| const result = Reflect.construct(fn, [], function () {}); | ||
| expect(typeof result).toBe("object"); | ||
| }); | ||
|
|
||
| test("implementation receives constructed this", () => { | ||
| const fn = jest.fn(function (this: { x: number }) { | ||
| this.x = 1; | ||
| }); | ||
| const result = Reflect.construct(fn, []); | ||
| expect(result).toEqual({ x: 1 }); | ||
| }); | ||
|
|
||
| test("new on jest.fn() returns an object", () => { | ||
| const fn = jest.fn(); | ||
| const result = new (fn as any)(); | ||
| expect(typeof result).toBe("object"); | ||
| expect(result).not.toBe(fn); | ||
| }); | ||
|
|
||
| test("regular call preserves this", () => { | ||
| const fn = jest.fn(function (this: unknown) { | ||
| return this; | ||
| }); | ||
| const obj = { fn }; | ||
| expect(obj.fn()).toBe(obj); | ||
| expect(fn.call(123 as any)).toBe(123); | ||
| }); | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔴 The synthesized
thisis created withconstructEmptyObject(globalObject), which always usesObject.prototypeand ignorescallframe->newTarget(). As a resultnew (jest.fn())() instanceof fnandReflect.construct(jest.fn(), [], Foo) instanceof Fooare bothfalse, whereas in Jest (and per ordinary [[Construct]]) they aretrue. Consider deriving the prototype fromnewTarget(e.g. viaInternalFunction::createSubclassStructure/getFunctionRealm, as done inNodeVMScript.cppand other native constructors here).Extended reasoning...
What the bug is
In the new construct path, the freshly-allocated
thisis created with the single-argument overloadJSC::constructEmptyObject(globalObject). That overload returns aJSFinalObjectwhose [[Prototype]] isglobalObject->objectPrototype()— i.e. plainObject.prototype. It never consultscallframe->newTarget().Per the ECMAScript [[Construct]] internal method (step OrdinaryCreateFromConstructor), the new object's [[Prototype]] must come from
newTarget.prototype. The PR description explicitly says the goal is to "match ordinary [[Construct]] semantics and Jest's behavior", but for the prototype chain it matches neither: in Jest, mock functions are real JS functions, sonew fn()goes through ordinary [[Construct]] and the instance inherits fromfn.prototype.Code path that triggers it
callframe->newTarget()is available here (this is the native-construct entry point) but is never read. Whether the call comes fromnew fn(),Reflect.construct(fn, []), orReflect.construct(fn, [], Foo), the resulting object's prototype is the same:Object.prototype.Why nothing else prevents it
The PR's own "with explicit newTarget" test only asserts
typeof result === 'object', which a plain{}satisfies. It would fail if it assertedresult instanceof newTarget— which is the observable contractnewTargetexists to provide. None of the other added tests check the prototype chain either.Step-by-step proof
In each case,
constructEmptyObject(globalObject)produced{ __proto__: Object.prototype }, soOrdinaryHasInstancewalks the chain, never findsfn.prototype/Foo.prototype, and returnsfalse.Impact
Not a crash and strictly better than the pre-PR behavior (which crashed/returned
undefined), but it's an observable correctness divergence in the exact lines this PR adds. Tests that mock a constructor and then assertexpect(new Mock()).toBeInstanceOf(Mock)— which pass under Jest — will fail. Methods placed onfn.prototypeare unreachable from constructed instances.How to fix
Use the same pattern this codebase already uses in ~15 native constructors (e.g.
NodeVMScript.cpp,JSX509Certificate.cpp,NapiClass.cpp): readcallframe->newTarget(), resolve its function realm, derive a structure viaInternalFunction::createSubclassStructure(falling back to a default ifnewTarget.prototypeisn't an object), and pass that structure toconstructEmptyObject. Thennew fn() instanceof fnandReflect.construct(fn, [], Foo) instanceof Foowill both hold, matching Jest.