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

Support the once option, when registering EventBus listeners #12793

Merged
merged 1 commit into from
Dec 30, 2020
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
Support the once option, when registering EventBus listeners
This follows the same principle as the `once` option that exists in the native `addEventListener` method, and will thus automatically remove an `EventBus` listener when it's invoked; see https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener#Parameters

Finally, this patch also tweaks some the existing `EventBus`-code to use modern features such as optional chaining and logical assignment operators.
  • Loading branch information
Snuffleupagus committed Dec 29, 2020
commit 739d7c6d774b6c1f1aa8ee2ab133ebb3c33d2453
24 changes: 24 additions & 0 deletions test/unit/ui_utils_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,30 @@ describe("ui_utils", function () {
expect(count).toEqual(2);
});

it("dispatch event to handlers with/without 'once' option", function () {
const eventBus = new EventBus();
let multipleCount = 0,
onceCount = 0;

eventBus.on("test", function () {
multipleCount++;
});
eventBus.on(
"test",
function () {
onceCount++;
},
{ once: true }
);

eventBus.dispatch("test");
eventBus.dispatch("test");
eventBus.dispatch("test");

expect(multipleCount).toEqual(3);
expect(onceCount).toEqual(1);
});

it("should not re-dispatch to DOM", function (done) {
if (isNodeJS) {
pending("Document in not supported in Node.js.");
Expand Down
12 changes: 7 additions & 5 deletions web/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -1497,11 +1497,13 @@ const PDFViewerApplication = {
// It should be *extremely* rare for metadata to not have been resolved
// when this code runs, but ensure that we handle that case here.
await new Promise(resolve => {
const metadataLoaded = () => {
this.eventBus._off("metadataloaded", metadataLoaded);
resolve();
};
this.eventBus._on("metadataloaded", metadataLoaded);
this.eventBus._on(
"metadataloaded",
evt => {
resolve();
},
{ once: true }
);
});
if (pdfDocument !== this.pdfDocument) {
return; // The document was closed while the metadata resolved.
Expand Down
12 changes: 7 additions & 5 deletions web/pdf_history.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,11 +76,13 @@ class PDFHistory {
this.eventBus._on("pagesinit", () => {
this._isPagesLoaded = false;

const onPagesLoaded = evt => {
this.eventBus._off("pagesloaded", onPagesLoaded);
this._isPagesLoaded = !!evt.pagesCount;
};
this.eventBus._on("pagesloaded", onPagesLoaded);
this.eventBus._on(
"pagesloaded",
evt => {
this._isPagesLoaded = !!evt.pagesCount;
},
{ once: true }
);
});
}

Expand Down
38 changes: 22 additions & 16 deletions web/ui_utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -804,24 +804,32 @@ class EventBus {
this._listeners = Object.create(null);

if (typeof PDFJSDev === "undefined" || PDFJSDev.test("MOZCENTRAL")) {
this._isInAutomation = (options && options.isInAutomation) === true;
this._isInAutomation = options?.isInAutomation === true;
}
}

/**
* @param {string} eventName
* @param {function} listener
* @param {Object} [options]
*/
on(eventName, listener) {
this._on(eventName, listener, { external: true });
on(eventName, listener, options = null) {
this._on(eventName, listener, {
external: true,
once: options?.once,
});
}

/**
* @param {string} eventName
* @param {function} listener
* @param {Object} [options]
*/
off(eventName, listener) {
this._off(eventName, listener, { external: true });
off(eventName, listener, options = null) {
this._off(eventName, listener, {
external: true,
once: options?.once,
});
}

dispatch(eventName) {
Expand All @@ -841,20 +849,20 @@ class EventBus {
let externalListeners;
// Making copy of the listeners array in case if it will be modified
// during dispatch.
eventListeners.slice(0).forEach(function ({ listener, external }) {
eventListeners.slice(0).forEach(({ listener, external, once }) => {
if (once) {
this._off(eventName, listener);
}
if (external) {
if (!externalListeners) {
externalListeners = [];
}
externalListeners.push(listener);
(externalListeners ||= []).push(listener);
return;
}
listener.apply(null, args);
});
// Dispatch any "external" listeners *after* the internal ones, to give the
// viewer components time to handle events and update their state first.
if (externalListeners) {
externalListeners.forEach(function (listener) {
externalListeners.forEach(listener => {
listener.apply(null, args);
});
externalListeners = null;
Expand All @@ -871,13 +879,11 @@ class EventBus {
* @ignore
*/
_on(eventName, listener, options = null) {
let eventListeners = this._listeners[eventName];
if (!eventListeners) {
this._listeners[eventName] = eventListeners = [];
}
const eventListeners = (this._listeners[eventName] ||= []);
eventListeners.push({
listener,
external: (options && options.external) === true,
external: options?.external === true,
once: options?.once === true,
});
}

Expand Down