From 3398070e26586164218d17a20b91bf6428902396 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Wed, 17 Feb 2021 15:52:01 +0100 Subject: [PATCH] [api-minor] Remove support for synchronous event dispatching in `LoopbackPort` *Please note:* The `defer` parameter has been enabled by default ever since PR 9777 (in 2018), which first shipped in PDF.js release `2.0.943`. With workers *disabled*, e.g. in Node.js environments, this has been used ever since without any problems reported[1]. The impetus for this change was that I happened to notice that *if* the `LoopbackPort` was used with synchronous event dispatching, we'd simply send that data as-is to the listeners. This created an inconsistency in the data returned from the `pdf.worker.js` file, since `postMessage` used with *actual* workers (or the `LoopbackPort` with `defer = true`) will ignore/throw when encountering unclonable data. Originally my intention was simply to just call `cloneValue` regardless of the event dispatching used in `LoopbackPort`, however looking at the use-cases (or lack thereof) of the `LoopbackPort` it seemed reasonable to simply remove the `defer` parameter instead. This patch is tagged "[api-minor]" since the `LoopbackPort` is still exposed in the API, although I really hope that no third-party is using this (since disabling workers leads to bad performance). Finally, this patch changes a `forEach` loop to `for...of` and makes uses of optional changing in existing code. --- [1] As evident by the `npm test` command run by Github Actions, and previously by Travis. --- src/display/api.js | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/src/display/api.js b/src/display/api.js index c8c5c025b..9339e5bc7 100644 --- a/src/display/api.js +++ b/src/display/api.js @@ -1624,9 +1624,8 @@ class PDFPageProxy { } class LoopbackPort { - constructor(defer = true) { + constructor() { this._listeners = []; - this._defer = defer; this._deferred = Promise.resolve(undefined); } @@ -1670,7 +1669,7 @@ class LoopbackPort { continue; } if (typeof desc.value === "function") { - if (value.hasOwnProperty && value.hasOwnProperty(i)) { + if (value.hasOwnProperty?.(i)) { throw new Error( `LoopbackPort.postMessage - cannot clone: ${value[i]}` ); @@ -1682,19 +1681,13 @@ class LoopbackPort { return result; } - if (!this._defer) { - this._listeners.forEach(listener => { - listener.call(this, { data: obj }); - }); - return; - } - const cloned = new WeakMap(); - const e = { data: cloneValue(obj) }; + const event = { data: cloneValue(obj) }; + this._deferred.then(() => { - this._listeners.forEach(listener => { - listener.call(this, e); - }); + for (const listener of this._listeners) { + listener.call(this, event); + } }); }