fix for lastObject/firstObject update issue

emberjs · Jul 17, 2017 · 45ebc8f · 45ebc8f
1 parent cb97ca5
commit 45ebc8f
Show file tree

Hide file tree

Showing 3 changed files with 52 additions and 13 deletions.
diff --git a/packages/ember-runtime/lib/mixins/array.js b/packages/ember-runtime/lib/mixins/array.js
@@ -141,19 +141,25 @@ 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) {
+    if (cache.firstObject !== undefined && startIdx === 0) {
+      propertyWillChange(array, 'firstObject');
+      propertyDidChange(array, 'firstObject');
     }
+
+    if (cache.lastObject !== undefined) {
+      let delta = (addAmt === -1 ? 0 : addAmt) - (removeAmt === -1 ? 0 : removeAmt);
+      let length = get(array, 'length');
+
+      let lastIndex = length - delta - 1;
+      let lastAffectedIndex = startIdx + (removeAmt === -1 ? 0 : removeAmt);
+      if (lastIndex < 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/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,8 +73,31 @@ 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');
 });
 
+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;