From fbe3f03ce3a692b6c42068f475055aa3d32e3475 Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Thu, 30 Jul 2015 22:21:09 -0400 Subject: [PATCH 1/3] [BUGFIX release] Avoid positionalParams changing attrs after create. --- .../node-managers/component-node-manager.js | 72 +++++--- .../integration/component_invocation_test.js | 156 +++++++++++++++++- 2 files changed, 195 insertions(+), 33 deletions(-) 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 f19055cfcd9..d625891474b 100644 --- a/packages/ember-htmlbars/lib/node-managers/component-node-manager.js +++ b/packages/ember-htmlbars/lib/node-managers/component-node-manager.js @@ -75,8 +75,10 @@ ComponentNodeManager.create = function(renderNode, env, options) { createOptions._deprecatedFlagForBlockProvided = true; } + let proto = extractPositionalParams(renderNode, component, params, attrs); + // Instantiate the component - component = createComponent(component, isAngleBracket, createOptions, renderNode, env, attrs); + component = createComponent(component, isAngleBracket, createOptions, renderNode, env, attrs, proto); // If the component specifies its template via the `layout` or `template` // properties instead of using the template looked up in the container, get @@ -85,7 +87,6 @@ ComponentNodeManager.create = function(renderNode, env, options) { layout = result.layout || layout; templates = result.templates || templates; - extractPositionalParams(renderNode, component, params, attrs); let results = buildComponentTemplate( { layout, component, isAngleBracket }, attrs, { templates, scope: parentScope } @@ -95,30 +96,52 @@ ComponentNodeManager.create = function(renderNode, env, options) { }; function extractPositionalParams(renderNode, component, params, attrs) { - if (component.positionalParams) { - // if the component is rendered via {{component}} helper, the first - // element of `params` is the name of the component, so we need to - // skip that when the positional parameters are constructed - const paramsStartIndex = renderNode.state.isComponentHelper ? 1 : 0; - const positionalParams = component.positionalParams; - const isNamed = typeof positionalParams === 'string'; - let paramsStream; + let positionalParams = component.positionalParams; + let proto; + + if (!positionalParams) { + proto = component.proto(); + positionalParams = proto.positionalParams; + + Ember.deprecate( + 'Calling `var Thing = Ember.Component.extend({ positionalParams: [\'a\', \'b\' ]});` ' + + 'is deprecated in favor of `Thing.reopenClass({ positionalParams: [\'a\', \'b\'] });', + !positionalParams, + { id: 'ember-htmlbars.component-positional-params', until: '2.0.0' } + ); + } - if (isNamed) { - paramsStream = new Stream(() => { - return readArray(params.slice(paramsStartIndex)); - }, 'params'); + if (positionalParams) { + processPositionalParams(renderNode, positionalParams, params, attrs); + } - attrs[positionalParams] = paramsStream; - } + // returns `proto` here so that we can avoid doing this + // twice for each initial render per component (it is also needed in `createComponent`) + return proto; +} - for (let i = 0; i < positionalParams.length; i++) { - let param = params[paramsStartIndex + i]; - if (isNamed) { - paramsStream.addDependency(param); - } else { - attrs[positionalParams[i]] = param; - } +function processPositionalParams(renderNode, positionalParams, params, attrs) { + // if the component is rendered via {{component}} helper, the first + // element of `params` is the name of the component, so we need to + // skip that when the positional parameters are constructed + const paramsStartIndex = renderNode.state.isComponentHelper ? 1 : 0; + const isNamed = typeof positionalParams === 'string'; + let paramsStream; + + if (isNamed) { + paramsStream = new Stream(() => { + return readArray(params.slice(paramsStartIndex)); + }, 'params'); + + attrs[positionalParams] = paramsStream; + } + + for (let i = 0; i < positionalParams.length; i++) { + let param = params[paramsStartIndex + i]; + if (isNamed) { + paramsStream.addDependency(param); + } else { + attrs[positionalParams[i]] = param; } } } @@ -274,7 +297,7 @@ ComponentNodeManager.prototype.destroy = function() { component.destroy(); }; -export function createComponent(_component, isAngleBracket, _props, renderNode, env, attrs = {}) { +export function createComponent(_component, isAngleBracket, _props, renderNode, env, attrs = {}, proto = _component.proto()) { let props = assign({}, _props); if (!isAngleBracket) { @@ -284,7 +307,6 @@ export function createComponent(_component, isAngleBracket, _props, renderNode, let snapshot = takeSnapshot(attrs); props.attrs = snapshot; - let proto = _component.proto(); mergeBindings(props, shadowedAttrs(proto, snapshot)); } else { props._isAngleBracket = true; diff --git a/packages/ember-htmlbars/tests/integration/component_invocation_test.js b/packages/ember-htmlbars/tests/integration/component_invocation_test.js index 619bba89609..ae8e42f92a0 100644 --- a/packages/ember-htmlbars/tests/integration/component_invocation_test.js +++ b/packages/ember-htmlbars/tests/integration/component_invocation_test.js @@ -404,7 +404,7 @@ if (isEnabled('ember-views-component-block-info')) { }); } -QUnit.test('static named positional parameters', function() { +QUnit.test('static named positional parameters [DEPRECATED]', function() { registry.register('template:components/sample-component', compile('{{attrs.name}}{{attrs.age}}')); registry.register('component:sample-component', Component.extend({ positionalParams: ['name', 'age'] @@ -415,12 +415,14 @@ QUnit.test('static named positional parameters', function() { container: container }).create(); - runAppend(view); + expectDeprecation(function() { + runAppend(view); + }, 'Calling `var Thing = Ember.Component.extend({ positionalParams: [\'a\', \'b\' ]});` is deprecated in favor of `Thing.reopenClass({ positionalParams: [\'a\', \'b\'] });'); equal(jQuery('#qunit-fixture').text(), 'Quint4'); }); -QUnit.test('dynamic named positional parameters', function() { +QUnit.test('dynamic named positional parameters [DEPRECATED]', function() { registry.register('template:components/sample-component', compile('{{attrs.name}}{{attrs.age}}')); registry.register('component:sample-component', Component.extend({ positionalParams: ['name', 'age'] @@ -435,7 +437,10 @@ QUnit.test('dynamic named positional parameters', function() { } }).create(); - runAppend(view); + expectDeprecation(function() { + runAppend(view); + }, 'Calling `var Thing = Ember.Component.extend({ positionalParams: [\'a\', \'b\' ]});` is deprecated in favor of `Thing.reopenClass({ positionalParams: [\'a\', \'b\'] });'); + equal(jQuery('#qunit-fixture').text(), 'Quint4'); run(function() { set(view.context, 'myName', 'Edward'); @@ -445,7 +450,7 @@ QUnit.test('dynamic named positional parameters', function() { equal(jQuery('#qunit-fixture').text(), 'Edward5'); }); -QUnit.test('static arbitrary number of positional parameters', function() { +QUnit.test('static arbitrary number of positional parameters [DEPRECATED]', function() { registry.register('template:components/sample-component', compile('{{#each attrs.names as |name|}}{{name}}{{/each}}')); registry.register('component:sample-component', Component.extend({ positionalParams: 'names' @@ -456,14 +461,17 @@ QUnit.test('static arbitrary number of positional parameters', function() { container: container }).create(); - runAppend(view); + expectDeprecation(function() { + runAppend(view); + }, 'Calling `var Thing = Ember.Component.extend({ positionalParams: [\'a\', \'b\' ]});` is deprecated in favor of `Thing.reopenClass({ positionalParams: [\'a\', \'b\'] });'); + equal(view.$('#args-3').text(), 'Foo4Bar'); equal(view.$('#args-5').text(), 'Foo4Bar5Baz'); equal(view.$('#helper').text(), 'Foo4Bar5Baz'); }); -QUnit.test('dynamic arbitrary number of positional parameters', function() { +QUnit.test('dynamic arbitrary number of positional parameters [DEPRECATED]', function() { registry.register('template:components/sample-component', compile('{{#each attrs.names as |name|}}{{name}}{{/each}}')); registry.register('component:sample-component', Component.extend({ positionalParams: 'names' @@ -478,7 +486,108 @@ QUnit.test('dynamic arbitrary number of positional parameters', function() { } }).create(); + expectDeprecation(function() { + runAppend(view); + }, 'Calling `var Thing = Ember.Component.extend({ positionalParams: [\'a\', \'b\' ]});` is deprecated in favor of `Thing.reopenClass({ positionalParams: [\'a\', \'b\'] });'); + + equal(view.$('#direct').text(), 'Foo4'); + equal(view.$('#helper').text(), 'Foo4'); + run(function() { + set(view.context, 'user1', 'Bar'); + set(view.context, 'user2', '5'); + }); + + equal(view.$('#direct').text(), 'Bar5'); + equal(view.$('#helper').text(), 'Bar5'); +}); + +QUnit.test('static named positional parameters', function() { + var SampleComponent = Component.extend(); + SampleComponent.reopenClass({ + positionalParams: ['name', 'age'] + }); + registry.register('template:components/sample-component', compile('{{attrs.name}}{{attrs.age}}')); + registry.register('component:sample-component', SampleComponent); + + view = EmberView.extend({ + layout: compile('{{sample-component "Quint" 4}}'), + container: container + }).create(); + + runAppend(view); + + equal(jQuery('#qunit-fixture').text(), 'Quint4'); +}); + +QUnit.test('dynamic named positional parameters', function() { + var SampleComponent = Component.extend(); + SampleComponent.reopenClass({ + positionalParams: ['name', 'age'] + }); + + registry.register('template:components/sample-component', compile('{{attrs.name}}{{attrs.age}}')); + registry.register('component:sample-component', SampleComponent); + + view = EmberView.extend({ + layout: compile('{{sample-component myName myAge}}'), + container: container, + context: { + myName: 'Quint', + myAge: 4 + } + }).create(); + runAppend(view); + + equal(jQuery('#qunit-fixture').text(), 'Quint4'); + run(function() { + set(view.context, 'myName', 'Edward'); + set(view.context, 'myAge', '5'); + }); + + equal(jQuery('#qunit-fixture').text(), 'Edward5'); +}); + +QUnit.test('static arbitrary number of positional parameters', function() { + var SampleComponent = Component.extend(); + SampleComponent.reopenClass({ + positionalParams: 'names' + }); + + registry.register('template:components/sample-component', compile('{{#each attrs.names as |name|}}{{name}}{{/each}}')); + registry.register('component:sample-component', SampleComponent); + + view = EmberView.extend({ + layout: compile('{{sample-component "Foo" 4 "Bar" id="args-3"}}{{sample-component "Foo" 4 "Bar" 5 "Baz" id="args-5"}}{{component "sample-component" "Foo" 4 "Bar" 5 "Baz" id="helper"}}'), + container: container + }).create(); + + runAppend(view); + + equal(view.$('#args-3').text(), 'Foo4Bar'); + equal(view.$('#args-5').text(), 'Foo4Bar5Baz'); + equal(view.$('#helper').text(), 'Foo4Bar5Baz'); +}); + +QUnit.test('dynamic arbitrary number of positional parameters', function() { + var SampleComponent = Component.extend(); + SampleComponent.reopenClass({ + positionalParams: 'names' + }); + registry.register('template:components/sample-component', compile('{{#each attrs.names as |name|}}{{name}}{{/each}}')); + registry.register('component:sample-component', SampleComponent); + + view = EmberView.extend({ + layout: compile('{{sample-component user1 user2 id="direct"}}{{component "sample-component" user1 user2 id="helper"}}'), + container: container, + context: { + user1: 'Foo', + user2: 4 + } + }).create(); + + runAppend(view); + equal(view.$('#direct').text(), 'Foo4'); equal(view.$('#helper').text(), 'Foo4'); run(function() { @@ -533,7 +642,7 @@ QUnit.test('moduleName is available on _renderNode when no layout is present', f }); if (isEnabled('ember-htmlbars-component-helper')) { - QUnit.test('{{component}} helper works with positional params', function() { + QUnit.test('{{component}} helper works with positional params [DEPRECATED]', function() { registry.register('template:components/sample-component', compile('{{attrs.name}}{{attrs.age}}')); registry.register('component:sample-component', Component.extend({ positionalParams: ['name', 'age'] @@ -548,6 +657,37 @@ if (isEnabled('ember-htmlbars-component-helper')) { } }).create(); + expectDeprecation(function() { + runAppend(view); + }, 'Calling `var Thing = Ember.Component.extend({ positionalParams: [\'a\', \'b\' ]});` is deprecated in favor of `Thing.reopenClass({ positionalParams: [\'a\', \'b\'] });'); + + equal(jQuery('#qunit-fixture').text(), 'Quint4'); + run(function() { + set(view.context, 'myName', 'Edward'); + set(view.context, 'myAge', '5'); + }); + + equal(jQuery('#qunit-fixture').text(), 'Edward5'); + }); + + QUnit.test('{{component}} helper works with positional params', function() { + var SampleComponent = Component.extend(); + SampleComponent.reopenClass({ + positionalParams: ['name', 'age'] + }); + + registry.register('template:components/sample-component', compile('{{attrs.name}}{{attrs.age}}')); + registry.register('component:sample-component', SampleComponent); + + view = EmberView.extend({ + layout: compile('{{component "sample-component" myName myAge}}'), + container: container, + context: { + myName: 'Quint', + myAge: 4 + } + }).create(); + runAppend(view); equal(jQuery('#qunit-fixture').text(), 'Quint4'); run(function() { From 4373ac654c4713c2969caabdc0559578a448ca05 Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Thu, 30 Jul 2015 22:27:13 -0400 Subject: [PATCH 2/3] [BUGFIX release] Setup named positionalParam array. Previously, when `positionalParams` was a string value (which means to set all params as an array at that particular prop in `attrs`) we were iterating over the length of the destination property name instead of the available `params`. --- .../lib/node-managers/component-node-manager.js | 11 +++++++---- .../tests/integration/component_invocation_test.js | 11 +++++++++-- 2 files changed, 16 insertions(+), 6 deletions(-) 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 d625891474b..28332135b76 100644 --- a/packages/ember-htmlbars/lib/node-managers/component-node-manager.js +++ b/packages/ember-htmlbars/lib/node-managers/component-node-manager.js @@ -136,11 +136,14 @@ function processPositionalParams(renderNode, positionalParams, params, attrs) { attrs[positionalParams] = paramsStream; } - for (let i = 0; i < positionalParams.length; i++) { - let param = params[paramsStartIndex + i]; - if (isNamed) { + if (isNamed) { + for (let i = paramsStartIndex; i < params.length; i++) { + let param = params[i]; paramsStream.addDependency(param); - } else { + } + } else { + for (let i = 0; i < positionalParams.length; i++) { + let param = params[paramsStartIndex + i]; attrs[positionalParams[i]] = param; } } diff --git a/packages/ember-htmlbars/tests/integration/component_invocation_test.js b/packages/ember-htmlbars/tests/integration/component_invocation_test.js index ae8e42f92a0..b046dee19c4 100644 --- a/packages/ember-htmlbars/tests/integration/component_invocation_test.js +++ b/packages/ember-htmlbars/tests/integration/component_invocation_test.js @@ -572,9 +572,9 @@ QUnit.test('static arbitrary number of positional parameters', function() { QUnit.test('dynamic arbitrary number of positional parameters', function() { var SampleComponent = Component.extend(); SampleComponent.reopenClass({ - positionalParams: 'names' + positionalParams: 'n' }); - registry.register('template:components/sample-component', compile('{{#each attrs.names as |name|}}{{name}}{{/each}}')); + registry.register('template:components/sample-component', compile('{{#each attrs.n as |name|}}{{name}}{{/each}}')); registry.register('component:sample-component', SampleComponent); view = EmberView.extend({ @@ -597,6 +597,13 @@ QUnit.test('dynamic arbitrary number of positional parameters', function() { equal(view.$('#direct').text(), 'Bar5'); equal(view.$('#helper').text(), 'Bar5'); + + run(function() { + set(view.context, 'user2', '6'); + }); + + equal(view.$('#direct').text(), 'Bar6'); + equal(view.$('#helper').text(), 'Bar6'); }); QUnit.test('moduleName is available on _renderNode when a layout is present', function() { From b0f6540e8f4ffbe7b11e1a8f246f7e15a65b8270 Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Fri, 31 Jul 2015 00:49:58 -0400 Subject: [PATCH 3/3] [BUGFIX release] Avoid double-setting attrs during initial renders This significantly reduces unnecessary event firing during the initial render of each component. In my sample scenario, it cut average events per component from 14.9 to 7.07. This does not have a major performance impact by itself, but applications that are observing any of these events may experience improvement. --- .../lib/node-managers/component-node-manager.js | 13 +++++++------ packages/ember-metal-views/lib/renderer.js | 1 - 2 files changed, 7 insertions(+), 7 deletions(-) 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 28332135b76..e6af331b6f2 100644 --- a/packages/ember-htmlbars/lib/node-managers/component-node-manager.js +++ b/packages/ember-htmlbars/lib/node-managers/component-node-manager.js @@ -213,13 +213,11 @@ function configureCreateOptions(attrs, createOptions) { } ComponentNodeManager.prototype.render = function(_env, visitor) { - var { component, attrs } = this; + var { component } = this; return instrument(component, function() { let env = _env.childWithView(component); - var snapshot = takeSnapshot(attrs); - env.renderer.componentInitAttrs(this.component, snapshot); env.renderer.componentWillRender(component); env.renderedViews.push(component.elementId); @@ -302,15 +300,16 @@ ComponentNodeManager.prototype.destroy = function() { export function createComponent(_component, isAngleBracket, _props, renderNode, env, attrs = {}, proto = _component.proto()) { let props = assign({}, _props); + let attrsSnapshot; if (!isAngleBracket) { let hasSuppliedController = 'controller' in attrs; // 2.0TODO remove Ember.deprecate('controller= is deprecated', !hasSuppliedController, { id: 'ember-htmlbars.create-component', until: '3.0.0' }); - let snapshot = takeSnapshot(attrs); - props.attrs = snapshot; + attrsSnapshot = takeSnapshot(attrs); + props.attrs = attrsSnapshot; - mergeBindings(props, shadowedAttrs(proto, snapshot)); + mergeBindings(props, shadowedAttrs(proto, attrsSnapshot)); } else { props._isAngleBracket = true; } @@ -320,6 +319,8 @@ export function createComponent(_component, isAngleBracket, _props, renderNode, let component = _component.create(props); + env.renderer.componentInitAttrs(component, attrsSnapshot); + // for the fallback case component.container = component.container || env.container; diff --git a/packages/ember-metal-views/lib/renderer.js b/packages/ember-metal-views/lib/renderer.js index 405dfa134e2..3e656592fed 100755 --- a/packages/ember-metal-views/lib/renderer.js +++ b/packages/ember-metal-views/lib/renderer.js @@ -136,7 +136,6 @@ Renderer.prototype.setAttrs = function (view, attrs) { }; // set attrs the first time Renderer.prototype.componentInitAttrs = function (component, attrs) { - set(component, 'attrs', attrs); component.trigger('didInitAttrs', { attrs }); component.trigger('didReceiveAttrs', { newAttrs: attrs }); }; // set attrs the first time