From af5b1376a5ae56a8cfa794f8b379323657600d5f Mon Sep 17 00:00:00 2001 From: Tamas Sule Date: Tue, 2 Mar 2021 22:16:44 +0100 Subject: [PATCH 1/3] Use single ResizeObserver for better performance --- addon/modifiers/did-resize.js | 51 +++++++++++++---- .../integration/modifiers/did-resize-test.js | 57 ++++++++----------- 2 files changed, 65 insertions(+), 43 deletions(-) diff --git a/addon/modifiers/did-resize.js b/addon/modifiers/did-resize.js index 280aae7d..887b7364 100644 --- a/addon/modifiers/did-resize.js +++ b/addon/modifiers/did-resize.js @@ -6,23 +6,41 @@ export default class DidResizeModifier extends Modifier { options = {}; // Private API - observer = null; + _didInstall = false; + static observer = null; + static handlers = []; + + addHandler() { + DidResizeModifier.handlers.push({ + element: this.element, + handler: this.handler, + }); + } + + removeHandler() { + DidResizeModifier.handlers.splice( + DidResizeModifier.handlers.findIndex((h) => h.element === this.element), + 1 + ); + } observe() { - if (this.observer) { - this.observer.observe(this.element, this.options); + if (DidResizeModifier.observer) { + this.addHandler(); + DidResizeModifier.observer.observe(this.element, this.options); } } unobserve() { - if (this.observer) { - this.observer.unobserve(); + if (DidResizeModifier.observer) { + DidResizeModifier.observer.unobserve(this.element); } } disconnect() { - if (this.observer) { - this.observer.disconnect(); + if (DidResizeModifier.observer) { + DidResizeModifier.observer.disconnect(); + this.removeHandler(); } } @@ -38,7 +56,10 @@ export default class DidResizeModifier extends Modifier { this.handler = handler; this.options = options || this.options; - this.observe(); + // Only observe if the modifier is installed + if (this._didInstall) { + this.observe(); + } } didInstall() { @@ -46,11 +67,19 @@ export default class DidResizeModifier extends Modifier { return; } - this.observer = new ResizeObserver((entries, observer) => { - this.handler(entries[0], observer); - }); + if (!DidResizeModifier.observer) { + DidResizeModifier.observer = new ResizeObserver((entries, observer) => { + for (let entry of entries) { + const lookup = DidResizeModifier.handlers.find( + (h) => h.element === entry.target + ); + if (lookup) lookup.handler(entry, observer); + } + }); + } this.observe(); + this._didInstall = true; } willRemove() { diff --git a/tests/integration/modifiers/did-resize-test.js b/tests/integration/modifiers/did-resize-test.js index 60bddbca..32d68678 100644 --- a/tests/integration/modifiers/did-resize-test.js +++ b/tests/integration/modifiers/did-resize-test.js @@ -1,40 +1,33 @@ import { module, test } from 'qunit'; import { setupRenderingTest } from 'ember-qunit'; -import { render } from '@ember/test-helpers'; +import { find, render } from '@ember/test-helpers'; import hbs from 'htmlbars-inline-precompile'; import sinon from 'sinon'; -let resizeCallback; -let observeStub; -let unobserveStub; -let disconnectStub; -let MockResizeObserver; - module('Integration | Modifier | did-resize', function (hooks) { setupRenderingTest(hooks); - let resizeObserver; + let resizeCallback = null; + const observeStub = sinon.stub(); + const unobserveStub = sinon.stub(); + const disconnectStub = sinon.stub(); + const resizeObserver = window.ResizeObserver; + const mockResizeObserver = class MockResizeObserver { + constructor(callback) { + resizeCallback = callback; + } + + observe = observeStub; + unobserve = unobserveStub; + disconnect = disconnectStub; + }; hooks.beforeEach(function () { - resizeCallback = null; - observeStub = sinon.stub(); - unobserveStub = sinon.stub(); - disconnectStub = sinon.stub(); - - MockResizeObserver = class MockResizeObserver { - constructor(callback) { - resizeCallback = callback; - } - - observe = observeStub; - unobserve = unobserveStub; - disconnect = disconnectStub; - }; - - resizeObserver = window.ResizeObserver; - window.ResizeObserver = MockResizeObserver; - + observeStub.reset(); + unobserveStub.reset(); + disconnectStub.reset(); this.resizeStub = sinon.stub(); + window.ResizeObserver = mockResizeObserver; }); hooks.afterEach(function () { @@ -58,15 +51,16 @@ module('Integration | Modifier | did-resize', function (hooks) { }); test('modifier triggers handler when ResizeObserver fires callback', async function (assert) { - await render(hbs`
`); - - let fakeEntry = { target: {} }; + await render( + hbs`
` + ); + let entry = { target: find('#test-element') }; let fakeObserver = { observe: {} }; - resizeCallback([fakeEntry], fakeObserver); + resizeCallback([entry], fakeObserver); assert.ok( - this.resizeStub.calledOnceWith(fakeEntry, fakeObserver), + this.resizeStub.calledOnceWith(entry, fakeObserver), 'handler fired with correct parameters' ); }); @@ -87,7 +81,6 @@ module('Integration | Modifier | did-resize', function (hooks) { await render(hbs`
`); - assert.notOk(resizeCallback, 'no callback received'); assert.notOk(observeStub.calledOnce, 'observe was not called'); }); From 755760fef1f6f25096e262db9bf20883dbf35e78 Mon Sep 17 00:00:00 2001 From: Tamas Sule Date: Wed, 3 Mar 2021 14:20:18 +0100 Subject: [PATCH 2/3] Fix PR comments and improve testing to run in isolation --- addon/modifiers/did-resize.js | 62 +++++++------------ .../integration/modifiers/did-resize-test.js | 18 ++++-- 2 files changed, 34 insertions(+), 46 deletions(-) diff --git a/addon/modifiers/did-resize.js b/addon/modifiers/did-resize.js index 887b7364..4b4f43ee 100644 --- a/addon/modifiers/did-resize.js +++ b/addon/modifiers/did-resize.js @@ -6,22 +6,33 @@ export default class DidResizeModifier extends Modifier { options = {}; // Private API - _didInstall = false; static observer = null; - static handlers = []; + static handlers = null; + + constructor() { + super(...arguments); + + if (!('ResizeObserver' in window)) { + return; + } + + if (!DidResizeModifier.observer) { + DidResizeModifier.handlers = new WeakMap(); + DidResizeModifier.observer = new ResizeObserver((entries, observer) => { + for (let entry of entries) { + const handler = DidResizeModifier.handlers.get(entry.target); + if (handler) handler(entry, observer); + } + }); + } + } addHandler() { - DidResizeModifier.handlers.push({ - element: this.element, - handler: this.handler, - }); + DidResizeModifier.handlers.set(this.element, this.handler); } removeHandler() { - DidResizeModifier.handlers.splice( - DidResizeModifier.handlers.findIndex((h) => h.element === this.element), - 1 - ); + DidResizeModifier.handlers.delete(this.element); } observe() { @@ -34,12 +45,6 @@ export default class DidResizeModifier extends Modifier { unobserve() { if (DidResizeModifier.observer) { DidResizeModifier.observer.unobserve(this.element); - } - } - - disconnect() { - if (DidResizeModifier.observer) { - DidResizeModifier.observer.disconnect(); this.removeHandler(); } } @@ -56,33 +61,10 @@ export default class DidResizeModifier extends Modifier { this.handler = handler; this.options = options || this.options; - // Only observe if the modifier is installed - if (this._didInstall) { - this.observe(); - } - } - - didInstall() { - if (!('ResizeObserver' in window)) { - return; - } - - if (!DidResizeModifier.observer) { - DidResizeModifier.observer = new ResizeObserver((entries, observer) => { - for (let entry of entries) { - const lookup = DidResizeModifier.handlers.find( - (h) => h.element === entry.target - ); - if (lookup) lookup.handler(entry, observer); - } - }); - } - this.observe(); - this._didInstall = true; } willRemove() { - this.disconnect(); + this.unobserve(); } } diff --git a/tests/integration/modifiers/did-resize-test.js b/tests/integration/modifiers/did-resize-test.js index 32d68678..cef837bc 100644 --- a/tests/integration/modifiers/did-resize-test.js +++ b/tests/integration/modifiers/did-resize-test.js @@ -3,16 +3,17 @@ import { setupRenderingTest } from 'ember-qunit'; import { find, render } from '@ember/test-helpers'; import hbs from 'htmlbars-inline-precompile'; import sinon from 'sinon'; +import DidResizeModifier from 'ember-resize-modifier/modifiers/did-resize'; module('Integration | Modifier | did-resize', function (hooks) { setupRenderingTest(hooks); let resizeCallback = null; - const observeStub = sinon.stub(); - const unobserveStub = sinon.stub(); - const disconnectStub = sinon.stub(); - const resizeObserver = window.ResizeObserver; - const mockResizeObserver = class MockResizeObserver { + let observeStub = sinon.stub(); + let unobserveStub = sinon.stub(); + let disconnectStub = sinon.stub(); + let resizeObserver = window.ResizeObserver; + let mockResizeObserver = class MockResizeObserver { constructor(callback) { resizeCallback = callback; } @@ -28,6 +29,11 @@ module('Integration | Modifier | did-resize', function (hooks) { disconnectStub.reset(); this.resizeStub = sinon.stub(); window.ResizeObserver = mockResizeObserver; + + // reset static properties to make sure every test case runs independently + DidResizeModifier.observer = null; + DidResizeModifier.handlers = null; + resizeCallback = null; }); hooks.afterEach(function () { @@ -80,7 +86,7 @@ module('Integration | Modifier | did-resize', function (hooks) { delete window.ResizeObserver; await render(hbs`
`); - + assert.notOk(resizeCallback, 'no callback received'); assert.notOk(observeStub.calledOnce, 'observe was not called'); }); From 6b8600a02b63e788775921c1991ae329124111bb Mon Sep 17 00:00:00 2001 From: Tamas Sule Date: Wed, 3 Mar 2021 20:23:04 +0100 Subject: [PATCH 3/3] Add multiple modifiers test case --- .../integration/modifiers/did-resize-test.js | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/tests/integration/modifiers/did-resize-test.js b/tests/integration/modifiers/did-resize-test.js index cef837bc..20450608 100644 --- a/tests/integration/modifiers/did-resize-test.js +++ b/tests/integration/modifiers/did-resize-test.js @@ -101,4 +101,52 @@ module('Integration | Modifier | did-resize', function (hooks) { assert.ok(unobserveStub.calledOnce, 'unobserve called'); assert.ok(observeStub.calledTwice, 'observe was called again'); }); + + test('handlers are setup and called correctly when multiple modifiers are present', async function (assert) { + this.setProperties({ + resizeStub2: sinon.stub(), + resizeStub3: sinon.stub(), + }); + + await render( + hbs`
+
+
` + ); + + let entries = ['#test-element1', '#test-element2', '#test-element3'].map( + (elementId) => { + return { target: find(elementId) }; + } + ); + let fakeObserver = { observe: {} }; + + // trigger resize on all elements + resizeCallback(entries, fakeObserver); + + assert.ok(this.resizeStub.calledOnce, 'First handler was called only once'); + assert.ok( + this.resizeStub2.calledOnce, + 'Second handler was called only once' + ); + assert.ok( + this.resizeStub3.calledOnce, + 'Third handler was called only once' + ); + + // trigger resize only on the first element + resizeCallback([entries[0]], fakeObserver); + assert.ok( + this.resizeStub.calledTwice, + 'First handler was called a second time' + ); + assert.notOk( + this.resizeStub2.calledTwice, + 'Second handler was not called a second time' + ); + assert.notOk( + this.resizeStub3.calledTwice, + 'Third handler was not called a second time' + ); + }); });