diff --git a/src/js/component.js b/src/js/component.js index 8c884acb3..1f338f908 100644 --- a/src/js/component.js +++ b/src/js/component.js @@ -7,7 +7,7 @@ import window from 'global/window'; import evented from './mixins/evented'; import stateful from './mixins/stateful'; import * as Dom from './utils/dom.js'; -import * as DomData from './utils/dom-data'; +import DomData from './utils/dom-data'; import * as Fn from './utils/fn.js'; import * as Guid from './utils/guid.js'; import toTitleCase from './utils/to-title-case.js'; @@ -153,7 +153,9 @@ class Component { this.el_.parentNode.removeChild(this.el_); } - DomData.removeData(this.el_); + if (DomData.has(this.el_)) { + DomData.delete(this.el_); + } this.el_ = null; } diff --git a/src/js/utils/dom-data.js b/src/js/utils/dom-data.js index de39dae60..43cf1a915 100644 --- a/src/js/utils/dom-data.js +++ b/src/js/utils/dom-data.js @@ -2,8 +2,6 @@ * @file dom-data.js * @module dom-data */ -import * as Guid from './guid.js'; -import window from 'global/window'; /** * Element Data Store. @@ -15,85 +13,4 @@ import window from 'global/window'; * @type {Object} * @private */ -export const elData = {}; - -/* - * Unique attribute name to store an element's guid in - * - * @type {String} - * @constant - * @private - */ -const elIdAttr = 'vdata' + Math.floor(window.performance && window.performance.now() || Date.now()); - -/** - * Returns the cache object where data for an element is stored - * - * @param {Element} el - * Element to store data for. - * - * @return {Object} - * The cache object for that el that was passed in. - */ -export function getData(el) { - let id = el[elIdAttr]; - - if (!id) { - id = el[elIdAttr] = Guid.newGUID(); - } - - if (!elData[id]) { - elData[id] = {}; - } - - return elData[id]; -} - -/** - * Returns whether or not an element has cached data - * - * @param {Element} el - * Check if this element has cached data. - * - * @return {boolean} - * - True if the DOM element has cached data. - * - False otherwise. - */ -export function hasData(el) { - const id = el[elIdAttr]; - - if (!id) { - return false; - } - - return !!Object.getOwnPropertyNames(elData[id]).length; -} - -/** - * Delete data for the element from the cache and the guid attr from getElementById - * - * @param {Element} el - * Remove cached data for this element. - */ -export function removeData(el) { - const id = el[elIdAttr]; - - if (!id) { - return; - } - - // Remove all stored data - delete elData[id]; - - // Remove the elIdAttr property from the DOM node - try { - delete el[elIdAttr]; - } catch (e) { - if (el.removeAttribute) { - el.removeAttribute(elIdAttr); - } else { - // IE doesn't appear to support removeAttribute on the document element - el[elIdAttr] = null; - } - } -} +export default new WeakMap(); diff --git a/src/js/utils/events.js b/src/js/utils/events.js index f5ff79e80..296c24073 100644 --- a/src/js/utils/events.js +++ b/src/js/utils/events.js @@ -7,7 +7,7 @@ * @file events.js * @module events */ -import * as DomData from './dom-data'; +import DomData from './dom-data'; import * as Guid from './guid.js'; import log from './log.js'; import window from 'global/window'; @@ -23,7 +23,10 @@ import document from 'global/document'; * Type of event to clean up */ function _cleanUpEvents(elem, type) { - const data = DomData.getData(elem); + if (!DomData.has(elem)) { + return; + } + const data = DomData.get(elem); // Remove the events of a particular type if there are none left if (data.handlers[type].length === 0) { @@ -48,7 +51,7 @@ function _cleanUpEvents(elem, type) { // Finally remove the element data if there is no data left if (Object.getOwnPropertyNames(data).length === 0) { - DomData.removeData(elem); + DomData.delete(elem); } } @@ -250,7 +253,11 @@ export function on(elem, type, fn) { return _handleMultipleEvents(on, elem, type, fn); } - const data = DomData.getData(elem); + if (!DomData.has(elem)) { + DomData.set(elem, {}); + } + + const data = DomData.get(elem); // We need a place to store all our handler data if (!data.handlers) { @@ -329,11 +336,11 @@ export function on(elem, type, fn) { */ export function off(elem, type, fn) { // Don't want to add a cache object through getElData if not needed - if (!DomData.hasData(elem)) { + if (!DomData.has(elem)) { return; } - const data = DomData.getData(elem); + const data = DomData.get(elem); // If no events exist, nothing to unbind if (!data.handlers) { @@ -405,7 +412,7 @@ export function trigger(elem, event, hash) { // Fetches element data and a reference to the parent (for bubbling). // Don't want to add a data object to cache for every parent, // so checking hasElData first. - const elemData = (DomData.hasData(elem)) ? DomData.getData(elem) : {}; + const elemData = DomData.has(elem) ? DomData.get(elem) : {}; const parent = elem.parentNode || elem.ownerDocument; // type = event.type || event, // handler; @@ -432,7 +439,10 @@ export function trigger(elem, event, hash) { // If at the top of the DOM, triggers the default action unless disabled. } else if (!parent && !event.defaultPrevented && event.target && event.target[event.type]) { - const targetData = DomData.getData(event.target); + if (!DomData.has(event.target)) { + DomData.set(event.target, {}); + } + const targetData = DomData.get(event.target); // Checks if the target has a default action for this event. if (event.target[event.type]) { diff --git a/test/unit/component.test.js b/test/unit/component.test.js index f1dffa1c6..0c842328b 100644 --- a/test/unit/component.test.js +++ b/test/unit/component.test.js @@ -2,7 +2,7 @@ import window from 'global/window'; import Component from '../../src/js/component.js'; import * as Dom from '../../src/js/utils/dom.js'; -import * as DomData from '../../src/js/utils/dom-data'; +import DomData from '../../src/js/utils/dom-data'; import * as Events from '../../src/js/utils/events.js'; import * as Obj from '../../src/js/utils/obj'; import * as browser from '../../src/js/utils/browser.js'; @@ -346,7 +346,7 @@ QUnit.test('should dispose of component and children', function(assert) { return true; }); const el = comp.el(); - const data = DomData.getData(el); + const data = DomData.get(el); let hasDisposed = false; let bubbles = null; @@ -365,7 +365,7 @@ QUnit.test('should dispose of component and children', function(assert) { assert.ok(!comp.el(), 'component element was deleted'); assert.ok(!child.children(), 'child children were deleted'); assert.ok(!child.el(), 'child element was deleted'); - assert.ok(!DomData.hasData(el), 'listener data nulled'); + assert.ok(!DomData.has(el), 'listener data nulled'); assert.ok( !Object.getOwnPropertyNames(data).length, 'original listener data object was emptied' @@ -979,7 +979,7 @@ QUnit.test('*AnimationFrame methods fall back to timers if rAF not supported', f QUnit.test('setTimeout should remove dispose handler on trigger', function(assert) { const comp = new Component(getFakePlayer()); const el = comp.el(); - const data = DomData.getData(el); + const data = DomData.get(el); comp.setTimeout(() => {}, 1); @@ -996,7 +996,7 @@ QUnit.test('setTimeout should remove dispose handler on trigger', function(asser QUnit.test('requestAnimationFrame should remove dispose handler on trigger', function(assert) { const comp = new Component(getFakePlayer()); const el = comp.el(); - const data = DomData.getData(el); + const data = DomData.get(el); const oldRAF = window.requestAnimationFrame; const oldCAF = window.cancelAnimationFrame; diff --git a/test/unit/menu.test.js b/test/unit/menu.test.js index 857830f20..7f07e8b0e 100644 --- a/test/unit/menu.test.js +++ b/test/unit/menu.test.js @@ -1,5 +1,5 @@ /* eslint-env qunit */ -import * as DomData from '../../src/js/utils/dom-data'; +import DomData from '../../src/js/utils/dom-data'; import MenuButton from '../../src/js/menu/menu-button.js'; import Menu from '../../src/js/menu/menu.js'; import CaptionSettingsMenuItem from '../../src/js/control-bar/text-track-controls/caption-settings-menu-item'; @@ -137,7 +137,7 @@ QUnit.test('should remove old event listeners when the menu item adds to the new * A reusable collection of assertions. */ function validateMenuEventListeners(watchedMenu) { - const eventData = DomData.getData(menuItem.eventBusEl_); + const eventData = DomData.get(menuItem.eventBusEl_); // `MenuButton`.`unpressButton` will be called when triggering click event on the menu item. const unpressButtonSpy = sinon.spy(menuButton, 'unpressButton'); // `MenuButton`.`focus` will be called when triggering click event on the menu item. diff --git a/test/unit/utils/dom-data.test.js b/test/unit/utils/dom-data.test.js deleted file mode 100644 index e1a0d3e95..000000000 --- a/test/unit/utils/dom-data.test.js +++ /dev/null @@ -1,63 +0,0 @@ -/* eslint-env qunit */ -import document from 'global/document'; -import * as DomData from '../../../src/js/utils/dom-data'; -import videojs from '../../../src/js/video.js'; -import window from 'global/window'; - -QUnit.module('dom-data'); - -QUnit.test('should get and remove data from an element', function(assert) { - const el = document.createElement('div'); - const data = DomData.getData(el); - - assert.strictEqual(typeof data, 'object', 'data object created'); - - // Add data - const testData = {asdf: 'fdsa'}; - - data.test = testData; - assert.strictEqual(DomData.getData(el).test, testData, 'data added'); - - // Remove all data - DomData.removeData(el); - - assert.notOk(DomData.hasData(el), 'cached item emptied'); -}); - -let memoryTestRun = false; - -QUnit.done(function(details) { - // don't run the extra dom data test on failures, there will likely be - // memory leaks - if (details.failed || memoryTestRun) { - return; - } - - memoryTestRun = true; - - QUnit.module('dom-data memory'); - - /** - * If this test fails you will want to add a debug statement - * in DomData.getData with the `id`. For instance if DomData.elData - * had 2 objects in it {5: {...}, 2003: {...} you would add: - * - * ```js - * if (id === 5) { - * debugger; - * } - * ``` - * to the tests to see what test. Then re-run the tests to see - * what leaking and where. - * - * > Note that the id can be off by 1-2 in either direction - * for larger guids, so you may have to account for that. - */ - QUnit.test('Memory is not leaking', function(assert) { - if (Object.keys(DomData.elData).length > 0) { - videojs.domData = DomData; - window.videojs = videojs; - } - assert.equal(Object.keys(DomData.elData).length, 0, 'no leaks, check videojs.domData.elData if failure'); - }); -}); diff --git a/test/unit/videojs-integration.test.js b/test/unit/videojs-integration.test.js index 2a36a57e0..7a2315ec1 100644 --- a/test/unit/videojs-integration.test.js +++ b/test/unit/videojs-integration.test.js @@ -2,7 +2,6 @@ import videojs from '../../src/js/video.js'; import window from 'global/window'; import document from 'global/document'; -import * as DomData from '../../src/js/utils/dom-data'; import * as Fn from '../../src/js/utils/fn'; /** @@ -70,54 +69,11 @@ QUnit.test('create a real player and dispose', function(assert) { player.muted(true); - const checkDomData = function() { - Object.keys(DomData.elData).forEach(function(elId) { - const data = DomData.elData[elId] || {}; - - Object.keys(data.handlers || {}).forEach(function(eventName) { - const listeners = data.handlers[eventName]; - const uniqueList = []; - - (listeners || []).forEach(function(listener) { - let add = true; - - for (let i = 0; i < uniqueList.length; i++) { - const obj = uniqueList[i]; - - if (listener.og_ && listener.cx_ && obj.fn === listener.og_ && obj.cx === listener.cx_) { - add = false; - break; - } - - if (listener.og_ && !listener.cx_ && obj.fn === listener.og_) { - add = false; - break; - } - - if (!listener.og_ && !listener.cx_ && obj.fn === listener) { - add = false; - break; - } - } - const obj = {fn: listener.og_ || listener, cx: listener.cx_}; - - if (add) { - uniqueList.push(obj); - assert.ok(true, `${elId}/${eventName}/${obj.fn.name} is unique`); - } else { - assert.ok(false, `${elId}/${eventName}/${obj.fn.name} is not unique`); - } - }); - }); - }); - }; - player.addTextTrack('captions', 'foo', 'en'); player.ready(function() { assert.ok(player.tech_, 'tech exists'); assert.equal(player.textTracks().length, 1, 'should have one text track'); - checkDomData(); player.dispose(); Object.keys(old).forEach(function(k) {