From 53b9f19a8fd737ed9dfede59316ec83825de44d2 Mon Sep 17 00:00:00 2001 From: Tom Dale Date: Wed, 3 Jun 2015 16:57:08 -0700 Subject: [PATCH 1/4] Change `assign` to not use a for-in loop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Apparently, v8’s JIT engineers require that we, as JavaScript developers perform this very simple transformation, since they do not seem capable of performing it themselves. --- packages/ember-metal/lib/merge.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/ember-metal/lib/merge.js b/packages/ember-metal/lib/merge.js index d56a4c990f2..0bed6ea3345 100644 --- a/packages/ember-metal/lib/merge.js +++ b/packages/ember-metal/lib/merge.js @@ -38,8 +38,11 @@ export function assign(original, ...args) { let arg = args[i]; if (!arg) { continue; } - for (let prop in arg) { - if (arg.hasOwnProperty(prop)) { original[prop] = arg[prop]; } + let updates = keys(arg); + + for (let i=0, l=updates.length; i Date: Wed, 3 Jun 2015 16:59:13 -0700 Subject: [PATCH 2/4] Fix bug where parent/ownerView was incorrect MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, `parentView` and `ownerView` were set based on `scope`, which is the “owner tree”. However, it should be based on the component tree instead. This commit properly wires up `parentView` and `ownerView`, while maintaining the correct semantics for the `{{view.foo}}` keyword in both legacy views and components. --- .../ember-htmlbars/lib/hooks/component.js | 3 +- .../tests/component_registration_test.js | 32 +++++++++++++++++-- 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/packages/ember-htmlbars/lib/hooks/component.js b/packages/ember-htmlbars/lib/hooks/component.js index edb210fb240..3a469a707fa 100644 --- a/packages/ember-htmlbars/lib/hooks/component.js +++ b/packages/ember-htmlbars/lib/hooks/component.js @@ -17,8 +17,7 @@ export default function componentHook(renderNode, env, scope, _tagName, params, isAngleBracket = true; } - var read = env.hooks.getValue; - var parentView = read(scope.view); + var parentView = env.view; var manager = ComponentNodeManager.create(renderNode, env, { tagName, diff --git a/packages/ember/tests/component_registration_test.js b/packages/ember/tests/component_registration_test.js index c6c2c5f472f..1b77744eb6f 100644 --- a/packages/ember/tests/component_registration_test.js +++ b/packages/ember/tests/component_registration_test.js @@ -2,6 +2,7 @@ import "ember"; import compile from "ember-template-compiler/system/compile"; import helpers from "ember-htmlbars/helpers"; +import { OutletView } from "ember-routing-views/views/outlet"; var App, registry, container; var originalHelpers; @@ -40,7 +41,7 @@ QUnit.module("Application Lifecycle - Component Registration", { teardown: cleanup }); -function boot(callback) { +function boot(callback, startURL="/") { Ember.run(function() { App = Ember.Application.create({ name: 'App', @@ -63,7 +64,7 @@ function boot(callback) { Ember.run(App, 'advanceReadiness'); Ember.run(function() { - router.handleURL('/'); + router.handleURL(startURL); }); } @@ -348,3 +349,30 @@ QUnit.test("Components trigger actions in the components context when called fro Ember.$('#fizzbuzz', "#wrapper").click(); }); + +QUnit.test("Components receive the top-level view as their ownerView", function(assert) { + Ember.TEMPLATES.application = compile("{{outlet}}"); + Ember.TEMPLATES.index = compile("{{my-component}}"); + Ember.TEMPLATES['components/my-component'] = compile('
'); + + let component; + + boot(function() { + registry.register('component:my-component', Ember.Component.extend({ + init() { + this._super(); + component = this; + } + })); + }); + + // Theses tests are intended to catch a regression where the owner view was + // not configured properly. Future refactors may break these tests, which + // should not be considered a breaking change to public APIs. + let ownerView = component.ownerView; + assert.ok(ownerView, "owner view was set"); + assert.ok(ownerView instanceof OutletView, "owner view has no parent view"); + assert.notStrictEqual(component, ownerView, "owner view is not itself"); + + assert.ok(ownerView._outlets, "owner view has an internal array of outlets"); +}); From b9020834ad51cd414678c8b3101c92907b539f2e Mon Sep 17 00:00:00 2001 From: Tom Dale Date: Wed, 3 Jun 2015 17:18:19 -0700 Subject: [PATCH 3/4] Fix a case where AttrMorph was being extended --- packages/ember-htmlbars/lib/morphs/attr-morph.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/ember-htmlbars/lib/morphs/attr-morph.js b/packages/ember-htmlbars/lib/morphs/attr-morph.js index 790574207e3..cf0e5989b3b 100644 --- a/packages/ember-htmlbars/lib/morphs/attr-morph.js +++ b/packages/ember-htmlbars/lib/morphs/attr-morph.js @@ -12,6 +12,8 @@ export var styleWarning = '' + function EmberAttrMorph(element, attrName, domHelper, namespace) { HTMLBarsAttrMorph.call(this, element, attrName, domHelper, namespace); + + this.streamUnsubscribers = null; } var proto = EmberAttrMorph.prototype = o_create(HTMLBarsAttrMorph.prototype); From d1d993d1becc0802169790c7858e7074592adbcc Mon Sep 17 00:00:00 2001 From: Tom Dale Date: Wed, 3 Jun 2015 17:21:24 -0700 Subject: [PATCH 4/4] Make the `env` more generally consistent (JIT) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, the `env` was a bag of properties, and got frequently extended or modified (in largely predictable ways). The consequence is that even though the environment is conceptually a single concept, it got treated internally as a megamorphic shapeless entity. For what it’s worth, this JIT confusion was somewhat mirrored by a confusion when reading the code: it was never precisely clear what the `env` would look like from the perspective of a human reader. This commit changes the environment to be a single class. There are only two kinds of child environments (a change in the `view` and a change to `outletState`), and there are now methods on `env` to make that simple and predictable. The consequence is that all methods that take an `env` now go down a fast path when accessing it, and human readers of the code can feel confident about what the environment looks like. --- package.json | 2 +- .../lib/keywords/real_outlet.js | 4 +- .../node-managers/component-node-manager.js | 6 +- .../lib/node-managers/view-node-manager.js | 6 +- .../ember-htmlbars/lib/system/render-env.js | 56 +++++++++++++++++++ .../ember-htmlbars/lib/system/render-view.js | 16 +----- packages/ember-metal-views/lib/renderer.js | 13 ++++- .../lib/keywords/render.js | 4 +- 8 files changed, 78 insertions(+), 29 deletions(-) create mode 100644 packages/ember-htmlbars/lib/system/render-env.js diff --git a/package.json b/package.json index 222f09a2170..7ac40e55e21 100644 --- a/package.json +++ b/package.json @@ -23,7 +23,7 @@ "express": "^4.5.0", "github": "^0.2.3", "glob": "~4.3.2", - "htmlbars": "0.13.22", + "htmlbars": "0.13.25", "qunit-extras": "^1.3.0", "qunitjs": "^1.16.0", "route-recognizer": "0.1.5", diff --git a/packages/ember-htmlbars/lib/keywords/real_outlet.js b/packages/ember-htmlbars/lib/keywords/real_outlet.js index 4ab625cee82..25a96916203 100644 --- a/packages/ember-htmlbars/lib/keywords/real_outlet.js +++ b/packages/ember-htmlbars/lib/keywords/real_outlet.js @@ -27,8 +27,8 @@ export default { return { outletState: selectedOutletState, hasParentOutlet: env.hasParentOutlet }; }, - childEnv(state) { - return { outletState: state.outletState && state.outletState.outlets, hasParentOutlet: true }; + childEnv(state, env) { + return env.childWithOutletState(state.outletState && state.outletState.outlets, true); }, isStable(lastState, nextState) { diff --git a/packages/ember-htmlbars/lib/node-managers/component-node-manager.js b/packages/ember-htmlbars/lib/node-managers/component-node-manager.js index cf3c0e2df5e..e59a876a0eb 100644 --- a/packages/ember-htmlbars/lib/node-managers/component-node-manager.js +++ b/packages/ember-htmlbars/lib/node-managers/component-node-manager.js @@ -157,9 +157,7 @@ ComponentNodeManager.prototype.render = function(_env, visitor) { var { component, attrs } = this; return instrument(component, function() { - let env = _env; - - env = assign({ view: component }, env); + let env = _env.childWithView(component); var snapshot = takeSnapshot(attrs); env.renderer.componentInitAttrs(this.component, snapshot); @@ -210,7 +208,7 @@ ComponentNodeManager.prototype.rerender = function(_env, attrs, visitor) { var snapshot = takeSnapshot(attrs); if (component._renderNode.shouldReceiveAttrs) { - env.renderer.componentUpdateAttrs(component, component.attrs, snapshot); + env.renderer.componentUpdateAttrs(component, snapshot); if (!component._isAngleBracket) { setProperties(component, mergeBindings({}, shadowedAttrs(component, snapshot))); diff --git a/packages/ember-htmlbars/lib/node-managers/view-node-manager.js b/packages/ember-htmlbars/lib/node-managers/view-node-manager.js index 6bb438aa510..76d1d2b0bbb 100644 --- a/packages/ember-htmlbars/lib/node-managers/view-node-manager.js +++ b/packages/ember-htmlbars/lib/node-managers/view-node-manager.js @@ -91,8 +91,7 @@ ViewNodeManager.prototype.render = function(env, attrs, visitor) { var newEnv = env; if (component) { - newEnv = merge({}, env); - newEnv.view = component; + newEnv = env.childWithView(component); } if (component) { @@ -124,8 +123,7 @@ ViewNodeManager.prototype.rerender = function(env, attrs, visitor) { return instrument(component, function() { var newEnv = env; if (component) { - newEnv = merge({}, env); - newEnv.view = component; + newEnv = env.childWithView(component); var snapshot = takeSnapshot(attrs); diff --git a/packages/ember-htmlbars/lib/system/render-env.js b/packages/ember-htmlbars/lib/system/render-env.js new file mode 100644 index 00000000000..10867645ed5 --- /dev/null +++ b/packages/ember-htmlbars/lib/system/render-env.js @@ -0,0 +1,56 @@ +import defaultEnv from "ember-htmlbars/env"; + +export default function RenderEnv(options) { + this.lifecycleHooks = options.lifecycleHooks || []; + this.renderedViews = options.renderedViews || []; + this.renderedNodes = options.renderedNodes || {}; + this.hasParentOutlet = options.hasParentOutlet || false; + + this.view = options.view; + this.outletState = options.outletState; + this.container = options.container; + this.renderer = options.renderer; + this.dom = options.dom; + + this.hooks = defaultEnv.hooks; + this.helpers = defaultEnv.helpers; + this.useFragmentCache = defaultEnv.useFragmentCache; +} + +RenderEnv.build = function(view) { + return new RenderEnv({ + view: view, + outletState: view.outletState, + container: view.container, + renderer: view.renderer, + dom: view.renderer._dom + }); +}; + +RenderEnv.prototype.childWithView = function(view) { + return new RenderEnv({ + view: view, + outletState: this.outletState, + container: this.container, + renderer: this.renderer, + dom: this.dom, + lifecycleHooks: this.lifecycleHooks, + renderedViews: this.renderedViews, + renderedNodes: this.renderedNodes, + hasParentOutlet: this.hasParentOutlet + }); +}; + +RenderEnv.prototype.childWithOutletState = function(outletState, hasParentOutlet=this.hasParentOutlet) { + return new RenderEnv({ + view: this.view, + outletState: outletState, + container: this.container, + renderer: this.renderer, + dom: this.dom, + lifecycleHooks: this.lifecycleHooks, + renderedViews: this.renderedViews, + renderedNodes: this.renderedNodes, + hasParentOutlet: hasParentOutlet + }); +}; diff --git a/packages/ember-htmlbars/lib/system/render-view.js b/packages/ember-htmlbars/lib/system/render-view.js index 365fd201bf0..77dc21b5a76 100644 --- a/packages/ember-htmlbars/lib/system/render-view.js +++ b/packages/ember-htmlbars/lib/system/render-view.js @@ -1,22 +1,10 @@ -import defaultEnv from "ember-htmlbars/env"; import ViewNodeManager, { createOrUpdateComponent } from "ember-htmlbars/node-managers/view-node-manager"; +import RenderEnv from "ember-htmlbars/system/render-env"; // This function only gets called once per render of a "root view" (`appendTo`). Otherwise, // HTMLBars propagates the existing env and renders templates for a given render node. export function renderHTMLBarsBlock(view, block, renderNode) { - var env = { - lifecycleHooks: [], - renderedViews: [], - renderedNodes: {}, - view: view, - outletState: view.outletState, - container: view.container, - renderer: view.renderer, - dom: view.renderer._dom, - hooks: defaultEnv.hooks, - helpers: defaultEnv.helpers, - useFragmentCache: defaultEnv.useFragmentCache - }; + var env = RenderEnv.build(view); view.env = env; createOrUpdateComponent(view, {}, null, renderNode, env); diff --git a/packages/ember-metal-views/lib/renderer.js b/packages/ember-metal-views/lib/renderer.js index ebf7c1000ce..d5098f89131 100755 --- a/packages/ember-metal-views/lib/renderer.js +++ b/packages/ember-metal-views/lib/renderer.js @@ -1,6 +1,8 @@ import run from "ember-metal/run_loop"; import { get } from "ember-metal/property_get"; import { set } from "ember-metal/property_set"; +import { assign } from "ember-metal/merge"; +import setProperties from "ember-metal/set_properties"; import buildComponentTemplate from "ember-views/system/build-component-template"; import { indexOf } from "ember-metal/enumerable_utils"; //import { deprecation } from "ember-views/compat/attrs-proxy"; @@ -161,8 +163,15 @@ Renderer.prototype.updateAttrs = function (view, attrs) { this.setAttrs(view, attrs); }; // setting new attrs -Renderer.prototype.componentUpdateAttrs = function (component, oldAttrs, newAttrs) { - set(component, 'attrs', newAttrs); +Renderer.prototype.componentUpdateAttrs = function (component, newAttrs) { + let oldAttrs = null; + + if (component.attrs) { + oldAttrs = assign({}, component.attrs); + setProperties(component.attrs, newAttrs); + } else { + set(component, 'attrs', newAttrs); + } component.trigger('didUpdateAttrs', { oldAttrs, newAttrs }); component.trigger('didReceiveAttrs', { oldAttrs, newAttrs }); diff --git a/packages/ember-routing-htmlbars/lib/keywords/render.js b/packages/ember-routing-htmlbars/lib/keywords/render.js index a543b9c5a1b..ed5b9783680 100644 --- a/packages/ember-routing-htmlbars/lib/keywords/render.js +++ b/packages/ember-routing-htmlbars/lib/keywords/render.js @@ -32,8 +32,8 @@ export default { }; }, - childEnv(state) { - return { outletState: state.childOutletState }; + childEnv(state, env) { + return env.childWithOutletState(state.childOutletState); }, isStable(lastState, nextState) {