Skip to content

Commit

Permalink
Merge pull request #16246 from emberjs/fix-empty-sort-properties
Browse files Browse the repository at this point in the history
[BUGFIX beta] computed.sort should not sort if sortProperties is empty
  • Loading branch information
rwjblue authored Feb 15, 2018
2 parents 7d0cfa2 + e99dd89 commit 5d6c252
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -837,7 +837,11 @@ function propertySort(itemsKey, sortPropertiesKey) {
let items = itemsKeyIsAtThis ? this : get(this, itemsKey);
if (!isArray(items)) { return emberA(); }

return sortByNormalizedSortProperties(items, normalizedSortProperties);
if (normalizedSortProperties.length === 0) {
return emberA(items.slice());
} else {
return sortByNormalizedSortProperties(items, normalizedSortProperties);
}
}, { dependentKeys: [`${sortPropertiesKey}.[]`], readOnly: true });

cp._activeObserverMap = undefined;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1116,6 +1116,20 @@ QUnit.test('updating an item\'s sort properties does not error when binary searc
], 'array is sorted correctly');
});

QUnit.test('array should not be sorted if sort properties array is empty', function(assert) {
var o = EmberObject.extend({
sortedItems: sort('items', 'itemSorting')
}).create({
itemSorting: emberA([]),
// This bug only manifests when array.sort(() => 0) is not equal to array.
// In order for this to happen, the browser must use an unstable sort and the
// array must be sufficient large. On Chrome, 12 items is currently sufficient.
items: emberA([6, 7, 8, 9, 10, 11, 0, 1, 2, 3, 4, 5])
});

assert.deepEqual(o.get('sortedItems'), [6, 7, 8, 9, 10, 11, 0, 1, 2, 3, 4, 5], 'array is not changed');
});

QUnit.test('array observers do not leak', function(assert) {
let daria = { name: 'Daria' };
let jane = { name: 'Jane' };
Expand Down

0 comments on commit 5d6c252

Please sign in to comment.