Added a firstObject/lastObject cache checks.

Basically the contents of https://gist.github.com/mmun/877bad33158578a8609b
emberjs · Feb 17, 2016 · 1dc8e16 · 1dc8e16
1 parent 033fe1b
commit 1dc8e16
Show file tree

Hide file tree

Showing 2 changed files with 52 additions and 10 deletions.
diff --git a/packages/ember-runtime/lib/mixins/array.js b/packages/ember-runtime/lib/mixins/array.js
@@ -9,6 +9,7 @@
 import Ember from 'ember-metal/core'; // ES6TODO: Ember.A
 
 import { get } from 'ember-metal/property_get';
+import { peekMeta } from 'ember-metal/utils';
 import {
   computed,
   cacheFor
@@ -499,19 +500,43 @@ export default Mixin.create(Enumerable, {
     }
 
     sendEvent(this, '@array:change', [this, startIdx, removeAmt, addAmt]);
+    this.arrayFirstLastObjectChange();
 
-    var length = get(this, 'length');
-    var cachedFirst = cacheFor(this, 'firstObject');
-    var cachedLast = cacheFor(this, 'lastObject');
+    return this;
+  },
 
-    if (objectAt(this, 0) !== cachedFirst) {
-      propertyWillChange(this, 'firstObject');
-      propertyDidChange(this, 'firstObject');
-    }
+  /**
+    Default implementation of proper notification of the firstObject and
+    lastObject properties. It requires the #objectAt() to be called upon
+    array contents manipulation.
 
-    if (objectAt(this, length - 1) !== cachedLast) {
-      propertyWillChange(this, 'lastObject');
-      propertyDidChange(this, 'lastObject');
+    There might however be cases in which the notifications are not strictly
+    necessary or could trigger even when no real change has been made.
+    In such cases this method could be overriden with an alternate way.
+
+    As an example a dynamically loaded sparse arrays could be considered
+    where touching the #contentAt(index) would trigger a store layer
+    roundtrip.
+
+    @method arrayFirstLastObjectChange
+    @public
+  */
+  arrayFirstLastObjectChange: function() {
+    var length = get(this, 'length');
+    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;