Skip to content

Commit

Permalink
Merge pull request #87 from teddyzeenny/fix-leak
Browse files Browse the repository at this point in the history
Fix memory leak caused by the click event listener
  • Loading branch information
GavinJoyce authored Oct 19, 2017
2 parents dea444b + 2ff82a1 commit 4887e9f
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 6 deletions.
16 changes: 10 additions & 6 deletions app/instance-initializers/ember-href-to.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import HrefTo from 'ember-href-to/href-to';

let hrefToClickHandler;
function closestLink(el) {
if (el.closest) {
return el.closest('a');
Expand All @@ -17,11 +16,7 @@ export default {
initialize(applicationInstance) {
// we only want this to run in the browser, not in fastboot
if (typeof(FastBoot) === "undefined") {
if (hrefToClickHandler !== undefined) {
document.body.removeEventListener('click', hrefToClickHandler);
}

hrefToClickHandler = function _hrefToClickHandler(e) {
let hrefToClickHandler = function _hrefToClickHandler(e) {
let link = e.target.tagName === 'A' ? e.target : closestLink(e.target);
if (link) {
let hrefTo = new HrefTo(applicationInstance, e, link);
Expand All @@ -30,6 +25,15 @@ export default {
};

document.body.addEventListener('click', hrefToClickHandler);

// Teardown on app destruction: clean up the event listener to avoid
// memory leaks.
applicationInstance.reopen({
willDestroy() {
document.body.removeEventListener('click', hrefToClickHandler);
return this._super(...arguments);
}
});
}
}
};
23 changes: 23 additions & 0 deletions tests/acceptance/href-to-test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { test } from 'qunit';
import moduleForAcceptance from '../../tests/helpers/module-for-acceptance';
import $ from 'jquery';
import destroyApp from '../../tests/helpers/destroy-app';

function leftClick(selector) {
triggerEvent(selector, 'click', { which: 1 });
Expand Down Expand Up @@ -122,3 +123,25 @@ test('[BUGFIX] it works with the `click` acceptance helper', function(assert) {
assertAnchorIsActive('#link-to-links a:contains(About)', assert);
});
});

test('The event listener does\'nt leak after the app is destroyed', function(assert) {
// Boot the app
visit('/');

andThen(app => {
// Destroy the app
destroyApp(app);
// Prevent an actual redirect if the test fails (when an error occurs
// inside an event handler `preventDefault()` never happens therefore
// causing an actual redirect).
let preventDefault = e => e.preventDefault();
document.body.addEventListener('click', preventDefault);

// Click on a link with a recognizable URL
let a = $('<a href="/about">').appendTo('body')[0].click();
assert.ok(true, 'no errors thrown due to attempting to handle a URL in a destroyed application');

$(a).remove();
document.body.removeEventListener('click', preventDefault);
});
});

0 comments on commit 4887e9f

Please sign in to comment.