Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 7 additions & 2 deletions src/js/builtins.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -765,14 +765,19 @@ declare function $ERR_HTTP2_PING_CANCEL(): Error;
*
* This does:
* - Sets the name of the function to the given name
* - Sets .prototype to Object.create(base?.prototype, { constructor: { value: fn } })
* - Defines .prototype with the spec's descriptor for a function-style class
* ({ writable: true, enumerable: false, configurable: false }), using the
* passed `prototype` object, or creating one inheriting base?.prototype.
* A pre-existing own .prototype (e.g. a lazy accessor) is kept as-is.
* - Sets prototype.constructor to fn unless the object already has its own
* - Calls Object.setPrototypeOf(fn, base ?? Function.prototype)
*
* @param fn - The function to convert to a class
* @param name - The name of the class
* @param base - The base class to inherit from
* @param prototype - Use this object as the prototype instead of creating one
*/
declare function $toClass(fn: Function, name: string, base?: Function | undefined | null);
declare function $toClass(fn: Function, name: string, base?: Function | undefined | null, prototype?: object);

declare function $min(a: number, b: number): number;

Expand Down
3 changes: 2 additions & 1 deletion src/js/builtins/ConsoleObject.ts
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,8 @@ export function createConsoleConstructor(console: typeof globalThis.console) {
const kColorInspectOptions = { colors: true };
const kNoColorInspectOptions = {};

Object.defineProperties((Console.prototype = {}), {
$toClass(Console, "Console");
Object.defineProperties(Console.prototype, {
[kBindStreamsEager]: {
...consolePropAttributes,
// Eager version for the Console constructor
Expand Down
3 changes: 1 addition & 2 deletions src/js/builtins/shell.ts
Original file line number Diff line number Diff line change
Expand Up @@ -340,8 +340,7 @@ export function createBunShellTemplateFunction(createShellInterpreter_, createPa
return Shell;
}

Shell.prototype = ShellPrototype.prototype;
Object.setPrototypeOf(Shell, ShellPrototype);
$toClass(Shell, "Shell", ShellPrototype, ShellPrototype.prototype);
Object.setPrototypeOf(BunShell, ShellPrototype.prototype);

BunShell[cwdSymbol] = defaultCwd;
Expand Down
2 changes: 1 addition & 1 deletion src/js/internal/streams/readable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ function ReadableState(options, stream, isDuplex) {
this.encoding = options.encoding;
}
}
ReadableState.prototype = {};
$toClass(ReadableState, "ReadableState");
ObjectDefineProperties(ReadableState.prototype, {
objectMode: makeBitMapDescriptor(kObjectMode),
ended: makeBitMapDescriptor(kEnded),
Expand Down
2 changes: 1 addition & 1 deletion src/js/internal/streams/writable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ function makeBitMapDescriptor(bit) {
},
};
}
WritableState.prototype = {};
$toClass(WritableState, "WritableState");
ObjectDefineProperties(WritableState.prototype, {
// Object stream flag to indicate whether or not this stream
// contains buffers or objects.
Expand Down
2 changes: 1 addition & 1 deletion src/js/node/crypto.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ function Certificate(): void {
this.exportPublicKey = exportPublicKey;
this.exportChallenge = exportChallenge;
}
Certificate.prototype = {};
$toClass(Certificate, "Certificate");
Certificate.verifySpkac = verifySpkac;
Certificate.exportPublicKey = exportPublicKey;
Certificate.exportChallenge = exportChallenge;
Expand Down
6 changes: 2 additions & 4 deletions src/js/node/events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,8 @@ function EventEmitter(opts) {
}
}
}
Object.defineProperty(EventEmitter, "name", { value: "EventEmitter", configurable: true });
const EventEmitterPrototype = (EventEmitter.prototype = {});
$toClass(EventEmitter, "EventEmitter");
const EventEmitterPrototype = EventEmitter.prototype;

EventEmitterPrototype.setMaxListeners = function setMaxListeners(n) {
validateNumber(n, "setMaxListeners", 0);
Expand All @@ -86,8 +86,6 @@ EventEmitterPrototype.setMaxListeners = function setMaxListeners(n) {
};
Object.defineProperty(EventEmitterPrototype.setMaxListeners, "name", { value: "setMaxListeners" });

EventEmitterPrototype.constructor = EventEmitter;

EventEmitterPrototype.getMaxListeners = function getMaxListeners() {
return _getMaxListeners(this);
};
Expand Down
30 changes: 24 additions & 6 deletions src/js/node/tty.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,16 @@ function ReadStream(fd): void {
// Only set isTTY to true if the fd is actually a TTY
this.isTTY = isatty(fd);
}
$toClass(ReadStream, "ReadStream", fs.ReadStream);

// Defined before $toClass so $toClass keeps this lazy accessor as the "prototype".
Object.defineProperty(ReadStream, "prototype", {
get() {
const Prototype = Object.create(fs.ReadStream.prototype);
Object.defineProperty(Prototype, "constructor", {
value: ReadStream,
writable: true,
enumerable: false,
configurable: true,
});

// Add ref/unref methods to make tty.ReadStream behave like Node.js
// where TTY streams have socket-like behavior
Expand Down Expand Up @@ -95,13 +100,20 @@ Object.defineProperty(ReadStream, "prototype", {
return this;
};

Object.defineProperty(ReadStream, "prototype", { value: Prototype });
// Once materialized, match the descriptor of a regular function's "prototype".
Object.defineProperty(ReadStream, "prototype", {
value: Prototype,
writable: true,
enumerable: false,
configurable: false,
});

return Prototype;
},
enumerable: true,
enumerable: false,
configurable: true,
});
$toClass(ReadStream, "ReadStream", fs.ReadStream);

function WriteStream(fd): void {
if (!(this instanceof WriteStream)) return new WriteStream(fd);
Expand All @@ -125,7 +137,13 @@ function WriteStream(fd): void {
Object.defineProperty(WriteStream, "prototype", {
get() {
const Real = fs.WriteStream.prototype;
Object.defineProperty(WriteStream, "prototype", { value: Real });
// Once materialized, match the descriptor of a regular function's "prototype".
Object.defineProperty(WriteStream, "prototype", {
value: Real,
writable: true,
enumerable: false,
configurable: false,
});

WriteStream.prototype._refreshSize = function () {
const oldCols = this.columns;
Expand Down Expand Up @@ -190,7 +208,7 @@ Object.defineProperty(WriteStream, "prototype", {

return Real;
},
enumerable: true,
enumerable: false,
configurable: true,
});

Expand Down
2 changes: 1 addition & 1 deletion src/js/node/url.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ function Url() {
this.path = null;
this.href = null;
}
Url.prototype = {};
$toClass(Url, "Url");

// Reference: RFC 3986, RFC 1808, RFC 2396

Expand Down
43 changes: 39 additions & 4 deletions src/jsc/bindings/ZigGlobalObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2756,6 +2756,7 @@ JSC_DEFINE_HOST_FUNCTION(jsFunctionToClass, (JSC::JSGlobalObject * globalObject,
auto target = callFrame->argument(0).toObject(globalObject);
auto name = callFrame->argument(1);
JSObject* base = callFrame->argument(2).getObject();
JSValue prototypeValue = callFrame->argument(3);
JSObject* prototypeBase = nullptr;
RETURN_IF_EXCEPTION(scope, encodedJSValue());

Expand All @@ -2774,14 +2775,48 @@ JSC_DEFINE_HOST_FUNCTION(jsFunctionToClass, (JSC::JSGlobalObject * globalObject,
}
}

JSObject* prototype = prototypeBase ? JSC::constructEmptyObject(globalObject, prototypeBase) : JSC::constructEmptyObject(globalObject);
// Builtin function declarations have no own "prototype" property, so one is
// created below. A `class` declaration already owns its prototype (holding the
// class body), and node:tty installs a lazy "prototype" accessor before calling
// $toClass; both of those are kept rather than overwritten.
JSC::PropertySlot prototypeSlot(target, JSC::PropertySlot::InternalMethodType::GetOwnProperty, nullptr);
bool hasOwnPrototype = target->methodTable()->getOwnPropertySlot(target, globalObject, vm.propertyNames->prototype, prototypeSlot);
RETURN_IF_EXCEPTION(scope, encodedJSValue());

prototype->structure()->setMayBePrototype(true);
prototype->putDirect(vm, vm.propertyNames->constructor, target, PropertyAttribute::DontEnum | 0);
if (!hasOwnPrototype) {
JSObject* prototype = prototypeValue.getObject();
if (!prototype) {
// No prototype object was passed: create one inheriting from the base's.
prototype = prototypeBase ? JSC::constructEmptyObject(globalObject, prototypeBase) : JSC::constructEmptyObject(globalObject);
RETURN_IF_EXCEPTION(scope, encodedJSValue());
}

// Transitions the structure rather than flagging it in place: the object may
// share its structure (object literals, the empty-object structure cache).
prototype->didBecomePrototype(vm);

// Keep a "constructor" the caller already defined (e.g. a shared class prototype).
bool hasOwnConstructor = prototype->hasOwnProperty(globalObject, vm.propertyNames->constructor);
RETURN_IF_EXCEPTION(scope, encodedJSValue());
if (!hasOwnConstructor) {
prototype->putDirect(vm, vm.propertyNames->constructor, target, PropertyAttribute::DontEnum | 0);
}

// A function's own "prototype" property is non-enumerable and non-configurable
// (writable for function-style constructors): https://tc39.es/ecma262/#sec-function-instances-prototype
target->putDirect(vm, vm.propertyNames->prototype, prototype, PropertyAttribute::DontEnum | PropertyAttribute::DontDelete | 0);
} else if (!prototypeSlot.isAccessor() && prototypeBase) {
// A `class` declaration keeps its own prototype (with the class body on it),
// but a bare `class X {}` given a base would not inherit from it. Wire the
// class prototype's [[Prototype]] to the base so instances inherit, mirroring
// `class X extends base {}`. An accessor prototype (node:tty) is left alone.
JSValue existingPrototype = target->getDirect(vm, vm.propertyNames->prototype);
if (JSObject* prototypeObject = existingPrototype.getObject()) {
prototypeObject->setPrototypeDirect(vm, prototypeBase);
}
}

target->setPrototypeDirect(vm, base);
target->putDirect(vm, vm.propertyNames->prototype, prototype, PropertyAttribute::DontEnum | 0);
target->putDirect(vm, vm.propertyNames->name, name, PropertyAttribute::DontEnum | 0);
Comment thread
robobun marked this conversation as resolved.

return JSValue::encode(jsUndefined());
Expand Down
76 changes: 76 additions & 0 deletions test/js/node/events/event-emitter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -912,3 +912,79 @@ test("getEventListeners", () => {
test("EventEmitter.name", () => {
expect(EventEmitter.name).toBe("EventEmitter");
});

// https://github.com/oven-sh/bun/issues/32160
describe("constructor 'prototype' property descriptor", () => {
test("EventEmitter.prototype is non-enumerable and non-configurable", () => {
expect(Object.getOwnPropertyDescriptor(EventEmitter, "prototype")).toEqual({
value: EventEmitter.prototype,
writable: true,
enumerable: false,
configurable: false,
});
});

test("copying EventEmitter's descriptors onto another function does not throw", () => {
const target = function () {};
Object.defineProperties(target, Object.getOwnPropertyDescriptors(EventEmitter));
expect(target.prototype).toBe(EventEmitter.prototype);
});

// The same bug applied to every builtin constructor whose prototype is
// assigned (or set up via $toClass) in builtin JS rather than created by
// a function declaration.
const constructors: [name: string, get: () => Function][] = [
["events.EventEmitter", () => require("node:events").EventEmitter],
["events.init", () => require("node:events").init],
["url.Url", () => require("node:url").Url],
["crypto.Certificate", () => require("node:crypto").Certificate],
["console.Console", () => require("node:console").Console],
["http.OutgoingMessage", () => require("node:http").OutgoingMessage],
["http.IncomingMessage", () => require("node:http").IncomingMessage],
["http.ClientRequest", () => require("node:http").ClientRequest],
["stream.Readable", () => require("node:stream").Readable],
["stream.Writable", () => require("node:stream").Writable],
["stream.Duplex", () => require("node:stream").Duplex],
["net.Socket", () => require("node:net").Socket],
["tty.ReadStream", () => require("node:tty").ReadStream],
["tty.WriteStream", () => require("node:tty").WriteStream],
["Bun.$.Shell", () => require("bun").$.Shell],
];

test.each(constructors)("%s matches a regular function's descriptor", (_name, get) => {
const ctor = get();
// Read .prototype first: tty's streams materialize theirs lazily.
const prototype = ctor.prototype;
const { value, ...attributes } = Object.getOwnPropertyDescriptor(ctor, "prototype")!;
expect(value).toBe(prototype);
expect(attributes).toEqual({
writable: true,
enumerable: false,
configurable: false,
});

// "name" has the spec's descriptor too (non-writable, unlike "prototype").
const { value: _value, ...nameAttributes } = Object.getOwnPropertyDescriptor(ctor, "name")!;
expect(nameAttributes).toEqual({
writable: false,
enumerable: false,
configurable: true,
});
});

// $toClass also fills in prototype.constructor the way a class declaration
// would. tty.WriteStream and Bun.$.Shell deliberately reuse prototype objects
// owned by other constructors, so they are excluded here.
const withOwnConstructor = constructors.filter(([name]) => name !== "tty.WriteStream" && name !== "Bun.$.Shell");

test.each(withOwnConstructor)("%s prototype.constructor matches Node's descriptor", (_name, get) => {
const ctor = get();
const { value, ...attributes } = Object.getOwnPropertyDescriptor(ctor.prototype, "constructor")!;
expect(value).toBe(ctor);
expect(attributes).toEqual({
writable: true,
enumerable: false,
configurable: true,
});
});
});
13 changes: 13 additions & 0 deletions test/js/node/perf_hooks/perf_hooks.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,19 @@ test("stubs", () => {
expect(perf.performance.eventLoopUtilization()).toBeObject();
});

// $toClass is used to wire PerformanceNodeTiming/PerformanceResourceTiming
// (class declarations) to PerformanceEntry as their base. The instance
// prototype chain must reach PerformanceEntry.prototype, and the class body's
// own getters must remain in place. https://github.com/oven-sh/bun/issues/32160
test("nodeTiming inherits from PerformanceEntry and keeps its class getters", () => {
const nodeTiming = perf.performance.nodeTiming;
expect(nodeTiming instanceof perf.PerformanceEntry).toBe(true);
expect(Object.getPrototypeOf(perf.PerformanceNodeTiming.prototype)).toBe(perf.PerformanceEntry.prototype);
expect(nodeTiming.name).toBe("node");
expect(nodeTiming.entryType).toBe("node");
expect(Object.getPrototypeOf(perf.PerformanceResourceTiming.prototype)).toBe(perf.PerformanceEntry.prototype);
});

test("doesn't throw", () => {
expect(() => performance.mark("test")).not.toThrow();
expect(() => performance.measure("test", "test")).not.toThrow();
Expand Down
Loading