Merge pull request #15855 from bekzod/filter-computed-expand

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

packages/ember-metal/lib/expand_properties.js

@@ -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.

packages/ember-runtime/lib/computed/reduce_computed_macros.js

@@ -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; }
@@ -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() {
@@ -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');
 }
 
 /**
@@ -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');
 }
 
 /**
@@ -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');
 }
 
 /**
@@ -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));
 }
@@ -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 {
@@ -403,7 +421,7 @@ export function uniq(...args) {
     });
 
     return uniq;
-  });
+  }, 'uniq');
 }
 
 /**
@@ -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);
@@ -542,8 +565,7 @@ export function intersect(...args) {
       }
 
       return true;
-    });
-
+    }, 'intersect');
 
     return emberA(results);
   });
@@ -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);
@@ -645,7 +671,7 @@ export function collect(...dependentKeys) {
       }
     }
     return res;
-  });
+  }, 'collect');
 }
 
 /**
@@ -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
   );

packages/ember-runtime/tests/computed/reduce_computed_macros_test.js

@@ -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({
@@ -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({
@@ -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');
 });