Merge pull request #18835 from emberjs/bugfix-lts/fix-array-proxy-content-update

[BUGFIX] Make ArrayProxy Lazy

Author: rwjblue

packages/@ember/-internals/glimmer/tests/integration/components/tracked-test.js

@@ -69,6 +69,31 @@ moduleFor(
       this.assertText('123');
     }
 
+    '@test creating an array proxy inside a tracking context and immediately updating its content before usage does not trigger backtracking assertion'() {
+      class LoaderComponent extends GlimmerishComponent {
+        get data() {
+          if (!this._data) {
+            this._data = ArrayProxy.create({
+              content: A(),
+            });
+
+            this._data.content.pushObjects([1, 2, 3]);
+          }
+
+          return this._data;
+        }
+      }
+
+      this.registerComponent('loader', {
+        ComponentClass: LoaderComponent,
+        template: '{{#each this.data as |item|}}{{item}}{{/each}}',
+      });
+
+      this.render('<Loader/>');
+
+      this.assertText('123');
+    }
+
     '@test tracked properties that are uninitialized do not throw an error'() {
       let CountComponent = Component.extend({
         count: tracked(),

packages/@ember/-internals/glimmer/tests/integration/syntax/each-test.js

@@ -1,6 +1,6 @@
 import { moduleFor, RenderingTestCase, applyMixins, strip, runTask } from 'internal-test-helpers';
 
-import { get, set, notifyPropertyChange, computed } from '@ember/-internals/metal';
+import { get, set, notifyPropertyChange, computed, on } from '@ember/-internals/metal';
 import { A as emberA, ArrayProxy, RSVP } from '@ember/-internals/runtime';
 import { HAS_NATIVE_SYMBOL } from '@ember/-internals/utils';
 
@@ -1108,6 +1108,22 @@ moduleFor(
   }
 );
 
+moduleFor(
+  'Syntax test: {{#each}} with array proxies, content is updated after init',
+  class extends EachTest {
+    createList(items) {
+      let wrapped = emberA(items);
+      let proxy = ArrayProxy.extend({
+        setup: on('init', function() {
+          this.set('content', emberA(wrapped));
+        }),
+      }).create();
+
+      return { list: proxy, delegate: wrapped };
+    }
+  }
+);
+
 moduleFor(
   'Syntax test: {{#each as}} undefined path',
   class extends RenderingTestCase {

packages/@ember/-internals/runtime/lib/mixins/array.js

@@ -569,7 +569,7 @@ const ArrayMixin = Mixin.create(Enumerable, {
     configurable: true,
     enumerable: false,
     get() {
-      hasListeners(this, '@array:change') || hasListeners(this, '@array:before');
+      return hasListeners(this, '@array:change') || hasListeners(this, '@array:before');
     },
   }),
 

packages/@ember/-internals/runtime/lib/system/array_proxy.js

@@ -11,14 +11,14 @@ import {
   removeArrayObserver,
   replace,
   getChainTagsForKey,
-  tagForProperty,
+  CUSTOM_TAG_FOR,
   arrayContentDidChange,
-  arrayContentWillChange,
+  createTagForProperty,
 } from '@ember/-internals/metal';
 import EmberObject from './object';
 import { isArray, MutableArray } from '../mixins/array';
 import { assert } from '@ember/debug';
-import { combine, update, validate, value } from '@glimmer/reference';
+import { combine, validate, value } from '@glimmer/reference';
 
 const ARRAY_OBSERVER_MAPPING = {
   willChange: '_arrangedContentArrayWillChange',
@@ -106,18 +106,24 @@ export default class ArrayProxy extends EmberObject {
     this._length = 0;
 
     this._arrangedContent = null;
-
     this._arrangedContentIsUpdating = false;
-    this._arrangedContentTag = combine(getChainTagsForKey(this, 'arrangedContent'));
-    this._arrangedContentRevision = value(this._arrangedContentTag);
+    this._arrangedContentTag = null;
+    this._arrangedContentRevision = null;
+  }
 
-    this._addArrangedContentArrayObserver();
+  [PROPERTY_DID_CHANGE]() {
+    this._revalidate();
+  }
 
-    update(tagForProperty(this, '[]'), combine(getChainTagsForKey(this, 'arrangedContent.[]')));
-    update(
-      tagForProperty(this, 'length'),
-      combine(getChainTagsForKey(this, 'arrangedContent.length'))
-    );
+  [CUSTOM_TAG_FOR](key) {
+    if (key === '[]' || key === 'length') {
+      // revalidate eagerly if we're being tracked, since we no longer will
+      // be able to later on due to backtracking re-render assertion
+      this._revalidate();
+      return combine(getChainTagsForKey(this, `arrangedContent.${key}`));
+    }
+
+    return createTagForProperty(this, key);
   }
 
   willDestroy() {
@@ -236,10 +242,6 @@ export default class ArrayProxy extends EmberObject {
     }
   }
 
-  [PROPERTY_DID_CHANGE]() {
-    this._revalidate();
-  }
-
   _updateArrangedContentArray() {
     let oldLength = this._objects === null ? 0 : this._objects.length;
     let arrangedContent = get(this, 'arrangedContent');
@@ -301,13 +303,21 @@ export default class ArrayProxy extends EmberObject {
   }
 
   _revalidate() {
+    if (this._arrangedContentIsUpdating === true) return;
+
     if (
-      !this._arrangedContentIsUpdating &&
+      this._arrangedContentTag === null ||
       !validate(this._arrangedContentTag, this._arrangedContentRevision)
     ) {
-      this._arrangedContentIsUpdating = true;
-      this._updateArrangedContentArray();
-      this._arrangedContentIsUpdating = false;
+      if (this._arrangedContentTag === null) {
+        // This is the first time the proxy has been setup, only add the observer
+        // don't trigger any events
+        this._addArrangedContentArrayObserver();
+      } else {
+        this._arrangedContentIsUpdating = true;
+        this._updateArrangedContentArray();
+        this._arrangedContentIsUpdating = false;
+      }
 
       this._arrangedContentTag = combine(getChainTagsForKey(this, 'arrangedContent'));
       this._arrangedContentRevision = value(this._arrangedContentTag);
@@ -328,10 +338,6 @@ ArrayProxy.reopen(MutableArray, {
 
   // Array proxies don't need to notify when they change since their `[]` tag is
   // already dependent on the `[]` tag of `arrangedContent`
-  arrayContentWillChange(startIdx, removeAmt, addAmt) {
-    return arrayContentWillChange(this, startIdx, removeAmt, addAmt, false);
-  },
-
   arrayContentDidChange(startIdx, removeAmt, addAmt) {
     return arrayContentDidChange(this, startIdx, removeAmt, addAmt, false);
   },

packages/@ember/-internals/runtime/tests/helpers/array.js

@@ -314,7 +314,7 @@ export function runArrayTests(name, Tests, ...types) {
           moduleFor(`EmberArray: ${name}`, Tests, EmberArrayHelpers);
           break;
         case 'MutableArray':
-          moduleFor(`MutableArray: ${name}`, Tests, EmberArrayHelpers);
+          moduleFor(`MutableArray: ${name}`, Tests, MutableArrayHelpers);
           break;
         case 'CopyableArray':
           moduleFor(`CopyableArray: ${name}`, Tests, CopyableArray);
@@ -323,7 +323,7 @@ export function runArrayTests(name, Tests, ...types) {
           moduleFor(`CopyableNativeArray: ${name}`, Tests, CopyableNativeArray);
           break;
         case 'NativeArray':
-          moduleFor(`NativeArray: ${name}`, Tests, EmberArrayHelpers);
+          moduleFor(`NativeArray: ${name}`, Tests, NativeArrayHelpers);
           break;
       }
     });

packages/@ember/-internals/runtime/tests/mixins/array_test.js

@@ -11,8 +11,7 @@ import {
   arrayContentWillChange,
 } from '@ember/-internals/metal';
 import EmberObject from '../../lib/system/object';
-import EmberArray from '../../lib/mixins/array';
-import { A as emberA } from '../../lib/mixins/array';
+import EmberArray, { A as emberA } from '../../lib/mixins/array';
 import { moduleFor, AbstractTestCase, runLoopSettled } from 'internal-test-helpers';
 
 /*
@@ -254,6 +253,22 @@ moduleFor(
       arrayContentDidChange(obj);
       assert.deepEqual(observer._after, null);
     }
+
+    ['@test hasArrayObservers should work'](assert) {
+      assert.equal(
+        obj.hasArrayObservers,
+        true,
+        'correctly shows it has an array observer when one exists'
+      );
+
+      removeArrayObserver(obj, observer);
+
+      assert.equal(
+        obj.hasArrayObservers,
+        false,
+        'correctly shows it has an array observer when one exists'
+      );
+    }
   }
 );
 

packages/@ember/-internals/runtime/tests/system/array_proxy/watching_and_listening_test.js

@@ -38,6 +38,12 @@ moduleFor(
       let content2 = A();
       let proxy = ArrayProxy.create({ content: content1 });
 
+      assert.deepEqual(sortedListenersFor(content1, '@array:before'), []);
+      assert.deepEqual(sortedListenersFor(content1, '@array:change'), []);
+
+      // setup proxy
+      proxy.length;
+
       assert.deepEqual(sortedListenersFor(content1, '@array:before'), [
         '_arrangedContentArrayWillChange',
       ]);