[BUGFIX] Fix arrangedContent observer lifecycle

emberjs · Feb 2, 2016 · 7552f6f · 7552f6f
1 parent 2cc7dbc
commit 7552f6f
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 30 deletions.
diff --git a/packages/ember-runtime/lib/system/array_proxy.js b/packages/ember-runtime/lib/system/array_proxy.js
@@ -23,6 +23,11 @@ import alias from 'ember-metal/alias';
 var OUT_OF_RANGE_EXCEPTION = 'Index out of range';
 var EMPTY = [];
 
+const ARRANGED_CONTENT_ARRAY_OBSERVER_EVENTS = {
+  willChange: 'arrangedContentArrayWillChange',
+  didChange: 'arrangedContentArrayDidChange'
+};
+
 function K() { return this; }
 
 /**
@@ -192,7 +197,10 @@ var ArrayProxy = EmberObject.extend(MutableArray, {
     this._prevContent = content;
 
     if (content) {
-      assert(`ArrayProxy expects an Array or Ember.ArrayProxy, but you passed ${typeof content}`, isArray(content) || content.isDestroyed);
+      assert(
+        `ArrayProxy expects an Array or Ember.ArrayProxy, but you passed ${typeof content}`,
+        isArray(content) || content.isDestroyed
+      );
 
       content.addArrayObserver(this, {
         willChange: 'contentArrayWillChange',
@@ -202,41 +210,34 @@ var ArrayProxy = EmberObject.extend(MutableArray, {
   },
 
   _arrangedContentDidChange: observer('arrangedContent', function() {
-    this._teardownArrangedContent(this._prevArrangedContent);
-    var arrangedContent = get(this, 'arrangedContent');
-    var len = arrangedContent ? get(arrangedContent, 'length') : 0;
-
-    assert('Can\'t set ArrayProxy\'s content to itself', arrangedContent !== this);
+    let arrangedContent = get(this, 'arrangedContent');
 
-    this._setupArrangedContent();
+    this._updateArrangedContent(arrangedContent);
 
+    var len = arrangedContent ? get(arrangedContent, 'length') : 0;
     this.arrangedContentDidChange(this);
     this.arrangedContentArrayDidChange(this, 0, undefined, len);
   }),
 
-  _setupArrangedContent() {
-    var arrangedContent = get(this, 'arrangedContent');
-    this._prevArrangedContent = arrangedContent;
+  _updateArrangedContent(newArrangedContent) {
+    let observedArrangedContent = this._observedArrangedContent;
+    if (observedArrangedContent !== newArrangedContent) {
+      if (observedArrangedContent) {
+        observedArrangedContent.removeArrayObserver(this, ARRANGED_CONTENT_ARRAY_OBSERVER_EVENTS);
+      }
 
-    if (arrangedContent) {
-      assert(`ArrayProxy expects an Array or Ember.ArrayProxy, but you passed ${typeof arrangedContent}`,
-        isArray(arrangedContent) || arrangedContent.isDestroyed);
+      if (newArrangedContent) {
+        assert(`Can't set ArrayProxy's content to itself`, newArrangedContent !== this);
 
-      arrangedContent.addArrayObserver(this, {
-        willChange: 'arrangedContentArrayWillChange',
-        didChange: 'arrangedContentArrayDidChange'
-      });
-    }
-  },
+        assert(
+          `ArrayProxy expects an Array or Ember.ArrayProxy, but you passed ${typeof newArrangedContent}`,
+          isArray(newArrangedContent) || newArrangedContent.isDestroyed
+        );
 
-  _teardownArrangedContent() {
-    var arrangedContent = get(this, 'arrangedContent');
+        newArrangedContent.addArrayObserver(this, ARRANGED_CONTENT_ARRAY_OBSERVER_EVENTS);
+      }
 
-    if (arrangedContent) {
-      arrangedContent.removeArrayObserver(this, {
-        willChange: 'arrangedContentArrayWillChange',
-        didChange: 'arrangedContentArrayDidChange'
-      });
+      this._observedArrangedContent = newArrangedContent;
     }
   },
 
@@ -369,14 +370,19 @@ var ArrayProxy = EmberObject.extend(MutableArray, {
   },
 
   init() {
+    this.__each = undefined;
     this._didInitArrayProxy = true;
+    this._content = undefined;
+    this._prevContent = undefined;
+    this._observedArrangedContent = undefined;
     this._super(...arguments);
     this._setupContent();
-    this._setupArrangedContent();
+
+    this._updateArrangedContent(get(this, 'arrangedContent'));
   },
 
   willDestroy() {
-    this._teardownArrangedContent();
+    this._updateArrangedContent(undefined);
     this._teardownContent(this.get('content'));
   }
 });

diff --git a/packages/ember-runtime/tests/system/array_proxy/content_update_test.js b/packages/ember-runtime/tests/system/array_proxy/content_update_test.js
@@ -2,7 +2,7 @@ import { computed } from 'ember-metal/computed';
 import ArrayProxy from 'ember-runtime/system/array_proxy';
 import { A as emberA } from 'ember-runtime/system/native_array';
 
-QUnit.module('Ember.ArrayProxy - content update');
+QUnit.module('ArrayProxy - content update');
 
 QUnit.test('The `contentArrayDidChange` method is invoked after `content` is updated.', function() {
   var proxy;

diff --git a/packages/ember-runtime/tests/system/array_proxy/length_test.js b/packages/ember-runtime/tests/system/array_proxy/length_test.js
@@ -4,7 +4,7 @@ import { observer } from 'ember-metal/mixin';
 import { computed } from 'ember-metal/computed';
 import { A as a } from 'ember-runtime/system/native_array';
 
-QUnit.module('Ember.ArrayProxy - content change (length)');
+QUnit.module('ArrayProxy - content change (length)');
 
 QUnit.test('array proxy + aliasedProperty complex test', function() {
   var aCalled, bCalled, cCalled, dCalled, eCalled;