From 1a439ecc721817a6f6b0521fb4d2ff05c777a817 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivan=20Lu=C4=8Din?= Date: Fri, 10 Nov 2017 15:22:54 +0100 Subject: [PATCH 1/3] Add failing test for computed.filter @each with {} regression --- .../computed/reduce_computed_macros_test.js | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/packages/ember-runtime/tests/computed/reduce_computed_macros_test.js b/packages/ember-runtime/tests/computed/reduce_computed_macros_test.js index 96c1499199b..2fee357c838 100644 --- a/packages/ember-runtime/tests/computed/reduce_computed_macros_test.js +++ b/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: [item] + }); + + deepEqual(obj.get('filtered'), [item]); + + item.set('prop', false); + + deepEqual(obj.get('filtered'), []); +}); + QUnit.module('filterBy', { setup() { obj = EmberObject.extend({ From c02250953a21f2ce85f4f8c02a2dc4d9bafd0fe7 Mon Sep 17 00:00:00 2001 From: bekzod Date: Tue, 14 Nov 2017 13:40:17 +0500 Subject: [PATCH 2/3] [BUGFIX release] fix regression with computed `filter/map/sort` --- .../ember-runtime/lib/computed/reduce_computed_macros.js | 5 +++-- .../tests/computed/reduce_computed_macros_test.js | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/ember-runtime/lib/computed/reduce_computed_macros.js b/packages/ember-runtime/lib/computed/reduce_computed_macros.js index 03278282eb2..ab3503fd181 100644 --- a/packages/ember-runtime/lib/computed/reduce_computed_macros.js +++ b/packages/ember-runtime/lib/computed/reduce_computed_macros.js @@ -17,7 +17,6 @@ import compare from '../compare'; import { isArray } from '../utils'; import { A as emberA } from '../system/native_array'; - function reduceMacro(dependentKey, callback, initialValue) { let cp = new ComputedProperty(function() { let arr = get(this, dependentKey); @@ -45,7 +44,9 @@ 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; } diff --git a/packages/ember-runtime/tests/computed/reduce_computed_macros_test.js b/packages/ember-runtime/tests/computed/reduce_computed_macros_test.js index 2fee357c838..eed6d920222 100644 --- a/packages/ember-runtime/tests/computed/reduce_computed_macros_test.js +++ b/packages/ember-runtime/tests/computed/reduce_computed_macros_test.js @@ -352,7 +352,7 @@ QUnit.test('it updates properly on @each with {} dependencies', function() { return item.get('prop') === true; }) }).create({ - items: [item] + items: emberA([item]) }); deepEqual(obj.get('filtered'), [item]); From aabc86b5e7506cf53d4df728051c3719b652b5ed Mon Sep 17 00:00:00 2001 From: bekzod Date: Wed, 15 Nov 2017 01:08:47 +0500 Subject: [PATCH 3/3] [BUGFIX beta] assert for passing brace expanding deps to `sum/max..` --- packages/ember-metal/lib/expand_properties.js | 2 +- .../lib/computed/reduce_computed_macros.js | 51 ++++++++++++++----- .../computed/reduce_computed_macros_test.js | 4 +- 3 files changed, 41 insertions(+), 16 deletions(-) diff --git a/packages/ember-metal/lib/expand_properties.js b/packages/ember-metal/lib/expand_properties.js index 5f3ceeaefe9..d44fc62dd0e 100644 --- a/packages/ember-metal/lib/expand_properties.js +++ b/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. diff --git a/packages/ember-runtime/lib/computed/reduce_computed_macros.js b/packages/ember-runtime/lib/computed/reduce_computed_macros.js index ab3503fd181..7ba2d7ec92b 100644 --- a/packages/ember-runtime/lib/computed/reduce_computed_macros.js +++ b/packages/ember-runtime/lib/computed/reduce_computed_macros.js @@ -17,7 +17,12 @@ import compare from '../compare'; import { isArray } from '../utils'; import { A as emberA } from '../system/native_array'; -function reduceMacro(dependentKey, callback, initialValue) { +function reduceMacro(dependentKey, callback, initialValue, name) { + assert( + `Dependent key passed to \`Ember.computed.${name}\` shouldn't contain brace expanding pattern.`, + !/[\[\]\{\}]/g.test(dependentKey) + ); + let cp = new ComputedProperty(function() { let arr = get(this, dependentKey); if (arr === null || typeof arr !== 'object') { return initialValue; } @@ -51,7 +56,11 @@ function arrayMacro(dependentKey, callback) { 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() { @@ -74,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'); } /** @@ -120,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'); } /** @@ -166,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'); } /** @@ -243,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)); } @@ -346,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 { @@ -404,7 +421,7 @@ export function uniq(...args) { }); return uniq; - }); + }, 'uniq'); } /** @@ -438,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); @@ -543,8 +565,7 @@ export function intersect(...args) { } return true; - }); - + }, 'intersect'); return emberA(results); }); @@ -585,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); @@ -646,7 +671,7 @@ export function collect(...dependentKeys) { } } return res; - }); + }, 'collect'); } /** @@ -717,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 ); diff --git a/packages/ember-runtime/tests/computed/reduce_computed_macros_test.js b/packages/ember-runtime/tests/computed/reduce_computed_macros_test.js index eed6d920222..3e940c34899 100644 --- a/packages/ember-runtime/tests/computed/reduce_computed_macros_test.js +++ b/packages/ember-runtime/tests/computed/reduce_computed_macros_test.js @@ -665,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({ @@ -675,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'); });