[BUGFIX release] Make ArrayProxy Lazy

This PR refactors ArrayProxy to only begin observing the proxied array's
changes the first time the proxy is actually used. This fixes a number
of bugs that have popped up recently, and also ensures that ArrayProxy
creation does not accidentally entangle with the autotracking stack. It
also simplifies the code overall, and conceptually is more in line with
the way that autotracking works.

In addition, it adds a CUSTOM_TAG_FOR to the proxy, to forward the `[]`
and `length` tags from its content directly rather than attempting to
managed them itself. This keeps the system from encountering issues
related to tag updates, and from needing to re-notify when changes are
propagated.

As part of fixing this, I also discovered that the `hasArrayObservers`
API has been broken since ~3.11. I fixed it and added a test.

(cherry picked from commit 8bbedfa)

Author: Chris Garrett

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,13 @@ import {
   removeArrayObserver,
   replace,
   getChainTagsForKey,
-  tagForProperty,
+  CUSTOM_TAG_FOR,
   arrayContentDidChange,
-  arrayContentWillChange,
 } 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/validator';
+import { combine, validate, value, tagFor } from '@glimmer/validator';
 
 const ARRAY_OBSERVER_MAPPING = {
   willChange: '_arrangedContentArrayWillChange',
@@ -106,18 +105,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 tagFor(this, key);
   }
 
   willDestroy() {
@@ -236,10 +241,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 +302,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 +337,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',
       ]);