Get firstObject/lastObject notifications only when cached

Basically the contents of https://gist.github.com/mmun/877bad33158578a8609b
emberjs · Apr 11, 2016 · 7bbbd6a · 7bbbd6a
1 parent 57cf48c
commit 7bbbd6a
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 11 deletions.
diff --git a/packages/ember-runtime/lib/mixins/array.js b/packages/ember-runtime/lib/mixins/array.js
@@ -10,6 +10,7 @@ import Ember from 'ember-metal/core'; // ES6TODO: Ember.A
 
 import symbol from 'ember-metal/symbol';
 import { get } from 'ember-metal/property_get';
+import { peekMeta } from 'ember-metal/utils';
 import {
   computed,
   cacheFor
@@ -517,18 +518,22 @@ export default Mixin.create(Enumerable, {
 
     sendEvent(this, '@array:change', [this, startIdx, removeAmt, addAmt]);
 
+    // firstObject/lastObject change notifications
+    // The call to objectAt() should only occur if there is a cached value
     var length = get(this, 'length');
-    var cachedFirst = cacheFor(this, 'firstObject');
-    var cachedLast = cacheFor(this, 'lastObject');
-
-    if (objectAt(this, 0) !== cachedFirst) {
-      propertyWillChange(this, 'firstObject');
-      propertyDidChange(this, 'firstObject');
-    }
-
-    if (objectAt(this, length - 1) !== cachedLast) {
-      propertyWillChange(this, 'lastObject');
-      propertyDidChange(this, 'lastObject');
+    var meta = peekMeta(this);
+    var cache  = meta && meta.readableCache();
+    if (cache) {
+      if (cache['firstObject'] !== undefined &&
+        this.objectAt(0) !== cacheFor(this, 'firstObject')) {
+        propertyWillChange(this, 'firstObject');
+        propertyDidChange(this, 'firstObject');
+      }
+      if (cache['lastObject'] !== undefined &&
+        this.objectAt(length - 1) !== cacheFor(this, 'lastObject')) {
+        propertyWillChange(this, 'lastObject');
+        propertyDidChange(this, 'lastObject');
+      }
     }
 
     return this;

diff --git a/packages/ember-runtime/tests/suites/mutable_array/replace.js b/packages/ember-runtime/tests/suites/mutable_array/replace.js
@@ -22,6 +22,23 @@ suite.test('[].replace(0,0,\'X\') => [\'X\'] + notify', function() {
   equal(observer.timesCalled('lastObject'), 1, 'should have notified lastObject once');
 });
 
+suite.test('[].replace(0,0,"X") => ["X"] + avoid calling objectAt and notifying fistObject/lastObject when not in cache', function() {
+  var obj, exp, observer;
+  var called = 0;
+  exp = this.newFixture(1);
+  obj = this.newObject([]);
+  obj.objectAt = function() {
+    called++;
+  };
+  observer = this.newObserver(obj, 'firstObject', 'lastObject');
+
+  obj.replace(0, 0, exp);
+
+  equal(called, 0, 'should NOT have called objectAt upon replace when firstObject/lastObject are not cached');
+  equal(observer.validate('firstObject'), false, 'should NOT have notified firstObject since not cached');
+  equal(observer.validate('lastObject'), false, 'should NOT have notified lastObject since not cached');
+});
+
 suite.test('[A,B,C,D].replace(1,2,X) => [A,X,D] + notify', function() {
   var obj, observer, before, replace, after;