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

[BUGFIX release] fix regression with computed filter/map/sort #15855

Merged
merged 3 commits into from
Nov 28, 2017
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
2 changes: 1 addition & 1 deletion packages/ember-metal/lib/expand_properties.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { assert } from 'ember-debug';
@module @ember/object
*/

var END_WITH_EACH_REGEX = /\.@each$/;
const END_WITH_EACH_REGEX = /\.@each$/;

/**
Expands `pattern`, invoking `callback` for each expansion.
Expand Down
54 changes: 40 additions & 14 deletions packages/ember-runtime/lib/computed/reduce_computed_macros.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,12 @@ import compare from '../compare';
import { isArray } from '../utils';
import { A as emberA } from '../system/native_array';

function reduceMacro(dependentKey, callback, initialValue, name) {
assert(
`Dependent key passed to \`Ember.computed.${name}\` shouldn't contain brace expanding pattern.`,
!/[\[\]\{\}]/g.test(dependentKey)
);

function reduceMacro(dependentKey, callback, initialValue) {
let cp = new ComputedProperty(function() {
let arr = get(this, dependentKey);
if (arr === null || typeof arr !== 'object') { return initialValue; }
Expand All @@ -45,12 +49,18 @@ function arrayMacro(dependentKey, callback) {
} else {
return emberA();
}
}, { dependentKeys: [ dependentKey ], readOnly: true });
}, { readOnly: true });

cp.property(dependentKey); // this forces to expand properties GH #15855

return cp;
}

function multiArrayMacro(_dependentKeys, callback) {
function multiArrayMacro(_dependentKeys, callback, name) {
assert(
`Dependent keys passed to \`Ember.computed.${name}\` shouldn't contain brace expanding pattern.`,
_dependentKeys.every((dependentKey)=> !/[\[\]\{\}]/g.test(dependentKey))
);
let dependentKeys = _dependentKeys.map(key => `${key}.[]`);

let cp = new ComputedProperty(function() {
Expand All @@ -73,7 +83,7 @@ function multiArrayMacro(_dependentKeys, callback) {
@public
*/
export function sum(dependentKey) {
return reduceMacro(dependentKey, (sum, item) => sum + item, 0);
return reduceMacro(dependentKey, (sum, item) => sum + item, 0, 'sum');
}

/**
Expand Down Expand Up @@ -119,7 +129,7 @@ export function sum(dependentKey) {
@public
*/
export function max(dependentKey) {
return reduceMacro(dependentKey, (max, item) => Math.max(max, item), -Infinity);
return reduceMacro(dependentKey, (max, item) => Math.max(max, item), -Infinity, 'max');
}

/**
Expand Down Expand Up @@ -165,7 +175,7 @@ export function max(dependentKey) {
@public
*/
export function min(dependentKey) {
return reduceMacro(dependentKey, (min, item) => Math.min(min, item), Infinity);
return reduceMacro(dependentKey, (min, item) => Math.min(min, item), Infinity, 'min');
}

/**
Expand Down Expand Up @@ -242,10 +252,14 @@ export function map(dependentKey, callback) {
*/
export function mapBy(dependentKey, propertyKey) {
assert(
'Ember.computed.mapBy expects a property string for its second argument, ' +
'\`Ember.computed.mapBy\` expects a property string for its second argument, ' +
'perhaps you meant to use "map"',
typeof propertyKey === 'string'
);
assert(
`Dependent key passed to \`Ember.computed.mapBy\` shouldn't contain brace expanding pattern.`,
!/[\[\]\{\}]/g.test(dependentKey)
);

return map(`${dependentKey}.@each.${propertyKey}`, item => get(item, propertyKey));
}
Expand Down Expand Up @@ -345,8 +359,12 @@ export function filter(dependentKey, callback) {
@public
*/
export function filterBy(dependentKey, propertyKey, value) {
let callback;
assert(
`Dependent key passed to \`Ember.computed.filterBy\` shouldn't contain brace expanding pattern.`,
!/[\[\]\{\}]/g.test(dependentKey)
);

let callback;
if (arguments.length === 2) {
callback = (item) => get(item, propertyKey);
} else {
Expand Down Expand Up @@ -403,7 +421,7 @@ export function uniq(...args) {
});

return uniq;
});
}, 'uniq');
}

