[BUGFIX release] Correctly links ArrayProxy tags to arrangedContent

Currently, if `arrangedContent` is overridden in an ArrayProxy with a
computed property that depends on changes to another array/context,
those changes will not propagate correctly. This is because we never
link the tags of the ArrayProxy to the corresponding tags of the
`arrangedContent`, instead relying on array observers to propagate
changes. This works when the underlying array is being changed directly,
but _doesn't_ work if the array is being replaced entirely (e.g. the
computed property has invalidated and needs to recompute).

This PR ensures that ArrayProxy tags are setup correctly, so that if
`arrangedContent` ever changes, the proxy will also propagate those
changes. This will affect anything that depends on the ArrayProxy
directly, such as `{{#each}}` loops and other computed properties.

One side effect of this is that ArrayProxy's no longer need to manually
dirty themselves, and in fact attempting to do so can trigger the
backtracking rerender assertion (specifically when the proxy first
attempts to update/synchronize while rendering). Internally, a boolean
flag has been added to the array change methods to allow it to opt-out
of sending a notification.

Author: Chris Garrett

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 } from '@ember/-internals/metal';
+import { get, set, notifyPropertyChange, computed } from '@ember/-internals/metal';
 import { A as emberA, ArrayProxy, RSVP } from '@ember/-internals/runtime';
 import { HAS_NATIVE_SYMBOL } from '@ember/-internals/utils';
 
@@ -1089,6 +1089,25 @@ moduleFor(
   }
 );
 
+moduleFor(
+  'Syntax test: {{#each}} with array proxies, arrangedContent depends on external content',
+  class extends EachTest {
+    createList(items) {
+      let wrapped = emberA(items);
+      let proxy = ArrayProxy.extend({
+        arrangedContent: computed('wrappedItems.[]', function() {
+          // Slice the items to ensure that updates must be propogated
+          return this.wrappedItems.slice();
+        }),
+      }).create({
+        wrappedItems: wrapped,
+      });
+
+      return { list: proxy, delegate: wrapped };
+    }
+  }
+);
+
 moduleFor(
   'Syntax test: {{#each as}} undefined path',
   class extends RenderingTestCase {

packages/@ember/-internals/metal/lib/array_events.ts

@@ -32,7 +32,8 @@ export function arrayContentDidChange<T extends { length: number }>(
   array: T,
   startIdx: number,
   removeAmt: number,
-  addAmt: number
+  addAmt: number,
+  notify = true
 ): T {
   // if no args are passed assume everything changes
   if (startIdx === undefined) {
@@ -50,11 +51,13 @@ export function arrayContentDidChange<T extends { length: number }>(
 
   let meta = peekMeta(array);
 
-  if (addAmt < 0 || removeAmt < 0 || addAmt - removeAmt !== 0) {
-    notifyPropertyChange(array, 'length', meta);
-  }
+  if (notify) {
+    if (addAmt < 0 || removeAmt < 0 || addAmt - removeAmt !== 0) {
+      notifyPropertyChange(array, 'length', meta);
+    }
 
-  notifyPropertyChange(array, '[]', meta);
+    notifyPropertyChange(array, '[]', meta);
+  }
 
   sendEvent(array, '@array:change', [array, startIdx, removeAmt, addAmt]);
 

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

@@ -11,11 +11,14 @@ import {
   removeArrayObserver,
   replace,
   getChainTagsForKey,
+  tagForProperty,
+  arrayContentDidChange,
+  arrayContentWillChange,
 } from '@ember/-internals/metal';
 import EmberObject from './object';
 import { isArray, MutableArray } from '../mixins/array';
 import { assert } from '@ember/debug';
-import { combine, validate, value } from '@glimmer/reference';
+import { combine, update, validate, value } from '@glimmer/reference';
 
 const ARRAY_OBSERVER_MAPPING = {
   willChange: '_arrangedContentArrayWillChange',
@@ -109,6 +112,12 @@ export default class ArrayProxy extends EmberObject {
     this._arrangedContentRevision = value(this._arrangedContentTag);
 
     this._addArrangedContentArrayObserver();
+
+    update(tagForProperty(this, '[]'), combine(getChainTagsForKey(this, 'arrangedContent.[]')));
+    update(
+      tagForProperty(this, 'length'),
+      combine(getChainTagsForKey(this, 'arrangedContent.length'))
+    );
   }
 
   willDestroy() {
@@ -316,4 +325,14 @@ ArrayProxy.reopen(MutableArray, {
     @public
   */
   arrangedContent: alias('content'),
+
+  // 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);
+  },
 });