Skip to content
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.

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

lib: replace MessageEvent with undici's #52370

Merged
merged 1 commit into from
Apr 29, 2024
Merged
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
2 changes: 1 addition & 1 deletion lib/.eslintrc.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ rules:
- name: MessageChannel
message: Use `const { MessageChannel } = require('internal/worker/io');` instead of the global.
- name: MessageEvent
message: Use `const { MessageEvent } = require('internal/worker/io');` instead of the global.
message: Use `const { MessageEvent } = require('internal/deps/undici/undici');` instead of the global.
- name: MessagePort
message: Use `const { MessagePort } = require('internal/worker/io');` instead of the global.
- name: Navigator
Expand Down
4 changes: 2 additions & 2 deletions lib/internal/bootstrap/web/exposed-window-or-worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ defineLazyProperties(globalThis, 'buffer', ['atob', 'btoa']);
// https://html.spec.whatwg.org/multipage/web-messaging.html#broadcasting-to-other-browsing-contexts
exposeLazyInterfaces(globalThis, 'internal/worker/io', ['BroadcastChannel']);
exposeLazyInterfaces(globalThis, 'internal/worker/io', [
'MessageChannel', 'MessagePort', 'MessageEvent',
'MessageChannel', 'MessagePort',
]);
// https://www.w3.org/TR/FileAPI/#dfn-Blob
exposeLazyInterfaces(globalThis, 'internal/blob', ['Blob']);
Expand Down Expand Up @@ -89,7 +89,7 @@ installObjectURLMethods();
// https://fetch.spec.whatwg.org/#request-class
// https://fetch.spec.whatwg.org/#response-class
exposeLazyInterfaces(globalThis, 'internal/deps/undici/undici', [
'FormData', 'Headers', 'Request', 'Response',
'FormData', 'Headers', 'Request', 'Response', 'MessageEvent',
]);

// https://websockets.spec.whatwg.org/
Expand Down
107 changes: 7 additions & 100 deletions lib/internal/worker/io.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ const {
} = primordials;

