Skip to content

Commit

Permalink
Merge pull request #11935 from ef4/attrs-perf
Browse files Browse the repository at this point in the history 
[BUGFIX release] Avoid unnecessary change events during initial render
  • Loading branch information
@stefanpenner
stefanpenner committed Jul 31, 2015
2 parents 02284d1 + b0f6540 commit facb04b
Show file tree
Hide file tree
Showing 3 changed files with 212 additions and 40 deletions.
88 changes: 57 additions & 31 deletions packages/ember-htmlbars/lib/node-managers/component-node-manager.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 }
Expand All @@ -95,30 +96,55 @@ 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;

if (isNamed) {
paramsStream = new Stream(() => {
return readArray(params.slice(paramsStartIndex));
}, 'params');

attrs[positionalParams] = 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 (positionalParams) {
processPositionalParams(renderNode, positionalParams, params, attrs);
}

// 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;
}

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;
}

if (isNamed) {
for (let i = paramsStartIndex; i < params.length; i++) {
let param = params[i];
paramsStream.addDependency(param);
}
} else {
for (let i = 0; i < positionalParams.length; i++) {
let param = params[paramsStartIndex + i];
if (isNamed) {
paramsStream.addDependency(param);
} else {
attrs[positionalParams[i]] = param;
}
attrs[positionalParams[i]] = param;
}
}
}
Expand Down Expand Up @@ -187,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);

Expand Down Expand Up @@ -274,18 +298,18 @@ 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);
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;

let proto = _component.proto();
mergeBindings(props, shadowedAttrs(proto, snapshot));
mergeBindings(props, shadowedAttrs(proto, attrsSnapshot));
} else {
props._isAngleBracket = true;
}
Expand All @@ -295,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;

Expand Down
163 changes: 155 additions & 8 deletions packages/ember-htmlbars/tests/integration/component_invocation_test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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']
Expand All @@ -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']
Expand All @@ -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');
Expand All @@ -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'
Expand All @@ -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'
Expand All @@ -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: 'n'
});
registry.register('template:components/sample-component', compile('{{#each attrs.n 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() {
Expand All @@ -488,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() {
Expand Down Expand Up @@ -533,7 +649,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']
Expand All @@ -548,6 +664,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() {
Expand Down
1 change: 0 additions & 1 deletion packages/ember-metal-views/lib/renderer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit facb04b

Please sign in to comment.