Merge pull request #18703 from emberjs/bugfix/fix-array-proxy-tags

[BUGFIX release] Correctly links ArrayProxy tags to `arrangedContent`

Author: rwjblue

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/validator';
+import { combine, update, validate, value } from '@glimmer/validator';
 
 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);
+  },
 });