Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not slice classNames and classNameBindings speculatively. #14389

Merged
merged 3 commits into from
Oct 27, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ moduleFor('ClassNameBindings integration', class extends RenderingTest {
init() {
this._super();

let bindings = this.classNameBindings;
let bindings = this.classNameBindings = this.classNameBindings.slice();

if (this.get('bindIsEnabled')) {
bindings.push('isEnabled:enabled');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,7 @@ moduleFor('Components test: curly components', class extends RenderingTest {
let FooBarComponent = Component.extend({
init() {
this._super();
this.classNames = this.classNames.slice();
this.classNames.push('foo', 'bar', `outside-${this.get('extraClass')}`);
}
});
Expand Down
20 changes: 16 additions & 4 deletions packages/ember-metal/lib/mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -143,20 +143,32 @@ function giveMethodSuper(obj, key, method, values, descs) {

function applyConcatenatedProperties(obj, key, value, values) {
let baseValue = values[key] || obj[key];
let ret;

if (baseValue) {
if ('function' === typeof baseValue.concat) {
if (value === null || value === undefined) {
return baseValue;
ret = baseValue;
} else {
return baseValue.concat(value);
ret = baseValue.concat(value);
}
} else {
return makeArray(baseValue).concat(value);
ret = makeArray(baseValue).concat(value);
}
} else {
return makeArray(value);
ret = makeArray(value);
}

runInDebug(() => {
// it is possible to use concatenatedProperties with strings (which cannot be frozen)
// only freeze objects...
if (typeof ret === 'object' && ret !== null) {
// prevent mutating `concatenatedProperties` array after it is applied
Object.freeze(ret);
}
});

return ret;
}

function applyMergedProperties(obj, key, value, values) {
Expand Down
6 changes: 1 addition & 5 deletions packages/ember-routing/lib/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -130,11 +130,7 @@ export function calculateCacheKey(prefix, _parts, values) {
'Array of fully defined objects' style.
*/
export function normalizeControllerQueryParams(queryParams) {
if (queryParams._qpMap) {
return queryParams._qpMap;
}

let qpMap = queryParams._qpMap = {};
let qpMap = {};

for (let i = 0; i < queryParams.length; ++i) {
accumulateQueryParamDescriptors(queryParams[i], qpMap);
Expand Down
9 changes: 0 additions & 9 deletions packages/ember-routing/tests/utils_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,6 @@ import {

QUnit.module('Routing query parameter utils - normalizeControllerQueryParams');

QUnit.test('returns the cached value if that has been previously set', function(assert) {
let cached = {};
let params = ['foo'];
params._qpMap = cached;

let normalized = normalizeControllerQueryParams(params);
equal(cached, normalized, 'cached value returned if previously set');
});

QUnit.test('converts array style into verbose object style', function(assert) {
let paramName = 'foo';
let params = [paramName];
Expand Down
3 changes: 0 additions & 3 deletions packages/ember-views/lib/mixins/class_names_support.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,7 @@ export default Mixin.create({
this._super(...arguments);

assert(`Only arrays are allowed for 'classNameBindings'`, Array.isArray(this.classNameBindings));
this.classNameBindings = this.classNameBindings.slice();

assert(`Only arrays of static class strings are allowed for 'classNames'. For dynamic classes, use 'classNameBindings'.`, Array.isArray(this.classNames));
this.classNames = this.classNames.slice();
},

/**
Expand Down