From 5752d95af1556a18774602091b108f43a1c4b322 Mon Sep 17 00:00:00 2001 From: bekzod Date: Fri, 14 Jul 2017 20:15:26 +0500 Subject: [PATCH] [BUGFIX beta] fix for lastObject/firstObject update issue --- packages/ember-runtime/lib/mixins/array.js | 34 ++++++++++++------- .../tests/suites/mutable_array/insertAt.js | 10 ++++++ .../tests/suites/mutable_array/pushObject.js | 22 ++++++++++++ .../tests/suites/mutable_array/replace.js | 21 ++++++++++++ .../suites/mutable_array/unshiftObject.js | 4 +-- 5 files changed, 77 insertions(+), 14 deletions(-) diff --git a/packages/ember-runtime/lib/mixins/array.js b/packages/ember-runtime/lib/mixins/array.js index 1fe9f3ae821..9d931333bd0 100644 --- a/packages/ember-runtime/lib/mixins/array.js +++ b/packages/ember-runtime/lib/mixins/array.js @@ -141,19 +141,29 @@ export function arrayContentDidChange(array, startIdx, removeAmt, addAmt) { let meta = peekMeta(array); let cache = meta && meta.readableCache(); - - if (cache) { - if (cache.firstObject !== undefined && - objectAt(array, 0) !== cacheFor.get(cache, 'firstObject')) { - propertyWillChange(array, 'firstObject', meta); - propertyDidChange(array, 'firstObject', meta); - } - if (cache.lastObject !== undefined && - objectAt(array, get(array, 'length') - 1) !== cacheFor.get(cache, 'lastObject')) { - propertyWillChange(array, 'lastObject', meta); - propertyDidChange(array, 'lastObject', meta); - } + if (cache !== undefined) { + let length = get(array, 'length'); + let addedAmount = (addAmt === -1 ? 0 : addAmt); + let removedAmount = (removeAmt === -1 ? 0 : removeAmt); + let delta = addedAmount - removedAmount; + let previousLength = length - delta; + + let normalStartIdx = startIdx < 0 ? previousLength + startIdx : startIdx; + if (cache.firstObject !== undefined && normalStartIdx === 0) { + propertyWillChange(array, 'firstObject'); + propertyDidChange(array, 'firstObject'); + } + + if (cache.lastObject !== undefined) { + let previousLastIndex = previousLength - 1; + let lastAffectedIndex = normalStartIdx + removedAmount; + if (previousLastIndex < lastAffectedIndex) { + propertyWillChange(array, 'lastObject'); + propertyDidChange(array, 'lastObject'); + } + } } + return array; } diff --git a/packages/ember-runtime/tests/suites/mutable_array/insertAt.js b/packages/ember-runtime/tests/suites/mutable_array/insertAt.js index be4e4de840b..eb230c160b6 100644 --- a/packages/ember-runtime/tests/suites/mutable_array/insertAt.js +++ b/packages/ember-runtime/tests/suites/mutable_array/insertAt.js @@ -135,10 +135,20 @@ suite.test('[A,B,C].insertAt(1,X) => [A,X,B,C] + notify', function() { let after = [before[0], item, before[1], before[2]]; let obj = this.newObject(before); let observer = this.newObserver(obj, '[]', '@each', 'length', 'firstObject', 'lastObject'); + let objectAtCalls = []; + + let objectAt = obj.objectAt; + obj.objectAt = (ix) => { + objectAtCalls.push(ix); + return objectAt.call(obj, ix); + } obj.getProperties('firstObject', 'lastObject'); /* Prime the cache */ + objectAtCalls.splice(0, objectAtCalls.length); + obj.insertAt(1, item); + deepEqual(objectAtCalls, [], 'objectAt is not called when only inserting items'); deepEqual(this.toArray(obj), after, 'post item results'); equal(get(obj, 'length'), after.length, 'length'); diff --git a/packages/ember-runtime/tests/suites/mutable_array/pushObject.js b/packages/ember-runtime/tests/suites/mutable_array/pushObject.js index fce14652d35..45ab22bc066 100644 --- a/packages/ember-runtime/tests/suites/mutable_array/pushObject.js +++ b/packages/ember-runtime/tests/suites/mutable_array/pushObject.js @@ -54,4 +54,26 @@ suite.test('[A,B,C].pushObject(X) => [A,B,C,X] + notify', function() { equal(observer.validate('firstObject'), false, 'should NOT have notified firstObject'); }); +suite.test('[A,B,C,C].pushObject(A) => [A,B,C,C] + notify', function() { + let before = this.newFixture(3); + let item = before[2]; // note same object as current tail. should end up twice + let after = [before[0], before[1], before[2], item]; + let obj = this.newObject(before); + let observer = this.newObserver(obj, '[]', '@each', 'length', 'firstObject', 'lastObject'); + + obj.getProperties('firstObject', 'lastObject'); /* Prime the cache */ + + obj.pushObject(item); + + deepEqual(this.toArray(obj), after, 'post item results'); + equal(get(obj, 'length'), after.length, 'length'); + + equal(observer.timesCalled('[]'), 1, 'should have notified [] once'); + equal(observer.timesCalled('@each'), 0, 'should not have notified @each once'); + equal(observer.timesCalled('length'), 1, 'should have notified length once'); + + equal(observer.validate('firstObject'), false, 'should NOT have notified firstObject'); + equal(observer.validate('lastObject'), true, 'should have notified lastObject'); +}); + export default suite; diff --git a/packages/ember-runtime/tests/suites/mutable_array/replace.js b/packages/ember-runtime/tests/suites/mutable_array/replace.js index b9f17b08484..a1ac5b636ec 100644 --- a/packages/ember-runtime/tests/suites/mutable_array/replace.js +++ b/packages/ember-runtime/tests/suites/mutable_array/replace.js @@ -126,6 +126,27 @@ suite.test('[A,B,C,D].replace(2,2) => [A,B] + notify', function() { equal(observer.validate('firstObject'), false, 'should NOT have notified firstObject once'); }); +suite.test('[A,B,C,D].replace(-1,1) => [A,B,C] + notify', function() { + let before = this.newFixture(4); + let after = [before[0], before[1], before[2]]; + + let obj = this.newObject(before); + let observer = this.newObserver(obj, '[]', '@each', 'length', 'firstObject', 'lastObject'); + + obj.getProperties('firstObject', 'lastObject'); /* Prime the cache */ + + obj.replace(-1, 1); + + deepEqual(this.toArray(obj), after, 'post item results'); + + equal(observer.timesCalled('[]'), 1, 'should have notified [] once'); + equal(observer.timesCalled('@each'), 0, 'should not have notified @each once'); + equal(observer.timesCalled('length'), 1, 'should have notified length once'); + equal(observer.timesCalled('lastObject'), 1, 'should have notified lastObject once'); + + equal(observer.validate('firstObject'), false, 'should NOT have notified firstObject once'); +}); + suite.test('Adding object should notify enumerable observer', function() { let fixtures = this.newFixture(4); let obj = this.newObject(fixtures); diff --git a/packages/ember-runtime/tests/suites/mutable_array/unshiftObject.js b/packages/ember-runtime/tests/suites/mutable_array/unshiftObject.js index e563dd7ac3c..1db55b51feb 100644 --- a/packages/ember-runtime/tests/suites/mutable_array/unshiftObject.js +++ b/packages/ember-runtime/tests/suites/mutable_array/unshiftObject.js @@ -58,7 +58,7 @@ suite.test('[A,B,C].unshiftObject(X) => [X,A,B,C] + notify', function() { suite.test('[A,B,C].unshiftObject(A) => [A,A,B,C] + notify', function() { let before = this.newFixture(3); let item = before[0]; // note same object as current head. should end up twice - let after = [item, before[0], before[1], before[2]]; + let after = [item, before[0], before[1], before[2]]; let obj = this.newObject(before); let observer = this.newObserver(obj, '[]', '@each', 'length', 'firstObject', 'lastObject'); @@ -73,7 +73,7 @@ suite.test('[A,B,C].unshiftObject(A) => [A,A,B,C] + notify', function() { equal(observer.timesCalled('@each'), 0, 'should not have notified @each once'); equal(observer.timesCalled('length'), 1, 'should have notified length once'); - equal(observer.validate('firstObject'), false, 'should NOT have notified firstObject'); + equal(observer.validate('firstObject'), true, 'should have notified firstObject'); equal(observer.validate('lastObject'), false, 'should NOT have notified lastObject'); });