[BUGFIX beta] Ensure roots added during rendering work properly.

When a new root is added during the rendering process (i.e. from
`didInsertElement` of one component you `.appendTo` another one), we
need to:

1) Ensure that we do not initiate a new transaction (`env.begin()` /
`env.commit()`)
2) Ensure that we trigger _renderRoots again if roots were added. This
ensures the new roots are properly rendered.

packages/ember-glimmer/lib/renderer.js

@@ -61,6 +61,8 @@ class RootState {
     this.root = root;
     this.result = undefined;
     this.shouldReflush = false;
+    this.destroyed = false;
+    this._removing = false;
 
     let options = this.options = {
       alwaysRevalidate: false
@@ -83,6 +85,8 @@ class RootState {
   destroy() {
     let { result, env } = this;
 
+    this.destroyed = true;
+
     this.env = null;
     this.root = null;
     this.result = null;
@@ -166,6 +170,8 @@ export class Renderer {
     this._destroyed = false;
     this._roots = [];
     this._lastRevision = null;
+    this._isRenderingRoots = false;
+    this._removedRoots = [];
   }
 
   // renderer HOOKS
@@ -242,13 +248,8 @@ export class Renderer {
       // check if the view being removed is a root view
       if (root.isFor(view)) {
         root.destroy();
-        roots.splice(i, 1);
       }
     }
-
-    if (this._roots.length === 0) {
-      deregister(this);
-    }
   }
 
   destroy() {
@@ -286,24 +287,34 @@ export class Renderer {
   }
 
   _renderRoots() {
-    let { _roots: roots, _env: env } = this;
-    let globalShouldReflush;
-
-    // ensure that for the first iteration of the loop
-    // each root is processed
-    let initial = true;
+    let { _roots: roots, _env: env, _removedRoots: removedRoots } = this;
+    let globalShouldReflush, initialRootsLength;
 
     do {
       env.begin();
+
+      // ensure that for the first iteration of the loop
+      // each root is processed
+      initialRootsLength = roots.length;
       globalShouldReflush = false;
 
       for (let i = 0; i < roots.length; i++) {
         let root = roots[i];
+
+        if (root.destroyed) {
+          // add to the list of roots to be removed
+          // they will be removed from `this._roots` later
+          removedRoots.push(root);
+
+          // skip over roots that have been marked as destroyed
+          continue;
+        }
+
         let { shouldReflush } = root;
 
         // when processing non-initial reflush loops,
         // do not process more roots than needed
-        if (!initial && !shouldReflush) {
+        if (i >= initialRootsLength && !shouldReflush) {
           continue;
         }
 
@@ -317,16 +328,37 @@ export class Renderer {
       }
 
       env.commit();
+    } while (globalShouldReflush || roots.length > initialRootsLength);
+
+    // remove any roots that were destroyed during this transaction
+    while (removedRoots.length) {
+      let root = removedRoots.pop();
+
+      let rootIndex = roots.indexOf(root);
+      roots.splice(rootIndex, 1);
+    }
 
-      initial = false;
-    } while (globalShouldReflush);
+    if (this._roots.length === 0) {
+      deregister(this);
+    }
   }
 
   _renderRootsTransaction() {
+    if (this._isRenderingRoots) {
+      // currently rendering roots, a new root was added and will
+      // be processed by the existing _renderRoots invocation
+      return;
+    }
+
+    // used to prevent calling _renderRoots again (see above)
+    // while we are actively rendering roots
+    this._isRenderingRoots = true;
+
     try {
       this._renderRoots();
     } finally {
       this._lastRevision = CURRENT_TAG.value();
+      this._isRenderingRoots = false;
     }
   }
 
@@ -337,8 +369,11 @@ export class Renderer {
       root.destroy();
     }
 
+    this._removedRoots.length = 0;
     this._roots = null;
 
+    // if roots were present before destroying
+    // deregister this renderer instance
     if (roots.length) {
       deregister(this);
     }

packages/ember-glimmer/tests/integration/components/append-test.js

@@ -1,7 +1,8 @@
 import { set } from 'ember-metal';
 import { jQuery } from 'ember-views';
 import { moduleFor, RenderingTest } from '../../utils/test-case';
-import { Component } from '../../utils/helpers';
+import { Component, compile } from '../../utils/helpers';
+import { strip } from '../../utils/abstract-test-case';
 
 class AbstractAppendTest extends RenderingTest {
 
@@ -195,6 +196,155 @@ class AbstractAppendTest extends RenderingTest {
     this.assert.equal(willDestroyCalled, 2);
   }
 
+  ['@test can appendTo while rendering'](assert) {
+    let owner = this.owner;
+
+    let append = (component) => {
+      return this.append(component);
+    };
+
+    let wrapper1, wrapper2, element1, element2;
+    this.registerComponent('first-component', {
+      ComponentClass: Component.extend({
+        layout: compile('component-one'),
+
+        didInsertElement() {
+          element1 = this.element;
+          let SecondComponent = owner._lookupFactory('component:second-component');
+
+          wrapper2 = append(SecondComponent.create());
+        }
+      })
+    });
+
+    this.registerComponent('second-component', {
+      ComponentClass: Component.extend({
+        layout: compile(`component-two`),
+
+        didInsertElement() {
+          element2 = this.element;
+        }
+      })
+    });
+
+    let FirstComponent = this.owner._lookupFactory('component:first-component');
+
+    this.runTask(() => wrapper1 = append(FirstComponent.create()));
+
+    this.assertComponentElement(element1, { content: 'component-one' });
+    this.assertComponentElement(element2, { content: 'component-two' });
+  }
+
+  ['@test can appendTo and remove while rendering'](assert) {
+    let owner = this.owner;
+
+    let append = (component) => {
+      return this.append(component);
+    };
+
+    let element1, element2, element3, element4, component1, component2;
+    this.registerComponent('foo-bar', {
+      ComponentClass: Component.extend({
+        layout: compile('foo-bar'),
+
+        init() {
+          this._super(...arguments);
+          component1 = this;
+        },
+
+        didInsertElement() {
+          element1 = this.element;
+          let OtherRoot = owner._lookupFactory('component:other-root');
+
+          this._instance = OtherRoot.create({
+            didInsertElement() {
+              element2 = this.element;
+            }
+          });
+
+          append(this._instance);
+        },
+
+        willDestroy() {
+          this._instance.destroy();
+        }
+      })
+    });
+
+    this.registerComponent('baz-qux', {
+      ComponentClass: Component.extend({
+        layout: compile('baz-qux'),
+
+        init() {
+          this._super(...arguments);
+          component2 = this;
+        },
+
+        didInsertElement() {
+          element3 = this.element;
+          let OtherRoot = owner._lookupFactory('component:other-root');
+
+          this._instance = OtherRoot.create({
+            didInsertElement() {
+              element4 = this.element;
+            }
+          });
+
+          append(this._instance);
+        },
+
+        willDestroy() {
+          this._instance.destroy();
+        }
+      })
+    });
+
+    let instantiatedRoots = 0;
+    let destroyedRoots = 0;
+    this.registerComponent('other-root', {
+      ComponentClass: Component.extend({
+        layout: compile(`fake-thing: {{counter}}`),
+        init() {
+          this._super(...arguments);
+          this.counter = instantiatedRoots++;
+        },
+        willDestroy() {
+          destroyedRoots++;
+          this._super(...arguments);
+        }
+      })
+    });
+
+    this.render(strip`
+      {{#if showFooBar}}
+        {{foo-bar}}
+      {{else}}
+        {{baz-qux}}
+      {{/if}}
+    `, { showFooBar: true });
+
+    this.assertComponentElement(element1, { });
+    this.assertComponentElement(element2, { content: 'fake-thing: 0' });
+    assert.equal(instantiatedRoots, 1);
+
+    this.assertStableRerender();
+
+    this.runTask(() => set(this.context, 'showFooBar', false));
+
+    assert.equal(instantiatedRoots, 2);
+    assert.equal(destroyedRoots, 1);
+
+    this.assertComponentElement(element3, { });
+    this.assertComponentElement(element4, { content: 'fake-thing: 1' });
+
+    this.runTask(() => {
+      component1.destroy();
+      component2.destroy();
+    });
+
+    assert.equal(instantiatedRoots, 2);
+    assert.equal(destroyedRoots, 2);
+  }
 }
 
 moduleFor('append: no arguments (attaching to document.body)', class extends AbstractAppendTest {
@@ -258,32 +408,6 @@ moduleFor('appendTo: with multiple components', class extends AbstractAppendTest
     this.didAppend(component);
     return jQuery('#qunit-fixture')[0];
   }
-
-  ['@test can appendTo while rendering'](assert) {
-    assert.expect(0);
-
-    let owner = this.owner;
-
-    this.registerComponent('first-component', {
-      ComponentClass: Component.extend({
-        layoutName: 'components/component-one',
-
-        didInsertElement() {
-          let SecondComponent = owner._lookupFactory('component:second-component');
-          SecondComponent.create().appendTo('#qunit-fixture');
-        }
-      })
-    });
-
-    this.registerComponent('second-component', {
-      ComponentClass: Component.extend()
-    });
-
-    let FirstComponent = this.owner._lookupFactory('component:first-component');
-
-    this.append(FirstComponent.create());
-  }
-
 });
 
 moduleFor('renderToElement: no arguments (defaults to a body context)', class extends AbstractAppendTest {