const {
kEmptyObject,
kEnumerableProperty,
setOwnProperty,
} = require('internal/util');
Expand All @@ -38,7 +37,6 @@ const {
moveMessagePortToContext,
receiveMessageOnPort: receiveMessageOnPort_,
stopMessagePort,
checkMessagePort,
DOMException,
} = internalBinding('messaging');
const {
Expand All @@ -59,25 +57,19 @@ const {
const { inspect } = require('internal/util/inspect');
const {
codes: {
ERR_INVALID_ARG_TYPE,
ERR_INVALID_THIS,
ERR_MISSING_ARGS,
},
} = require('internal/errors');

const kData = Symbol('kData');
const kHandle = Symbol('kHandle');
const kIncrementsPortRef = Symbol('kIncrementsPortRef');
const kLastEventId = Symbol('kLastEventId');
const kName = Symbol('kName');
const kOrigin = Symbol('kOrigin');
const kOnMessage = Symbol('kOnMessage');
const kOnMessageError = Symbol('kOnMessageError');
const kPort = Symbol('kPort');
const kPorts = Symbol('kPorts');
const kWaitingStreams = Symbol('kWaitingStreams');
const kWritableCallbacks = Symbol('kWritableCallbacks');
const kSource = Symbol('kSource');
const kStartedReading = Symbol('kStartedReading');
const kStdioWantsMoreDataCallback = Symbol('kStdioWantsMoreDataCallback');
const kCurrentlyReceivingPorts =
Expand All @@ -93,6 +85,11 @@ const messageTypes = {
LOAD_SCRIPT: 'loadScript',
};

let messageEvent;
function lazyMessageEvent() {
return messageEvent ??= require('internal/deps/undici/undici').MessageEvent;
}

// We have to mess with the MessagePort prototype a bit, so that a) we can make
// it inherit from NodeEventTarget, even though it is a C++ class, and b) we do
// not provide methods that are not present in the Browser and not documented
Expand All @@ -119,95 +116,6 @@ MessagePort.prototype.hasRef = function() {
return !!FunctionPrototypeCall(MessagePortPrototype.hasRef, this);
};

function validateMessagePort(port, name) {
if (!checkMessagePort(port))
throw new ERR_INVALID_ARG_TYPE(name, 'MessagePort', port);
}

function isMessageEvent(value) {
return value != null && kData in value;
}

class MessageEvent extends Event {
constructor(type, {
data = null,
origin = '',
lastEventId = '',
source = null,
ports = [],
} = kEmptyObject) {
super(type);
this[kData] = data;
this[kOrigin] = `${origin}`;
this[kLastEventId] = `${lastEventId}`;
this[kSource] = source;
this[kPorts] = [...ports];

if (this[kSource] !== null)
validateMessagePort(this[kSource], 'init.source');
for (let i = 0; i < this[kPorts].length; i++)
validateMessagePort(this[kPorts][i], `init.ports[${i}]`);
}
}

ObjectDefineProperties(MessageEvent.prototype, {
data: {
__proto__: null,
get() {
if (!isMessageEvent(this))
throw new ERR_INVALID_THIS('MessageEvent');
return this[kData];
},
enumerable: true,
configurable: true,
set: undefined,
},
origin: {
__proto__: null,
get() {
if (!isMessageEvent(this))
throw new ERR_INVALID_THIS('MessageEvent');
return this[kOrigin];
},
enumerable: true,
configurable: true,
set: undefined,
},
lastEventId: {
__proto__: null,
get() {
if (!isMessageEvent(this))
throw new ERR_INVALID_THIS('MessageEvent');
return this[kLastEventId];
},
enumerable: true,
configurable: true,
set: undefined,
},
source: {
__proto__: null,
get() {
if (!isMessageEvent(this))
throw new ERR_INVALID_THIS('MessageEvent');
return this[kSource];
},
enumerable: true,
configurable: true,
set: undefined,
},
ports: {
__proto__: null,
get() {
if (!isMessageEvent(this))
throw new ERR_INVALID_THIS('MessageEvent');
return this[kPorts];
},
enumerable: true,
configurable: true,
set: undefined,
},
});

const originalCreateEvent = EventTarget.prototype[kCreateEvent];
ObjectDefineProperty(
MessagePort.prototype,
Expand All @@ -220,7 +128,7 @@ ObjectDefineProperty(
}
const ports = this[kCurrentlyReceivingPorts];
this[kCurrentlyReceivingPorts] = undefined;
return new MessageEvent(type, { data, ports });
return new (lazyMessageEvent())(type, { data, ports });
},
configurable: false,
writable: false,
Expand Down Expand Up @@ -413,7 +321,7 @@ function receiveMessageOnPort(port) {
}

function onMessageEvent(type, data) {
this.dispatchEvent(new MessageEvent(type, { data }));
this.dispatchEvent(new (lazyMessageEvent())(type, { data }));
}

function isBroadcastChannel(value) {
Expand Down Expand Up @@ -546,7 +454,6 @@ module.exports = {
moveMessagePortToContext,
MessagePort,
MessageChannel,
MessageEvent,
receiveMessageOnPort,
setupPortReferencing,
ReadableWorkerStdio,
Expand Down
9 changes: 1 addition & 8 deletions test/parallel/test-messageevent-brandcheck.js
Original file line number Diff line number Diff line change
@@ -1,21 +1,14 @@
// Flags: --expose-internals
'use strict';

require('../common');
const assert = require('assert');

const {
MessageEvent,
} = require('internal/worker/io');

[
'data',
'origin',
'lastEventId',
'source',
'ports',
].forEach((i) => {
assert.throws(() => Reflect.get(MessageEvent.prototype, i, {}), {
code: 'ERR_INVALID_THIS',
});
assert.throws(() => Reflect.get(MessageEvent.prototype, i, {}), TypeError);
});
18 changes: 9 additions & 9 deletions test/parallel/test-worker-message-event.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,31 +61,31 @@ const dummyPort = new MessageChannel().port1;
assert.throws(() => {
new MessageEvent('message', { source: 1 });
}, {
code: 'ERR_INVALID_ARG_TYPE',
message: /The "init\.source" property must be an instance of MessagePort/,
name: 'TypeError',
message: /Expected 1 to be an instance of MessagePort/,
});
assert.throws(() => {
new MessageEvent('message', { source: {} });
}, {
code: 'ERR_INVALID_ARG_TYPE',
message: /The "init\.source" property must be an instance of MessagePort/,
name: 'TypeError',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we are changing error code this becomes a semver-major

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@KhafraDev it would be better if this PR wouldn't be semver-major, so we can land it in v22.

message: /Expected {} to be an instance of MessagePort/,
});
assert.throws(() => {
new MessageEvent('message', { ports: 0 });
}, {
message: /ports is not iterable/,
message: /Sequence: Value of type Number is not an Object/,
});
assert.throws(() => {
new MessageEvent('message', { ports: [ null ] });
}, {
code: 'ERR_INVALID_ARG_TYPE',
message: /The "init\.ports\[0\]" property must be an instance of MessagePort/,
name: 'TypeError',
message: /Expected null to be an instance of MessagePort/,
});
assert.throws(() => {
new MessageEvent('message', { ports: [ {} ] });
}, {
code: 'ERR_INVALID_ARG_TYPE',
message: /The "init\.ports\[0\]" property must be an instance of MessagePort/,
name: 'TypeError',
message: /Expected {} to be an instance of MessagePort/,
});
}

Expand Down