From 3df8b5ce7d9844ac436ccc7d63d4d016272a7ebf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B2an?= Date: Sun, 26 Feb 2023 20:14:30 +0000 Subject: [PATCH 1/7] Maintain backwards compatibility with wire format from 4.3.0 --- karma.conf.js | 4 ++ package.json | 1 + src/comlink.ts | 13 +++++- src/protocol.ts | 43 +++++++++--------- tests/fixtures/attack-iframe.html | 4 +- tests/fixtures/oldcomlink.html | 29 ++++++++++++ tests/oldcomlink.test.js | 74 +++++++++++++++++++++++++++++++ 7 files changed, 143 insertions(+), 25 deletions(-) create mode 100644 tests/fixtures/oldcomlink.html create mode 100644 tests/oldcomlink.test.js diff --git a/karma.conf.js b/karma.conf.js index c7840f40..fdea0649 100644 --- a/karma.conf.js +++ b/karma.conf.js @@ -24,6 +24,10 @@ module.exports = function (config) { pattern: "dist/**/*.@(mjs|js)", included: false, }, + { + pattern: "node_modules/comlink/dist/esm/**/*.@(mjs|js)", + type: "module", + }, { pattern: "tests/*.test.js", type: "module", diff --git a/package.json b/package.json index 689d80b8..9b0b1167 100644 --- a/package.json +++ b/package.json @@ -31,6 +31,7 @@ "@rollup/plugin-typescript": "11.0.0", "chai": "^4.3.7", "conditional-type-checks": "1.0.6", + "comlink": "4.3.0", "husky": "8.0.3", "karma": "6.4.1", "karma-chai": "0.1.0", diff --git a/src/comlink.ts b/src/comlink.ts index 8896b71d..bc931664 100644 --- a/src/comlink.ts +++ b/src/comlink.ts @@ -303,10 +303,17 @@ export function expose( console.warn(`Invalid origin '${ev.origin}' for comlink proxy`); return; } - const { id, type, path } = { + const { id, type, path, wireType } = { path: [] as string[], - ...(ev.data as Message), + ...(ev.data as Message | WireValue), }; + + // We ignore messages that are not for this event listener. + // This only happen when two-way communication is used. + if (wireType) { + return; + } + const argumentList = (ev.data.argumentList || []).map(fromWireValue); let returnValue; try { @@ -568,6 +575,7 @@ function toWireValue(value: any): [WireValue, Transferable[]] { return [ { type: WireValueType.HANDLER, + wireType: true, name, value: serializedValue, }, @@ -578,6 +586,7 @@ function toWireValue(value: any): [WireValue, Transferable[]] { return [ { type: WireValueType.RAW, + wireType: true, value, }, transferCache.get(value) || [], diff --git a/src/protocol.ts b/src/protocol.ts index d3eb2692..7a0c1926 100644 --- a/src/protocol.ts +++ b/src/protocol.ts @@ -33,21 +33,23 @@ export interface Endpoint extends EventSource { } export const enum WireValueType { - RAW = "RAW", - PROXY = "PROXY", - THROW = "THROW", - HANDLER = "HANDLER", + RAW, + PROXY, + THROW, + HANDLER, } export interface RawWireValue { id?: string; type: WireValueType.RAW; + wireType?: true; value: {}; } export interface HandlerWireValue { id?: string; type: WireValueType.HANDLER; + wireType?: true; name: string; value: unknown; } @@ -57,48 +59,47 @@ export type WireValue = RawWireValue | HandlerWireValue; export type MessageID = string; export const enum MessageType { - GET = "GET", - SET = "SET", - APPLY = "APPLY", - CONSTRUCT = "CONSTRUCT", - ENDPOINT = "ENDPOINT", - RELEASE = "RELEASE", + GET, + SET, + APPLY, + CONSTRUCT, + ENDPOINT, + RELEASE, } -export interface GetMessage { +interface BaseMessage { id?: MessageID; + wireType?: undefined; +} + +export interface GetMessage extends BaseMessage { type: MessageType.GET; path: string[]; } -export interface SetMessage { - id?: MessageID; +export interface SetMessage extends BaseMessage { type: MessageType.SET; path: string[]; value: WireValue; } -export interface ApplyMessage { - id?: MessageID; +export interface ApplyMessage extends BaseMessage { type: MessageType.APPLY; path: string[]; argumentList: WireValue[]; } -export interface ConstructMessage { - id?: MessageID; +export interface ConstructMessage extends BaseMessage { type: MessageType.CONSTRUCT; path: string[]; argumentList: WireValue[]; } -export interface EndpointMessage { - id?: MessageID; +export interface EndpointMessage extends BaseMessage { type: MessageType.ENDPOINT; } -export interface ReleaseMessage { - id?: MessageID; +export interface ReleaseMessage extends BaseMessage { type: MessageType.RELEASE; } diff --git a/tests/fixtures/attack-iframe.html b/tests/fixtures/attack-iframe.html index e32c951b..1daa807c 100644 --- a/tests/fixtures/attack-iframe.html +++ b/tests/fixtures/attack-iframe.html @@ -6,8 +6,8 @@ // send back a message to modify the prototype parent.postMessage( { - type: "SET", - value: { type: "RAW", value: "x" }, + type: 1 /* MessageType.SET */, + value: { type: 0 /* WireValueType.RAW */, value: "x" }, path: ["__proto__", "foo"], }, "*" diff --git a/tests/fixtures/oldcomlink.html b/tests/fixtures/oldcomlink.html new file mode 100644 index 00000000..efb2ab39 --- /dev/null +++ b/tests/fixtures/oldcomlink.html @@ -0,0 +1,29 @@ + diff --git a/tests/oldcomlink.test.js b/tests/oldcomlink.test.js new file mode 100644 index 00000000..eb561980 --- /dev/null +++ b/tests/oldcomlink.test.js @@ -0,0 +1,74 @@ +/** + * Copyright 2017 Google Inc. All Rights Reserved. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import * as Comlink from "/base/dist/esm/comlink.mjs"; + +describe("Comlink across versions (4.3.0 with latest main)", function () { + let oldTruncateThreshold; + + beforeEach(function () { + oldTruncateThreshold = chai.config.truncateThreshold; + chai.config.truncateThreshold = 0; + this.ifr = document.createElement("iframe"); + this.ifr.sandbox.add("allow-scripts", "allow-same-origin"); + this.ifr.src = "/base/tests/fixtures/oldcomlink.html"; + document.body.appendChild(this.ifr); + return new Promise((resolve) => (this.ifr.onload = resolve)); + }); + + afterEach(function () { + this.ifr.remove(); + chai.config.truncateThreshold = oldTruncateThreshold; + }); + + it("can expose, wrap and transfer", async function () { + let notcalled = true; + let error = ""; + window.addEventListener("unhandledrejection", (ev) => { + notcalled = false; + error = ev.reason; + }); + + let maybecalled = false; + class Test { + maybe() { + maybecalled = true; + } + } + const channel = new MessageChannel(); + Comlink.expose(new Test(), channel.port1); + + const iframe = Comlink.windowEndpoint(this.ifr.contentWindow); + const proxy = Comlink.wrap(iframe); + + expect(await proxy.add(1, 3)).to.equal(4); + await proxy.callme(Comlink.transfer(channel.port2, [channel.port2])); + + expect(maybecalled).to.equal(true); + expect(error).to.equal(""); + expect(notcalled).to.equal(true); + }); + + it("works with custom handlers", async function () { + const iframe = Comlink.windowEndpoint(this.ifr.contentWindow); + const proxy = Comlink.wrap(iframe); + + Comlink.transferHandlers.set("pingpong", { + canHandle: (obj) => obj === "ping" || obj === "pong", + serialize: (obj) => [obj, []], + deserialize: (obj) => obj, + }); + + expect(await proxy.pong("ping")).to.equal("pong"); + }); +}); From 191bb15761e1f73da167c6f539e58bbf2654e484 Mon Sep 17 00:00:00 2001 From: Joan Varvenne Date: Tue, 7 Mar 2023 14:27:03 +0000 Subject: [PATCH 2/7] Support multiple format to allow communication with 4.3.0 or older The approach taken is a bit different from the previous commit. We now support type values to be either the format from 4.3.0 or the format introduced in 4.3.1. When communicating with 4.3.0, the only way to communicate is if it's done like this: // 4.3.0 ------------> latest const a = Comlink.wrap(ep) Comlink.expose(b, ep) This works because `a` has to initiate the conversation with `b` giving us a chance to always treat future messages as 4.3.0 ones. In particular we support proxy sent by `a`: a.registerCallback(Comlink.proxy(() => {})) But we also support sending MessagePort that can be later wrapped by the other side: a.callMeBack(Comlink.transfer(port, [port])) --- src/comlink.ts | 149 +++++++++++++----- src/protocol.ts | 52 ++++-- tests/fixtures/attack-iframe.html | 4 +- .../{oldcomlink.html => v430comlink.html} | 14 +- ...oldcomlink.test.js => v430comlink.test.js} | 78 +++++++-- 5 files changed, 222 insertions(+), 75 deletions(-) rename tests/fixtures/{oldcomlink.html => v430comlink.html} (64%) rename tests/{oldcomlink.test.js => v430comlink.test.js} (51%) diff --git a/src/comlink.ts b/src/comlink.ts index bc931664..9660f069 100644 --- a/src/comlink.ts +++ b/src/comlink.ts @@ -8,10 +8,13 @@ import { Endpoint, EventSource, Message, + V430MessageType, MessageType, PostMessageWithOrigin, WireValue, + V430WireValueType, WireValueType, + MessageTypeMap, } from "./protocol"; export type { Endpoint }; @@ -199,8 +202,11 @@ export interface TransferHandler { * Gets called to deserialize an incoming value that was serialized in the * other thread with this transfer handler (known through the name it was * registered under). + * + * @param value the serialized value to build a T from + * @param isV430 true if this was deserialized from a version of comlink older than 4.3.0 */ - deserialize(value: S): T; + deserialize(value: S, isV430: boolean): T; } /** @@ -214,9 +220,9 @@ const proxyTransferHandler: TransferHandler = { expose(obj, port1); return [port2, [port2]]; }, - deserialize(port) { + deserialize(port, isV430) { port.start(); - return wrap(port); + return wrap(port, undefined, isV430); }, }; @@ -303,45 +309,50 @@ export function expose( console.warn(`Invalid origin '${ev.origin}' for comlink proxy`); return; } - const { id, type, path, wireType } = { + const { id, type, path } = { path: [] as string[], ...(ev.data as Message | WireValue), }; - // We ignore messages that are not for this event listener. - // This only happen when two-way communication is used. - if (wireType) { - return; - } - + const isV430 = typeof type === "number"; const argumentList = (ev.data.argumentList || []).map(fromWireValue); + + markV430Ports(...argumentList); + let returnValue; try { const parent = path.slice(0, -1).reduce((obj, prop) => obj[prop], obj); const rawValue = path.reduce((obj, prop) => obj[prop], obj); switch (type) { + case V430MessageType.GET: case MessageType.GET: { returnValue = rawValue; } break; + case V430MessageType.SET: case MessageType.SET: { - parent[path.slice(-1)[0]] = fromWireValue(ev.data.value); + const value = fromWireValue(ev.data.value); + markV430Ports(value); + parent[path.slice(-1)[0]] = value; returnValue = true; } break; + case V430MessageType.APPLY: case MessageType.APPLY: { returnValue = rawValue.apply(parent, argumentList); } break; + case V430MessageType.CONSTRUCT: case MessageType.CONSTRUCT: { const value = new rawValue(...argumentList); returnValue = proxy(value); } break; + case V430MessageType.ENDPOINT: case MessageType.ENDPOINT: { const { port1, port2 } = new MessageChannel(); @@ -349,6 +360,7 @@ export function expose( returnValue = transfer(port1, [port1]); } break; + case V430MessageType.RELEASE: case MessageType.RELEASE: { returnValue = undefined; @@ -365,7 +377,7 @@ export function expose( return { value, [throwMarker]: 0 }; }) .then((returnValue) => { - const [wireValue, transferables] = toWireValue(returnValue); + const [wireValue, transferables] = toWireValue(returnValue, isV430); ep.postMessage({ ...wireValue, id }, transferables); if (type === MessageType.RELEASE) { // detach and deactive after sending release response above. @@ -378,10 +390,13 @@ export function expose( }) .catch((error) => { // Send Serialization Error To Caller - const [wireValue, transferables] = toWireValue({ - value: new TypeError("Unserializable return value"), - [throwMarker]: 0, - }); + const [wireValue, transferables] = toWireValue( + { + value: new TypeError("Unserializable return value"), + [throwMarker]: 0, + }, + isV430 + ); ep.postMessage({ ...wireValue, id }, transferables); }); } as any); @@ -394,11 +409,30 @@ function isMessagePort(endpoint: Endpoint): endpoint is MessagePort { return endpoint.constructor.name === "MessagePort"; } +function isEndpoint(endpoint: object): endpoint is Endpoint { + return "postMessage" in endpoint && "addEventListener" in endpoint && "removeEventListener" in endpoint; +} + +function markV430Ports(...args: any[]) { + for (const arg of args) { + if (isObject(arg) && isEndpoint(arg) && isMessagePort(arg)) { + v430Endpoints.add(arg); + } + } +} + function closeEndPoint(endpoint: Endpoint) { if (isMessagePort(endpoint)) endpoint.close(); } -export function wrap(ep: Endpoint, target?: any): Remote { +export function wrap( + ep: Endpoint, + target?: any, + isV430: boolean = false +): Remote { + if (isV430) { + v430Endpoints.add(ep); + } return createProxy(ep, [], target) as any; } @@ -408,9 +442,9 @@ function throwIfProxyReleased(isReleased: boolean) { } } -function releaseEndpoint(ep: Endpoint) { +function releaseEndpoint(ep: Endpoint, isV430: boolean) { return requestResponseMessage(ep, { - type: MessageType.RELEASE, + type: msgType(MessageType.RELEASE, isV430), }).then(() => { closeEndPoint(ep); }); @@ -427,6 +461,7 @@ interface FinalizationRegistry { } declare var FinalizationRegistry: FinalizationRegistry; +const v430Endpoints = new WeakSet(); const proxyCounter = new WeakMap(); const proxyFinalizers = "FinalizationRegistry" in globalThis && @@ -434,7 +469,8 @@ const proxyFinalizers = const newCount = (proxyCounter.get(ep) || 0) - 1; proxyCounter.set(ep, newCount); if (newCount === 0) { - releaseEndpoint(ep); + releaseEndpoint(ep, v430Endpoints.has(ep)); + v430Endpoints.delete(ep); } }); @@ -458,13 +494,14 @@ function createProxy( target: object = function () {} ): Remote { let isProxyReleased = false; + const isV430 = v430Endpoints.has(ep); const proxy = new Proxy(target, { get(_target, prop) { throwIfProxyReleased(isProxyReleased); if (prop === releaseProxy) { return () => { unregisterProxy(proxy); - releaseEndpoint(ep); + releaseEndpoint(ep, isV430); isProxyReleased = true; }; } @@ -473,7 +510,7 @@ function createProxy( return { then: () => proxy }; } const r = requestResponseMessage(ep, { - type: MessageType.GET, + type: msgType(MessageType.GET, isV430), path: path.map((p) => p.toString()), }).then(fromWireValue); return r.then.bind(r); @@ -484,11 +521,11 @@ function createProxy( throwIfProxyReleased(isProxyReleased); // FIXME: ES6 Proxy Handler `set` methods are supposed to return a // boolean. To show good will, we return true asynchronously ¯\_(ツ)_/¯ - const [value, transferables] = toWireValue(rawValue); + const [value, transferables] = toWireValue(rawValue, isV430); return requestResponseMessage( ep, { - type: MessageType.SET, + type: msgType(MessageType.SET, isV430), path: [...path, prop].map((p) => p.toString()), value, }, @@ -500,18 +537,21 @@ function createProxy( const last = path[path.length - 1]; if ((last as any) === createEndpoint) { return requestResponseMessage(ep, { - type: MessageType.ENDPOINT, + type: msgType(MessageType.ENDPOINT, isV430), }).then(fromWireValue); } // We just pretend that `bind()` didn’t happen. if (last === "bind") { return createProxy(ep, path.slice(0, -1)); } - const [argumentList, transferables] = processArguments(rawArgumentList); + const [argumentList, transferables] = processArguments( + rawArgumentList, + isV430 + ); return requestResponseMessage( ep, { - type: MessageType.APPLY, + type: msgType(MessageType.APPLY, isV430), path: path.map((p) => p.toString()), argumentList, }, @@ -520,11 +560,14 @@ function createProxy( }, construct(_target, rawArgumentList) { throwIfProxyReleased(isProxyReleased); - const [argumentList, transferables] = processArguments(rawArgumentList); + const [argumentList, transferables] = processArguments( + rawArgumentList, + isV430 + ); return requestResponseMessage( ep, { - type: MessageType.CONSTRUCT, + type: msgType(MessageType.CONSTRUCT, isV430), path: path.map((p) => p.toString()), argumentList, }, @@ -540,8 +583,11 @@ function myFlat(arr: (T | T[])[]): T[] { return Array.prototype.concat.apply([], arr); } -function processArguments(argumentList: any[]): [WireValue[], Transferable[]] { - const processed = argumentList.map(toWireValue); +function processArguments( + argumentList: any[], + isV430: boolean +): [WireValue[], Transferable[]] { + const processed = argumentList.map((arg) => toWireValue(arg, isV430)); return [processed.map((v) => v[0]), myFlat(processed.map((v) => v[1]))]; } @@ -568,14 +614,13 @@ export function windowEndpoint( }; } -function toWireValue(value: any): [WireValue, Transferable[]] { +function toWireValue(value: any, isV430: boolean): [WireValue, Transferable[]] { for (const [name, handler] of transferHandlers) { if (handler.canHandle(value)) { const [serializedValue, transferables] = handler.serialize(value); return [ { - type: WireValueType.HANDLER, - wireType: true, + type: isV430 ? V430WireValueType.HANDLER : WireValueType.HANDLER, name, value: serializedValue, }, @@ -585,8 +630,7 @@ function toWireValue(value: any): [WireValue, Transferable[]] { } return [ { - type: WireValueType.RAW, - wireType: true, + type: isV430 ? V430WireValueType.RAW : WireValueType.RAW, value, }, transferCache.get(value) || [], @@ -595,8 +639,11 @@ function toWireValue(value: any): [WireValue, Transferable[]] { function fromWireValue(value: WireValue): any { switch (value.type) { + case V430WireValueType.HANDLER: case WireValueType.HANDLER: - return transferHandlers.get(value.name)!.deserialize(value.value); + const isV430 = value.type === V430WireValueType.HANDLER; + return transferHandlers.get(value.name)!.deserialize(value.value, isV430); + case V430WireValueType.RAW: case WireValueType.RAW: return value.value; } @@ -623,6 +670,34 @@ function requestResponseMessage( }); } +function msgType( + type: T, + isV430: boolean +): MessageTypeMap[T] | T { + if (!isV430) { + return type; + } + + function mapMessageTypeToV430Type(type: MessageType): V430MessageType { + switch (type) { + case MessageType.GET: + return V430MessageType.GET; + case MessageType.SET: + return V430MessageType.SET; + case MessageType.APPLY: + return V430MessageType.APPLY; + case MessageType.CONSTRUCT: + return V430MessageType.CONSTRUCT; + case MessageType.ENDPOINT: + return V430MessageType.ENDPOINT; + case MessageType.RELEASE: + return V430MessageType.RELEASE; + } + } + + return mapMessageTypeToV430Type(type) as MessageTypeMap[T]; +} + function generateUUID(): string { return new Array(4) .fill(0) diff --git a/src/protocol.ts b/src/protocol.ts index 7a0c1926..f16ea6d7 100644 --- a/src/protocol.ts +++ b/src/protocol.ts @@ -32,24 +32,27 @@ export interface Endpoint extends EventSource { start?: () => void; } -export const enum WireValueType { +export const enum V430WireValueType { RAW, - PROXY, - THROW, - HANDLER, + HANDLER = 3, +} + +export const enum WireValueType { + RAW = "RAW", + PROXY = "PROXY", + THROW = "THROW", + HANDLER = "HANDLER", } export interface RawWireValue { id?: string; - type: WireValueType.RAW; - wireType?: true; + type: WireValueType.RAW | V430WireValueType.RAW; value: {}; } export interface HandlerWireValue { id?: string; - type: WireValueType.HANDLER; - wireType?: true; + type: WireValueType.HANDLER | V430WireValueType.HANDLER; name: string; value: unknown; } @@ -58,7 +61,7 @@ export type WireValue = RawWireValue | HandlerWireValue; export type MessageID = string; -export const enum MessageType { +export const enum V430MessageType { GET, SET, APPLY, @@ -67,40 +70,57 @@ export const enum MessageType { RELEASE, } +export interface MessageTypeMap { + [MessageType.GET]: V430MessageType.GET; + [MessageType.SET]: V430MessageType.SET; + [MessageType.APPLY]: V430MessageType.APPLY; + [MessageType.CONSTRUCT]: V430MessageType.CONSTRUCT; + [MessageType.ENDPOINT]: V430MessageType.ENDPOINT; + [MessageType.RELEASE]: V430MessageType.RELEASE; +} + +export const enum MessageType { + GET = "GET", + SET = "SET", + APPLY = "APPLY", + CONSTRUCT = "CONSTRUCT", + ENDPOINT = "ENDPOINT", + RELEASE = "RELEASE", +} + interface BaseMessage { id?: MessageID; - wireType?: undefined; } export interface GetMessage extends BaseMessage { - type: MessageType.GET; + type: MessageType.GET | V430MessageType.GET; path: string[]; } export interface SetMessage extends BaseMessage { - type: MessageType.SET; + type: MessageType.SET | V430MessageType.SET; path: string[]; value: WireValue; } export interface ApplyMessage extends BaseMessage { - type: MessageType.APPLY; + type: MessageType.APPLY | V430MessageType.APPLY; path: string[]; argumentList: WireValue[]; } export interface ConstructMessage extends BaseMessage { - type: MessageType.CONSTRUCT; + type: MessageType.CONSTRUCT | V430MessageType.CONSTRUCT; path: string[]; argumentList: WireValue[]; } export interface EndpointMessage extends BaseMessage { - type: MessageType.ENDPOINT; + type: MessageType.ENDPOINT | V430MessageType.ENDPOINT; } export interface ReleaseMessage extends BaseMessage { - type: MessageType.RELEASE; + type: MessageType.RELEASE | V430MessageType.RELEASE; } export type Message = diff --git a/tests/fixtures/attack-iframe.html b/tests/fixtures/attack-iframe.html index 1daa807c..e32c951b 100644 --- a/tests/fixtures/attack-iframe.html +++ b/tests/fixtures/attack-iframe.html @@ -6,8 +6,8 @@ // send back a message to modify the prototype parent.postMessage( { - type: 1 /* MessageType.SET */, - value: { type: 0 /* WireValueType.RAW */, value: "x" }, + type: "SET", + value: { type: "RAW", value: "x" }, path: ["__proto__", "foo"], }, "*" diff --git a/tests/fixtures/oldcomlink.html b/tests/fixtures/v430comlink.html similarity index 64% rename from tests/fixtures/oldcomlink.html rename to tests/fixtures/v430comlink.html index efb2ab39..1ea7adc3 100644 --- a/tests/fixtures/oldcomlink.html +++ b/tests/fixtures/v430comlink.html @@ -1,18 +1,24 @@ diff --git a/tests/v430comlink.test.js b/tests/v430comlink.test.js index 34a75273..6702cf52 100644 --- a/tests/v430comlink.test.js +++ b/tests/v430comlink.test.js @@ -18,6 +18,7 @@ describe("Comlink across versions (4.3.0 to latest main)", function () { let oldTruncateThreshold; let notcalled; let error; + let iframePort; function unhandledrejectionCallback(ev) { notcalled = false; @@ -40,7 +41,13 @@ describe("Comlink across versions (4.3.0 to latest main)", function () { this.ifr.sandbox.add("allow-scripts", "allow-same-origin"); this.ifr.src = "/base/tests/fixtures/v430comlink.html"; document.body.appendChild(this.ifr); - return new Promise((resolve) => (this.ifr.onload = resolve)); + return new Promise((resolve) => (this.ifr.onload = resolve)).then(() => { + const iframeChannel = new MessageChannel(); + iframePort = iframeChannel.port1; + this.ifr.contentWindow.postMessage(iframeChannel.port2, "*", [ + iframeChannel.port2, + ]); + }); }); afterEach(function () { @@ -53,7 +60,7 @@ describe("Comlink across versions (4.3.0 to latest main)", function () { }); it("can send a proxy and call a function", async function () { - const iframe = Comlink.windowEndpoint(this.ifr.contentWindow); + const iframe = iframePort; const latest = Comlink.wrap(iframe); expect(await latest.acceptProxy(Comlink.proxy(() => 3))).to.equal(4); @@ -62,7 +69,7 @@ describe("Comlink across versions (4.3.0 to latest main)", function () { }); it("can send port that get wrapped and transfer by argument", async function () { - const iframe = Comlink.windowEndpoint(this.ifr.contentWindow); + const iframe = iframePort; const latest = Comlink.wrap(iframe); let maybecalled = false; class Test { @@ -82,7 +89,7 @@ describe("Comlink across versions (4.3.0 to latest main)", function () { it("can send port that get wrapped and transfer by a setter", async function () { // Verify that it also works with a setter - const iframe = Comlink.windowEndpoint(this.ifr.contentWindow); + const iframe = iframePort; const latest = Comlink.wrap(iframe); const channel2 = new MessageChannel(); let risecalled = false; @@ -104,7 +111,7 @@ describe("Comlink across versions (4.3.0 to latest main)", function () { }); it("works with custom handlers", async function () { - const iframe = Comlink.windowEndpoint(this.ifr.contentWindow); + const iframe = iframePort; const proxy = Comlink.wrap(iframe); Comlink.transferHandlers.set("pingpong", { From 1d8bac05bd094a7c4653ae69529add9c1eb3242e Mon Sep 17 00:00:00 2001 From: Joan Varvenne Date: Tue, 7 Mar 2023 17:12:45 +0000 Subject: [PATCH 4/7] Revert the addition of BaseMessage, it's not really needed anymore --- src/protocol.ts | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/protocol.ts b/src/protocol.ts index f16ea6d7..2bb29fc8 100644 --- a/src/protocol.ts +++ b/src/protocol.ts @@ -88,38 +88,40 @@ export const enum MessageType { RELEASE = "RELEASE", } -interface BaseMessage { +export interface GetMessage { id?: MessageID; -} - -export interface GetMessage extends BaseMessage { type: MessageType.GET | V430MessageType.GET; path: string[]; } -export interface SetMessage extends BaseMessage { +export interface SetMessage { + id?: MessageID; type: MessageType.SET | V430MessageType.SET; path: string[]; value: WireValue; } -export interface ApplyMessage extends BaseMessage { +export interface ApplyMessage { + id?: MessageID; type: MessageType.APPLY | V430MessageType.APPLY; path: string[]; argumentList: WireValue[]; } -export interface ConstructMessage extends BaseMessage { +export interface ConstructMessage { + id?: MessageID; type: MessageType.CONSTRUCT | V430MessageType.CONSTRUCT; path: string[]; argumentList: WireValue[]; } -export interface EndpointMessage extends BaseMessage { +export interface EndpointMessage { + id?: MessageID; type: MessageType.ENDPOINT | V430MessageType.ENDPOINT; } -export interface ReleaseMessage extends BaseMessage { +export interface ReleaseMessage { + id?: MessageID; type: MessageType.RELEASE | V430MessageType.RELEASE; } From 5c7458bde45509d85ade21511967b76d43150cd0 Mon Sep 17 00:00:00 2001 From: Joan Varvenne Date: Wed, 8 Mar 2023 09:00:35 +0000 Subject: [PATCH 5/7] Refactor V430* to Legacy* and isV430 to isLegacy --- src/comlink.ts | 115 +++++++++++++++++++++++++----------------------- src/protocol.ts | 32 +++++++------- 2 files changed, 75 insertions(+), 72 deletions(-) diff --git a/src/comlink.ts b/src/comlink.ts index 0fb938b1..1c027629 100644 --- a/src/comlink.ts +++ b/src/comlink.ts @@ -8,11 +8,11 @@ import { Endpoint, EventSource, Message, - V430MessageType, + LegacyMessageType, MessageType, PostMessageWithOrigin, WireValue, - V430WireValueType, + LegacyWireValueType, WireValueType, MessageTypeMap, } from "./protocol"; @@ -204,9 +204,9 @@ export interface TransferHandler { * registered under). * * @param value the serialized value to build a T from - * @param isV430 true if this was deserialized from a version of comlink older than 4.3.0 + * @param legacy true if this was deserialized from a version of comlink older than 4.3.0 */ - deserialize(value: S, isV430: boolean): T; + deserialize(value: S, legacy: boolean): T; } /** @@ -220,9 +220,9 @@ const proxyTransferHandler: TransferHandler = { expose(obj, port1); return [port2, [port2]]; }, - deserialize(port, isV430) { + deserialize(port, legacy) { port.start(); - return wrap(port, undefined, isV430); + return wrap(port, undefined, legacy); }, }; @@ -314,45 +314,45 @@ export function expose( ...(ev.data as Message | WireValue), }; - const isV430 = typeof type === "number"; + const isLegacy = typeof type === "number"; const argumentList = (ev.data.argumentList || []).map(fromWireValue); - markV430Ports(...argumentList); + markLegacyPorts(...argumentList); let returnValue; try { const parent = path.slice(0, -1).reduce((obj, prop) => obj[prop], obj); const rawValue = path.reduce((obj, prop) => obj[prop], obj); switch (type) { - case V430MessageType.GET: + case LegacyMessageType.GET: case MessageType.GET: { returnValue = rawValue; } break; - case V430MessageType.SET: + case LegacyMessageType.SET: case MessageType.SET: { const value = fromWireValue(ev.data.value); - markV430Ports(value); + markLegacyPorts(value); parent[path.slice(-1)[0]] = value; returnValue = true; } break; - case V430MessageType.APPLY: + case LegacyMessageType.APPLY: case MessageType.APPLY: { returnValue = rawValue.apply(parent, argumentList); } break; - case V430MessageType.CONSTRUCT: + case LegacyMessageType.CONSTRUCT: case MessageType.CONSTRUCT: { const value = new rawValue(...argumentList); returnValue = proxy(value); } break; - case V430MessageType.ENDPOINT: + case LegacyMessageType.ENDPOINT: case MessageType.ENDPOINT: { const { port1, port2 } = new MessageChannel(); @@ -360,7 +360,7 @@ export function expose( returnValue = transfer(port1, [port1]); } break; - case V430MessageType.RELEASE: + case LegacyMessageType.RELEASE: case MessageType.RELEASE: { returnValue = undefined; @@ -377,7 +377,7 @@ export function expose( return { value, [throwMarker]: 0 }; }) .then((returnValue) => { - const [wireValue, transferables] = toWireValue(returnValue, isV430); + const [wireValue, transferables] = toWireValue(returnValue, isLegacy); ep.postMessage({ ...wireValue, id }, transferables); if (type === MessageType.RELEASE) { // detach and deactive after sending release response above. @@ -395,7 +395,7 @@ export function expose( value: new TypeError("Unserializable return value"), [throwMarker]: 0, }, - isV430 + isLegacy ); ep.postMessage({ ...wireValue, id }, transferables); }); @@ -417,10 +417,10 @@ function isEndpoint(endpoint: object): endpoint is Endpoint { ); } -function markV430Ports(...args: any[]) { +function markLegacyPorts(...args: any[]) { for (const arg of args) { if (isObject(arg) && isEndpoint(arg) && isMessagePort(arg)) { - v430Endpoints.add(arg); + legacyEndpoints.add(arg); } } } @@ -432,10 +432,10 @@ function closeEndPoint(endpoint: Endpoint) { export function wrap( ep: Endpoint, target?: any, - isV430: boolean = false + legacy: boolean = false ): Remote { - if (isV430) { - v430Endpoints.add(ep); + if (legacy) { + legacyEndpoints.add(ep); } return createProxy(ep, [], target) as any; } @@ -446,9 +446,9 @@ function throwIfProxyReleased(isReleased: boolean) { } } -function releaseEndpoint(ep: Endpoint, isV430: boolean) { +function releaseEndpoint(ep: Endpoint, legacy: boolean) { return requestResponseMessage(ep, { - type: msgType(MessageType.RELEASE, isV430), + type: msgType(MessageType.RELEASE, legacy), }).then(() => { closeEndPoint(ep); }); @@ -465,7 +465,7 @@ interface FinalizationRegistry { } declare var FinalizationRegistry: FinalizationRegistry; -const v430Endpoints = new WeakSet(); +const legacyEndpoints = new WeakSet(); const proxyCounter = new WeakMap(); const proxyFinalizers = "FinalizationRegistry" in globalThis && @@ -473,8 +473,8 @@ const proxyFinalizers = const newCount = (proxyCounter.get(ep) || 0) - 1; proxyCounter.set(ep, newCount); if (newCount === 0) { - releaseEndpoint(ep, v430Endpoints.has(ep)); - v430Endpoints.delete(ep); + releaseEndpoint(ep, legacyEndpoints.has(ep)); + legacyEndpoints.delete(ep); } }); @@ -498,14 +498,14 @@ function createProxy( target: object = function () {} ): Remote { let isProxyReleased = false; - const isV430 = v430Endpoints.has(ep); + const isLegacy = legacyEndpoints.has(ep); const proxy = new Proxy(target, { get(_target, prop) { throwIfProxyReleased(isProxyReleased); if (prop === releaseProxy) { return () => { unregisterProxy(proxy); - releaseEndpoint(ep, isV430); + releaseEndpoint(ep, isLegacy); isProxyReleased = true; }; } @@ -514,7 +514,7 @@ function createProxy( return { then: () => proxy }; } const r = requestResponseMessage(ep, { - type: msgType(MessageType.GET, isV430), + type: msgType(MessageType.GET, isLegacy), path: path.map((p) => p.toString()), }).then(fromWireValue); return r.then.bind(r); @@ -525,11 +525,11 @@ function createProxy( throwIfProxyReleased(isProxyReleased); // FIXME: ES6 Proxy Handler `set` methods are supposed to return a // boolean. To show good will, we return true asynchronously ¯\_(ツ)_/¯ - const [value, transferables] = toWireValue(rawValue, isV430); + const [value, transferables] = toWireValue(rawValue, isLegacy); return requestResponseMessage( ep, { - type: msgType(MessageType.SET, isV430), + type: msgType(MessageType.SET, isLegacy), path: [...path, prop].map((p) => p.toString()), value, }, @@ -541,7 +541,7 @@ function createProxy( const last = path[path.length - 1]; if ((last as any) === createEndpoint) { return requestResponseMessage(ep, { - type: msgType(MessageType.ENDPOINT, isV430), + type: msgType(MessageType.ENDPOINT, isLegacy), }).then(fromWireValue); } // We just pretend that `bind()` didn’t happen. @@ -550,12 +550,12 @@ function createProxy( } const [argumentList, transferables] = processArguments( rawArgumentList, - isV430 + isLegacy ); return requestResponseMessage( ep, { - type: msgType(MessageType.APPLY, isV430), + type: msgType(MessageType.APPLY, isLegacy), path: path.map((p) => p.toString()), argumentList, }, @@ -566,12 +566,12 @@ function createProxy( throwIfProxyReleased(isProxyReleased); const [argumentList, transferables] = processArguments( rawArgumentList, - isV430 + isLegacy ); return requestResponseMessage( ep, { - type: msgType(MessageType.CONSTRUCT, isV430), + type: msgType(MessageType.CONSTRUCT, isLegacy), path: path.map((p) => p.toString()), argumentList, }, @@ -589,9 +589,9 @@ function myFlat(arr: (T | T[])[]): T[] { function processArguments( argumentList: any[], - isV430: boolean + isLegacy: boolean ): [WireValue[], Transferable[]] { - const processed = argumentList.map((arg) => toWireValue(arg, isV430)); + const processed = argumentList.map((arg) => toWireValue(arg, isLegacy)); return [processed.map((v) => v[0]), myFlat(processed.map((v) => v[1]))]; } @@ -618,13 +618,16 @@ export function windowEndpoint( }; } -function toWireValue(value: any, isV430: boolean): [WireValue, Transferable[]] { +function toWireValue( + value: any, + isLegacy: boolean +): [WireValue, Transferable[]] { for (const [name, handler] of transferHandlers) { if (handler.canHandle(value)) { const [serializedValue, transferables] = handler.serialize(value); return [ { - type: isV430 ? V430WireValueType.HANDLER : WireValueType.HANDLER, + type: isLegacy ? LegacyWireValueType.HANDLER : WireValueType.HANDLER, name, value: serializedValue, }, @@ -634,7 +637,7 @@ function toWireValue(value: any, isV430: boolean): [WireValue, Transferable[]] { } return [ { - type: isV430 ? V430WireValueType.RAW : WireValueType.RAW, + type: isLegacy ? LegacyWireValueType.RAW : WireValueType.RAW, value, }, transferCache.get(value) || [], @@ -643,11 +646,11 @@ function toWireValue(value: any, isV430: boolean): [WireValue, Transferable[]] { function fromWireValue(value: WireValue): any { switch (value.type) { - case V430WireValueType.HANDLER: + case LegacyWireValueType.HANDLER: case WireValueType.HANDLER: - const isV430 = value.type === V430WireValueType.HANDLER; - return transferHandlers.get(value.name)!.deserialize(value.value, isV430); - case V430WireValueType.RAW: + const legacy = value.type === LegacyWireValueType.HANDLER; + return transferHandlers.get(value.name)!.deserialize(value.value, legacy); + case LegacyWireValueType.RAW: case WireValueType.RAW: return value.value; } @@ -676,30 +679,30 @@ function requestResponseMessage( function msgType( type: T, - isV430: boolean + isLegacy: boolean ): MessageTypeMap[T] | T { - if (!isV430) { + if (!isLegacy) { return type; } - function mapMessageTypeToV430Type(type: MessageType): V430MessageType { + function mapMessageTypeToLegacyType(type: MessageType): LegacyMessageType { switch (type) { case MessageType.GET: - return V430MessageType.GET; + return LegacyMessageType.GET; case MessageType.SET: - return V430MessageType.SET; + return LegacyMessageType.SET; case MessageType.APPLY: - return V430MessageType.APPLY; + return LegacyMessageType.APPLY; case MessageType.CONSTRUCT: - return V430MessageType.CONSTRUCT; + return LegacyMessageType.CONSTRUCT; case MessageType.ENDPOINT: - return V430MessageType.ENDPOINT; + return LegacyMessageType.ENDPOINT; case MessageType.RELEASE: - return V430MessageType.RELEASE; + return LegacyMessageType.RELEASE; } } - return mapMessageTypeToV430Type(type) as MessageTypeMap[T]; + return mapMessageTypeToLegacyType(type) as MessageTypeMap[T]; } function generateUUID(): string { diff --git a/src/protocol.ts b/src/protocol.ts index 2bb29fc8..a32bcd71 100644 --- a/src/protocol.ts +++ b/src/protocol.ts @@ -32,7 +32,7 @@ export interface Endpoint extends EventSource { start?: () => void; } -export const enum V430WireValueType { +export const enum LegacyWireValueType { RAW, HANDLER = 3, } @@ -46,13 +46,13 @@ export const enum WireValueType { export interface RawWireValue { id?: string; - type: WireValueType.RAW | V430WireValueType.RAW; + type: WireValueType.RAW | LegacyWireValueType.RAW; value: {}; } export interface HandlerWireValue { id?: string; - type: WireValueType.HANDLER | V430WireValueType.HANDLER; + type: WireValueType.HANDLER | LegacyWireValueType.HANDLER; name: string; value: unknown; } @@ -61,7 +61,7 @@ export type WireValue = RawWireValue | HandlerWireValue; export type MessageID = string; -export const enum V430MessageType { +export const enum LegacyMessageType { GET, SET, APPLY, @@ -71,12 +71,12 @@ export const enum V430MessageType { } export interface MessageTypeMap { - [MessageType.GET]: V430MessageType.GET; - [MessageType.SET]: V430MessageType.SET; - [MessageType.APPLY]: V430MessageType.APPLY; - [MessageType.CONSTRUCT]: V430MessageType.CONSTRUCT; - [MessageType.ENDPOINT]: V430MessageType.ENDPOINT; - [MessageType.RELEASE]: V430MessageType.RELEASE; + [MessageType.GET]: LegacyMessageType.GET; + [MessageType.SET]: LegacyMessageType.SET; + [MessageType.APPLY]: LegacyMessageType.APPLY; + [MessageType.CONSTRUCT]: LegacyMessageType.CONSTRUCT; + [MessageType.ENDPOINT]: LegacyMessageType.ENDPOINT; + [MessageType.RELEASE]: LegacyMessageType.RELEASE; } export const enum MessageType { @@ -90,39 +90,39 @@ export const enum MessageType { export interface GetMessage { id?: MessageID; - type: MessageType.GET | V430MessageType.GET; + type: MessageType.GET | LegacyMessageType.GET; path: string[]; } export interface SetMessage { id?: MessageID; - type: MessageType.SET | V430MessageType.SET; + type: MessageType.SET | LegacyMessageType.SET; path: string[]; value: WireValue; } export interface ApplyMessage { id?: MessageID; - type: MessageType.APPLY | V430MessageType.APPLY; + type: MessageType.APPLY | LegacyMessageType.APPLY; path: string[]; argumentList: WireValue[]; } export interface ConstructMessage { id?: MessageID; - type: MessageType.CONSTRUCT | V430MessageType.CONSTRUCT; + type: MessageType.CONSTRUCT | LegacyMessageType.CONSTRUCT; path: string[]; argumentList: WireValue[]; } export interface EndpointMessage { id?: MessageID; - type: MessageType.ENDPOINT | V430MessageType.ENDPOINT; + type: MessageType.ENDPOINT | LegacyMessageType.ENDPOINT; } export interface ReleaseMessage { id?: MessageID; - type: MessageType.RELEASE | V430MessageType.RELEASE; + type: MessageType.RELEASE | LegacyMessageType.RELEASE; } export type Message = From 8ba107a1f2531dbd268dfce526bd2ff44c32ffa5 Mon Sep 17 00:00:00 2001 From: Joan Varvenne Date: Thu, 9 Mar 2023 11:00:48 +0000 Subject: [PATCH 6/7] Simplify markLegacyPort logic and only mark them when needed --- src/comlink.ts | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/src/comlink.ts b/src/comlink.ts index 1c027629..6df838d6 100644 --- a/src/comlink.ts +++ b/src/comlink.ts @@ -202,11 +202,8 @@ export interface TransferHandler { * Gets called to deserialize an incoming value that was serialized in the * other thread with this transfer handler (known through the name it was * registered under). - * - * @param value the serialized value to build a T from - * @param legacy true if this was deserialized from a version of comlink older than 4.3.0 */ - deserialize(value: S, legacy: boolean): T; + deserialize(value: S): T; } /** @@ -220,9 +217,9 @@ const proxyTransferHandler: TransferHandler = { expose(obj, port1); return [port2, [port2]]; }, - deserialize(port, legacy) { + deserialize(port) { port.start(); - return wrap(port, undefined, legacy); + return wrap(port); }, }; @@ -317,8 +314,6 @@ export function expose( const isLegacy = typeof type === "number"; const argumentList = (ev.data.argumentList || []).map(fromWireValue); - markLegacyPorts(...argumentList); - let returnValue; try { const parent = path.slice(0, -1).reduce((obj, prop) => obj[prop], obj); @@ -334,7 +329,6 @@ export function expose( case MessageType.SET: { const value = fromWireValue(ev.data.value); - markLegacyPorts(value); parent[path.slice(-1)[0]] = value; returnValue = true; } @@ -417,11 +411,13 @@ function isEndpoint(endpoint: object): endpoint is Endpoint { ); } -function markLegacyPorts(...args: any[]) { - for (const arg of args) { - if (isObject(arg) && isEndpoint(arg) && isMessagePort(arg)) { - legacyEndpoints.add(arg); - } +function markLegacyPort(maybePort: unknown) { + if ( + isObject(maybePort) && + isEndpoint(maybePort) && + isMessagePort(maybePort) + ) { + legacyEndpoints.add(maybePort); } } @@ -647,10 +643,11 @@ function toWireValue( function fromWireValue(value: WireValue): any { switch (value.type) { case LegacyWireValueType.HANDLER: + markLegacyPort(value.value); case WireValueType.HANDLER: - const legacy = value.type === LegacyWireValueType.HANDLER; - return transferHandlers.get(value.name)!.deserialize(value.value, legacy); + return transferHandlers.get(value.name)!.deserialize(value.value); case LegacyWireValueType.RAW: + markLegacyPort(value.value); case WireValueType.RAW: return value.value; } From fee83e650fddbd55653d2bb5a41200cf3a233157 Mon Sep 17 00:00:00 2001 From: Joan Varvenne Date: Sun, 12 Mar 2023 12:32:48 +0000 Subject: [PATCH 7/7] Add traversal as it's more useful than anticipated With traversal the communication with an older version of the library will always work as long as the code using the older version of the lib is the one to initiate the communication. Also the overhead is probably not as bad as I thought given this: 1. Does not impact the new format 2. Does not make it worse for function with many arguments 3. Only has a big impact on trees being transmitted. However doing structured cloning on a tree is already costly and probably be something that should be avoided in the first place. --- src/comlink.ts | 14 ++++++++------ tests/fixtures/v430comlink.html | 2 +- tests/v430comlink.test.js | 4 +++- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/comlink.ts b/src/comlink.ts index 6df838d6..cd811d4a 100644 --- a/src/comlink.ts +++ b/src/comlink.ts @@ -412,12 +412,14 @@ function isEndpoint(endpoint: object): endpoint is Endpoint { } function markLegacyPort(maybePort: unknown) { - if ( - isObject(maybePort) && - isEndpoint(maybePort) && - isMessagePort(maybePort) - ) { - legacyEndpoints.add(maybePort); + if (isObject(maybePort)) { + if (isEndpoint(maybePort) && isMessagePort(maybePort)) { + legacyEndpoints.add(maybePort); + } else { + for (const prop in maybePort) { + markLegacyPort(maybePort[prop as keyof typeof maybePort]); + } + } } } diff --git a/tests/fixtures/v430comlink.html b/tests/fixtures/v430comlink.html index 83e57b55..2614f58c 100644 --- a/tests/fixtures/v430comlink.html +++ b/tests/fixtures/v430comlink.html @@ -20,7 +20,7 @@ } callme(port) { - Comlink.wrap(port).maybe(); + Comlink.wrap(port.foo).maybe(); } iwannabeaport = undefined; diff --git a/tests/v430comlink.test.js b/tests/v430comlink.test.js index 6702cf52..06cc398f 100644 --- a/tests/v430comlink.test.js +++ b/tests/v430comlink.test.js @@ -80,7 +80,9 @@ describe("Comlink across versions (4.3.0 to latest main)", function () { const channel = new MessageChannel(); Comlink.expose(new Test(), channel.port1); - await latest.callme(Comlink.transfer(channel.port2, [channel.port2])); + await latest.callme( + Comlink.transfer({ foo: channel.port2 }, [channel.port2]) + ); expect(maybecalled).to.equal(true);