Skip to content

Commit

Permalink
fix: evented should cleanup dom data (#7350)
Browse files Browse the repository at this point in the history
  • Loading branch information
brandonocasey authored Jul 28, 2021
1 parent 774f9e7 commit ada25c4
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 4 deletions.
4 changes: 0 additions & 4 deletions src/js/component.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import window from 'global/window';
import evented from './mixins/evented';
import stateful from './mixins/stateful';
import * as Dom from './utils/dom.js';
import DomData from './utils/dom-data';
import * as Fn from './utils/fn.js';
import * as Guid from './utils/guid.js';
import {toTitleCase, toLowerCase} from './utils/string-cases.js';
Expand Down Expand Up @@ -178,9 +177,6 @@ class Component {
this.el_.parentNode.removeChild(this.el_);
}

if (DomData.has(this.el_)) {
DomData.delete(this.el_);
}
this.el_ = null;
}

Expand Down
6 changes: 6 additions & 0 deletions src/js/mixins/evented.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import * as Events from '../utils/events';
import * as Fn from '../utils/fn';
import * as Obj from '../utils/obj';
import EventTarget from '../event-target';
import DomData from '../utils/dom-data';
import log from '../utils/log';

const objName = (obj) => {
Expand Down Expand Up @@ -499,6 +500,11 @@ function evented(target, options = {}) {
// When any evented object is disposed, it removes all its listeners.
target.on('dispose', () => {
target.off();
[target, target.el_, target.eventBusEl_].forEach(function(val) {
if (val && DomData.has(val)) {
DomData.delete(val);
}
});
window.setTimeout(() => {
target.eventBusEl_ = null;
}, 0);
Expand Down
30 changes: 30 additions & 0 deletions test/unit/mixins/evented.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@
import sinon from 'sinon';
import evented from '../../../src/js/mixins/evented';
import log from '../../../src/js/utils/log';
import DomData from '../../../src/js/utils/dom-data';
import * as Dom from '../../../src/js/utils/dom';
import * as Obj from '../../../src/js/utils/obj';
import * as Events from '../../../src/js/utils/events.js';

// Common errors thrown by evented objects.
const errors = {
Expand Down Expand Up @@ -569,3 +571,31 @@ QUnit.test('off() can remove a listener from an array of events on a different t
target: b.eventBusEl_
});
});

QUnit.test('Removes DomData on dispose', function(assert) {
const el_ = Dom.createEl('div');
const eventBusEl_ = Dom.createEl('span', {className: 'vjs-event-bus'});
const target = evented({el_, eventBusEl_}, {eventBusKey: 'eventBusEl_'});

assert.equal(DomData.get(eventBusEl_).handlers.dispose.length, 1, 'event bus has dispose handler');
assert.notOk(DomData.get(target), 'evented obj has no handlers');
assert.notOk(DomData.get(el_), 'evented el_ has handlers');

target.on('foo', () => {});

assert.equal(DomData.get(eventBusEl_).handlers.foo.length, 1, 'foo handler added to bus');

Events.on(eventBusEl_, 'bar', () => {});
assert.equal(DomData.get(eventBusEl_).handlers.bar.length, 1, 'bar handler added to bus');

Events.on(el_, 'foo', () => {});
assert.equal(DomData.get(el_).handlers.foo.length, 1, 'foo handler added to el_');

Events.on(target, 'foo', () => {});
assert.equal(DomData.get(target).handlers.foo.length, 1, 'foo handler added to evented object');

target.trigger('dispose');
assert.notOk(DomData.get(eventBusEl_), 'eventBusEl_ DomData deleted');
assert.notOk(DomData.get(target), 'evented object DomData deleted');
assert.notOk(DomData.get(el_), 'el_ DomData deleted');
});

0 comments on commit ada25c4

Please sign in to comment.