Skip to content

Commit

Permalink
Split cleanup into two buckets.
Browse files Browse the repository at this point in the history
The following are the two cleanup buckets:

* `RENDERING_CLEANUP` is done when `teardownRenderingContext` is done
(which should **always** be before `teardownContext` is ran).
* `CLEANUP` is done _after_ everything else is destroyed and settled.

---

This refactor fixes the vast majority of issues on IE11. Prior to these
changes we were resetting the DOM test fixtures to their original values
_before_ we had properly cleaned up the DOM. This meant that when the
DOM cleanup actually does run, the various nodes being removed by
glimmer's internal cleanup system are not actually present in DOM.
Apparently, Chrome / FireFox / Safari / Edge are all fine with this
situation (and silently allow `node.removeChild(someThingNotInNode)`
without an error), but IE11 did properly (IMHO) error that
`someThingNotInNode` was not found.
  • Loading branch information
rwjblue committed Dec 15, 2017
1 parent a2b9130 commit aa1612d
Show file tree
Hide file tree
Showing 5 changed files with 55 additions and 28 deletions.
14 changes: 14 additions & 0 deletions addon-test-support/@ember/test-helpers/-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,17 @@ export function nextTickPromise() {
nextTick(resolve);
});
}

export function runDestroyablesFor(bucket, key) {
let destroyables = bucket[key];

if (!destroyables) {
return;
}

for (let i = 0; i < destroyables.length; i++) {
destroyables[i];
}

delete bucket[key];
}
6 changes: 6 additions & 0 deletions addon-test-support/@ember/test-helpers/setup-context.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { run } from '@ember/runloop';
import { set, setProperties, get, getProperties } from '@ember/object';
import { guidFor } from '@ember/object/internals';
import buildOwner from './build-owner';
import { _setupPromiseListeners } from './ext/rsvp';
import { _setupAJAXHooks } from './settled';
Expand Down Expand Up @@ -49,6 +50,8 @@ export function resumeTest() {
return context.resumeTest();
}

export const CLEANUP = Object.create(null);

/*
* Responsible for:
*
Expand All @@ -62,6 +65,9 @@ export default function(context, options = {}) {
Ember.testing = true;
setContext(context);

let contextGuid = guidFor(context);
CLEANUP[contextGuid] = [];

return nextTickPromise()
.then(() => {
let { resolver } = options;
Expand Down
20 changes: 12 additions & 8 deletions addon-test-support/@ember/test-helpers/setup-rendering-context.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { guidFor } from '@ember/object/internals';
import { run } from '@ember/runloop';
import Ember from 'ember';
import global from './global';
import { getContext } from './setup-context';
import { getContext, CLEANUP } from './setup-context';
import { nextTickPromise } from './-utils';
import settled from './settled';

Expand Down Expand Up @@ -38,16 +38,17 @@ export function clearRender() {
* - Adding `this.clearRender` to the provided context
*/
export default function(context) {
let guid = guidFor(context);
let contextGuid = guidFor(context);
RENDERING_CLEANUP[contextGuid] = [];

let testElementContainer = document.getElementById('ember-testing-container');
let fixtureResetValue = testElementContainer.innerHTML;

RENDERING_CLEANUP[guid] = [
() => {
testElementContainer.innerHTML = fixtureResetValue;
},
];
// push this into the final cleanup bucket, to be ran _after_ the owner
// is destroyed and settled (e.g. flushed run loops, etc)
CLEANUP[contextGuid].push(() => {
testElementContainer.innerHTML = fixtureResetValue;
});

return nextTickPromise().then(() => {
let { owner } = context;
Expand All @@ -67,7 +68,10 @@ export default function(context) {
: owner._lookupFactory('view:-outlet');
let OutletTemplate = owner.lookup('template:-outlet');
let toplevelView = OutletView.create();
RENDERING_CLEANUP[guid].push(() => toplevelView.destroy());

// push this into the rendering specific cleanup bucket, to be ran during
// `teardownRenderingContext` but before the owner itself is destroyed
RENDERING_CLEANUP[contextGuid].push(() => toplevelView.destroy());

let hasOutletTemplate = Boolean(OutletTemplate);
let outletState = {
Expand Down
31 changes: 20 additions & 11 deletions addon-test-support/@ember/test-helpers/teardown-context.js
Original file line number Diff line number Diff line change
@@ -1,23 +1,32 @@
import { guidFor } from '@ember/object/internals';
import { run } from '@ember/runloop';
import { _teardownPromiseListeners } from './ext/rsvp';
import { _teardownAJAXHooks } from './settled';
import { unsetContext } from './setup-context';
import { nextTickPromise } from './-utils';
import { unsetContext, CLEANUP } from './setup-context';
import { nextTickPromise, runDestroyablesFor } from './-utils';
import settled from './settled';
import Ember from 'ember';

export default function(context) {
return nextTickPromise().then(() => {
let { owner } = context;
return nextTickPromise()
.then(() => {
let { owner } = context;

_teardownPromiseListeners();
_teardownAJAXHooks();
_teardownPromiseListeners();
_teardownAJAXHooks();

run(owner, 'destroy');
Ember.testing = false;
run(owner, 'destroy');
Ember.testing = false;

unsetContext();
unsetContext();

return settled();
});
return settled();
})
.finally(() => {
let contextGuid = guidFor(context);

runDestroyablesFor(CLEANUP, contextGuid);

return settled();
});
}
Original file line number Diff line number Diff line change
@@ -1,19 +1,13 @@
import { guidFor } from '@ember/object/internals';
import { run } from '@ember/runloop';
import { RENDERING_CLEANUP } from './setup-rendering-context';
import { nextTickPromise } from './-utils';
import { nextTickPromise, runDestroyablesFor } from './-utils';
import settled from './settled';

export default function(context) {
return nextTickPromise().then(() => {
let guid = guidFor(context);
let destroyables = RENDERING_CLEANUP[guid];
let contextGuid = guidFor(context);

for (let i = 0; i < destroyables.length; i++) {
run(destroyables[i]);
}

delete RENDERING_CLEANUP[guid];
runDestroyablesFor(RENDERING_CLEANUP, contextGuid);

return settled();
});
Expand Down

0 comments on commit aa1612d

Please sign in to comment.