/**
Expand Down Expand Up @@ -437,6 +455,11 @@ export function uniq(...args) {
@public
*/
export function uniqBy(dependentKey, propertyKey) {
assert(
`Dependent key passed to \`Ember.computed.uniqBy\` shouldn't contain brace expanding pattern.`,
!/[\[\]\{\}]/g.test(dependentKey)
);

let cp = new ComputedProperty(function() {
let uniq = emberA();
let seen = Object.create(null);
Expand Down Expand Up @@ -542,8 +565,7 @@ export function intersect(...args) {
}

return true;
});

}, 'intersect');

return emberA(results);
});
Expand Down Expand Up @@ -584,9 +606,13 @@ export function intersect(...args) {
@public
*/
export function setDiff(setAProperty, setBProperty) {
assert('Ember.computed.setDiff requires exactly two dependent arrays.',
assert('\`Ember.computed.setDiff\` requires exactly two dependent arrays.',
arguments.length === 2
);
assert(
`Dependent keys passed to \`Ember.computed.setDiff\` shouldn't contain brace expanding pattern.`,
!/[\[\]\{\}]/g.test(setAProperty) && !/[\[\]\{\}]/g.test(setBProperty)
);

let cp = new ComputedProperty(function() {
let setA = this.get(setAProperty);
Expand Down Expand Up @@ -645,7 +671,7 @@ export function collect(...dependentKeys) {
}
}
return res;
});
}, 'collect');
}

/**
Expand Down Expand Up @@ -716,7 +742,7 @@ export function collect(...dependentKeys) {
*/
export function sort(itemsKey, sortDefinition) {
assert(
'Ember.computed.sort requires two arguments: an array key to sort and ' +
'\`Ember.computed.sort\` requires two arguments: an array key to sort and ' +
'either a sort properties key or sort function',
arguments.length === 2
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,24 @@ QUnit.test('it updates as the array is replaced', function() {
deepEqual(obj.get('filtered'), [20, 22, 24], 'computed array is updated when array is changed');
});

QUnit.test('it updates properly on @each with {} dependencies', function() {
let item = EmberObject.create({prop: true});

obj = EmberObject.extend({
filtered: filter('items.@each.{prop}', function(item, index) {
return item.get('prop') === true;
})
}).create({
items: emberA([item])
});

deepEqual(obj.get('filtered'), [item]);

item.set('prop', false);

deepEqual(obj.get('filtered'), []);
});

QUnit.module('filterBy', {
setup() {
obj = EmberObject.extend({
Expand Down Expand Up @@ -647,7 +665,7 @@ QUnit.test('it asserts if given fewer or more than two dependent properties', fu
array: emberA([1, 2, 3, 4, 5, 6, 7]),
array2: emberA([3, 4, 5])
});
}, /Ember\.computed\.setDiff requires exactly two dependent arrays/, 'setDiff requires two dependent arrays');
}, /\`Ember\.computed\.setDiff\` requires exactly two dependent arrays/, 'setDiff requires two dependent arrays');

expectAssertion(function () {
EmberObject.extend({
Expand All @@ -657,7 +675,7 @@ QUnit.test('it asserts if given fewer or more than two dependent properties', fu
array2: emberA([3, 4, 5]),
array3: emberA([7])
});
}, /Ember\.computed\.setDiff requires exactly two dependent arrays/, 'setDiff requires two dependent arrays');
}, /\`Ember\.computed\.setDiff\` requires exactly two dependent arrays/, 'setDiff requires two dependent arrays');
});


Expand Down