Commit 91c4b724 authored by Eduardo Sanz García's avatar Eduardo Sanz García Committed by Eduardo

Use `Set` data structure instead of `Array`

`Set` is a better data structure because it has a faster search: `Set.has` has a O(1) while `Array.includes` has a O(n).
parent fb7acdbf
import debounce from 'lodash.debounce'; import debounce from 'lodash.debounce';
/** @typedef {(frame: HTMLIFrameElement) => void} FrameCallback */
// Find difference of two arrays
let difference = (arrayA, arrayB) => {
return arrayA.filter(x => !arrayB.includes(x));
};
export const DEBOUNCE_WAIT = 40; export const DEBOUNCE_WAIT = 40;
/** @typedef {(frame: HTMLIFrameElement) => void} FrameCallback */
export default class FrameObserver { export default class FrameObserver {
/** /**
* @param {Element} element - root of the DOM subtree to watch for the addition * @param {Element} element - root of the DOM subtree to watch for the addition
...@@ -20,8 +15,8 @@ export default class FrameObserver { ...@@ -20,8 +15,8 @@ export default class FrameObserver {
this._element = element; this._element = element;
this._onFrameAdded = onFrameAdded; this._onFrameAdded = onFrameAdded;
this._onFrameRemoved = onFrameRemoved; this._onFrameRemoved = onFrameRemoved;
/** @type {HTMLIFrameElement[]} */ /** @type {Set<HTMLIFrameElement>} */
this._handledFrames = []; this._handledFrames = new Set();
this._mutationObserver = new MutationObserver( this._mutationObserver = new MutationObserver(
debounce(() => { debounce(() => {
...@@ -50,7 +45,7 @@ export default class FrameObserver { ...@@ -50,7 +45,7 @@ export default class FrameObserver {
frameWindow.addEventListener('unload', () => { frameWindow.addEventListener('unload', () => {
this._removeFrame(frame); this._removeFrame(frame);
}); });
this._handledFrames.push(frame); this._handledFrames.add(frame);
this._onFrameAdded(frame); this._onFrameAdded(frame);
}); });
} else { } else {
...@@ -63,22 +58,22 @@ export default class FrameObserver { ...@@ -63,22 +58,22 @@ export default class FrameObserver {
*/ */
_removeFrame(frame) { _removeFrame(frame) {
this._onFrameRemoved(frame); this._onFrameRemoved(frame);
this._handledFrames.delete(frame);
// Remove the frame from our list
this._handledFrames = this._handledFrames.filter(x => x !== frame);
} }
_discoverFrames() { _discoverFrames() {
let frames = findFrames(this._element); let frames = findFrames(this._element);
for (let frame of frames) { for (let frame of frames) {
if (!this._handledFrames.includes(frame)) { if (!this._handledFrames.has(frame)) {
this._addFrame(frame); this._addFrame(frame);
} }
} }
for (let frame of difference(this._handledFrames, frames)) { for (let frame of this._handledFrames) {
this._removeFrame(frame); if (!frames.has(frame)) {
this._removeFrame(frame);
}
} }
} }
} }
...@@ -122,8 +117,8 @@ export function isDocumentReady(iframe, callback) { ...@@ -122,8 +117,8 @@ export function isDocumentReady(iframe, callback) {
* can do that. See https://github.com/hypothesis/client/issues/530 * can do that. See https://github.com/hypothesis/client/issues/530
* *
* @param {Element} container * @param {Element} container
* @return {HTMLIFrameElement[]} * @return {Set<HTMLIFrameElement>}
*/ */
export function findFrames(container) { export function findFrames(container) {
return Array.from(container.querySelectorAll('iframe[enable-annotation]')); return new Set(container.querySelectorAll('iframe[enable-annotation]'));
} }
...@@ -22,13 +22,13 @@ describe('findFrames', () => { ...@@ -22,13 +22,13 @@ describe('findFrames', () => {
it('should return valid frames', () => { it('should return valid frames', () => {
let foundFrames = findFrames(container); let foundFrames = findFrames(container);
assert.deepEqual(foundFrames, []); assert.lengthOf(foundFrames, 0);
const frame1 = _addFrameToContainer(); const frame1 = _addFrameToContainer();
const frame2 = _addFrameToContainer(); const frame2 = _addFrameToContainer();
foundFrames = findFrames(container); foundFrames = findFrames(container);
assert.deepEqual(foundFrames, [frame1, frame2]); assert.deepEqual([...foundFrames], [frame1, frame2]);
}); });
it('should not return frames that have not opted into annotation', () => { it('should not return frames that have not opted into annotation', () => {
...@@ -37,6 +37,6 @@ describe('findFrames', () => { ...@@ -37,6 +37,6 @@ describe('findFrames', () => {
frame.removeAttribute('enable-annotation'); frame.removeAttribute('enable-annotation');
const foundFrames = findFrames(container); const foundFrames = findFrames(container);
assert.deepEqual(foundFrames, []); assert.lengthOf(foundFrames, 0);
}); });
}); });
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment