[BUGFIX beta] fix for lastObject/firstObject update issue

emberjs · Jul 18, 2017 · 5752d95 · 5752d95
1 parent cb97ca5
commit 5752d95
Show file tree

Hide file tree

Showing 5 changed files with 77 additions and 14 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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');
 });