Support rerendering parent render nodes

In idiomatic Ember, you should render “data-down”: that is, parent
components should modify their children, but child components should
not mutate their parents, particularly during the rendering process.

By violating this rule, it is easy to trigger infinite loops or multiple
rendering passes that dramatically harm performance.

In certain situations, it was possible in previous versions of Ember to
rely on this behavior, and several regressions were reported. This
commit adds support for the old behavior and now emits a deprecation to
help users migrate to a more performant architecture.

In particular, we now track which render nodes have been rendered during
a rendering pass. If we detect that a node has been dirtied after it has
been rendered (and thus requires an additional rendering pass), we now
schedule a re-render appropriately but emit a warning.

Note that there *are* legitimate use cases for multi-pass rendering
(e.g., measuring rendered text to layout a parent container component)
and we intend to add a public API for handling this use case that can
still prevent inadvertent pathological performance.

package.json

@@ -23,7 +23,7 @@
     "express": "^4.5.0",
     "github": "^0.2.3",
     "glob": "~4.3.2",
-    "htmlbars": "0.13.18",
+    "htmlbars": "0.13.19",
     "qunit-extras": "^1.3.0",
     "qunitjs": "^1.16.0",
     "route-recognizer": "0.1.5",

packages/ember-htmlbars/lib/env.js

@@ -18,6 +18,7 @@ import getValue from "ember-htmlbars/hooks/get-value";
 import getCellOrValue from "ember-htmlbars/hooks/get-cell-or-value";
 import cleanupRenderNode from "ember-htmlbars/hooks/cleanup-render-node";
 import destroyRenderNode from "ember-htmlbars/hooks/destroy-render-node";
+import didRenderNode from "ember-htmlbars/hooks/did-render-node";
 import willCleanupTree from "ember-htmlbars/hooks/will-cleanup-tree";
 import didCleanupTree from "ember-htmlbars/hooks/did-cleanup-tree";
 import classify from "ember-htmlbars/hooks/classify";
@@ -53,6 +54,7 @@ merge(emberHooks, {
   destroyRenderNode: destroyRenderNode,
   willCleanupTree: willCleanupTree,
   didCleanupTree: didCleanupTree,
+  didRenderNode: didRenderNode,
   classify: classify,
   component: component,
   lookupHelper: lookupHelper,

packages/ember-htmlbars/lib/hooks/did-render-node.js

@@ -0,0 +1,3 @@
+export default function didRenderNode(morph, env) {
+  env.renderedNodes[morph.guid] = true;
+}

packages/ember-htmlbars/lib/hooks/link-render-node.js

@@ -34,13 +34,13 @@ export default function linkRenderNode(renderNode, env, scope, path, params, has
 
   if (params && params.length) {
     for (var i = 0; i < params.length; i++) {
-      subscribe(renderNode, scope, params[i]);
+      subscribe(renderNode, env, scope, params[i]);
     }
   }
 
   if (hash) {
     for (var key in hash) {
-      subscribe(renderNode, scope, hash[key]);
+      subscribe(renderNode, env, scope, hash[key]);
     }
   }
 

packages/ember-htmlbars/lib/morphs/morph.js

@@ -2,13 +2,15 @@ import DOMHelper from "dom-helper";
 import o_create from 'ember-metal/platform/create';
 
 var HTMLBarsMorph = DOMHelper.prototype.MorphClass;
+let guid = 1;
 
 function EmberMorph(DOMHelper, contextualElement) {
   this.HTMLBarsMorph$constructor(DOMHelper, contextualElement);
 
   this.emberView = null;
   this.emberToDestroy = null;
   this.streamUnsubscribers = null;
+  this.guid = guid++;
 
   // A component can become dirty either because one of its
   // attributes changed, or because it was re-rendered. If any part
@@ -49,4 +51,8 @@ proto.cleanup = function() {
   }
 };
 
+proto.didRender = function(env, scope) {
+  env.renderedNodes[this.guid] = true;
+};
+
 export default EmberMorph;

packages/ember-htmlbars/lib/system/render-view.js

@@ -7,6 +7,7 @@ export function renderHTMLBarsBlock(view, block, renderNode) {
   var env = {
     lifecycleHooks: [],
     renderedViews: [],
+    renderedNodes: {},
     view: view,
     outletState: view.outletState,
     container: view.container,

packages/ember-htmlbars/lib/utils/subscribe.js

@@ -1,6 +1,6 @@
-import { isStream } from "ember-metal/streams/utils";
+import { isStream, labelFor } from "ember-metal/streams/utils";
 
-export default function subscribe(node, scope, stream) {
+export default function subscribe(node, env, scope, stream) {
   if (!isStream(stream)) { return; }
   var component = scope.component;
   var unsubscribers = node.streamUnsubscribers = node.streamUnsubscribers || [];
@@ -21,6 +21,6 @@ export default function subscribe(node, scope, stream) {
       node.shouldReceiveAttrs = true;
     }
 
-    node.ownerNode.emberView.scheduleRevalidate();
+    node.ownerNode.emberView.scheduleRevalidate(node, labelFor(stream));
   }));
 }

packages/ember-htmlbars/tests/integration/component_invocation_test.js

@@ -453,6 +453,8 @@ QUnit.test('implementing `render` allows pushing into a string buffer', function
 });
 
 QUnit.test("comopnent should rerender when a property is changed during children's rendering", function() {
+  expectDeprecation(/twice in a single render/);
+
   var outer, middle;
 
   registry.register('component:x-outer', Component.extend({
@@ -502,6 +504,8 @@ QUnit.test("comopnent should rerender when a property is changed during children
 });
 
 QUnit.test("comopnent should rerender when a property (with a default) is changed during children's rendering", function() {
+  expectDeprecation(/modified value twice in a single render/);
+
   var outer, middle;
 
   registry.register('component:x-outer', Component.extend({
@@ -538,7 +542,7 @@ QUnit.test("comopnent should rerender when a property (with a default) is change
   runAppend(view);
 
   equal(view.$('#inner-value').text(), '1', 'initial render of inner');
-  equal(view.$('#middle-value').text(), '1', 'initial render of middle');
+  equal(view.$('#middle-value').text(), '', 'initial render of middle (observers do not run during init)');
 
   run(() => outer.set('value', 2));
 

packages/ember-metal-views/lib/renderer.js

@@ -89,6 +89,7 @@ Renderer.prototype.ensureViewNotRendering =
 
 Renderer.prototype.clearRenderedViews =
   function Renderer_clearRenderedViews(env) {
+    env.renderedNodes = {};
     env.renderedViews.length = 0;
   };
 

packages/ember-routing-views/lib/views/outlet.js

@@ -26,7 +26,7 @@ export var CoreOutletView = View.extend({
       this.dirtyOutlets();
       this._outlets = [];
 
-      this.scheduleRevalidate();
+      this.scheduleRevalidate(null, null);
     }
   },
 

packages/ember-views/lib/views/states/has_element.js

@@ -40,7 +40,8 @@ merge(hasElement, {
       }
       node.isDirty = true;
     });
-    renderNode.ownerNode.emberView.scheduleRevalidate();
+
+    renderNode.ownerNode.emberView.scheduleRevalidate(renderNode, view.toString(), "rerendering");
   },
 
   cleanup(view) {

packages/ember-views/lib/views/view.js

@@ -1330,7 +1330,17 @@ var View = CoreView.extend(
     this.scheduledRevalidation = false;
   },
 
-  scheduleRevalidate() {
+  scheduleRevalidate(node, label, manualRerender) {
+    if (node && !this._dispatching && node.guid in this.env.renderedNodes) {
+      if (manualRerender) {
+        Ember.deprecate(`You manually rerendered ${label} (a parent component) from a child component during the rendering process. This rarely worked in Ember 1.x and will be removed in Ember 2.0`);
+      } else {
+        Ember.deprecate(`You modified ${label} twice in a single render. This was unreliable in Ember 1.x and will be removed in Ember 2.0`);
+      }
+      run.scheduleOnce('render', this, this.revalidate);
+      return;
+    }
+
     Ember.deprecate(`A property of ${this} was modified inside the ${this._dispatching} hook. You should never change properties on components, services or models during ${this._dispatching} because it causes significant performance degradation.`, !this._dispatching);
 
     if (!this.scheduledRevalidation || this._dispatching) {