Merge pull request #19164 from emberjs/bugfix/entangle-ember-arrays-i…

…n-get

[BUGFIX lts] Entangles custom EmberArray implementations when accessed

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

@@ -1,5 +1,11 @@
-import { Object as EmberObject, A } from '@ember/-internals/runtime';
-import { get, set, tracked, nativeDescDecorator as descriptor } from '@ember/-internals/metal';
+import { Object as EmberObject, A, MutableArray } from '@ember/-internals/runtime';
+import {
+  get,
+  set,
+  tracked,
+  nativeDescDecorator as descriptor,
+  notifyPropertyChange,
+} from '@ember/-internals/metal';
 import Service, { inject } from '@ember/service';
 import { moduleFor, RenderingTestCase, strip, runTask } from 'internal-test-helpers';
 
@@ -169,6 +175,61 @@ moduleFor(
       this.assertText('1, 2, 3, 4');
     }
 
+    '@test custom ember array properties rerender when updated'() {
+      let CustomArray = EmberObject.extend(MutableArray, {
+        init() {
+          this._super(...arguments);
+          this._vals = [1, 2, 3];
+        },
+
+        objectAt(index) {
+          return this._vals[index];
+        },
+
+        replace(start, deleteCount, items = []) {
+          this._vals.splice(start, deleteCount, ...items);
+          notifyPropertyChange(this, '[]');
+        },
+
+        join() {
+          return this._vals.join(...arguments);
+        },
+
+        get length() {
+          return this._vals.length;
+        },
+      });
+
+      let NumListComponent = Component.extend({
+        numbers: tracked({ initializer: () => CustomArray.create() }),
+
+        addNumber() {
+          this.numbers.pushObject(4);
+        },
+      });
+
+      this.registerComponent('num-list', {
+        ComponentClass: NumListComponent,
+        template: strip`
+            <button {{action this.addNumber}}>
+              {{join this.numbers}}
+            </button>
+          `,
+      });
+
+      this.registerHelper('join', ([value]) => {
+        return value.join(', ');
+      });
+
+      this.render('<NumList />');
+
+      this.assertText('1, 2, 3');
+
+      runTask(() => this.$('button').click());
+
+      this.assertText('1, 2, 3, 4');
+    }
+
     '@test nested getters update when dependent properties are invalidated'(assert) {
       let computeCount = 0;
 

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

@@ -1,7 +1,7 @@
 /**
 @module @ember/object
 */
-import { HAS_NATIVE_PROXY, setProxy, symbol } from '@ember/-internals/utils';
+import { HAS_NATIVE_PROXY, isEmberArray, setProxy, symbol } from '@ember/-internals/utils';
 import { assert, deprecate } from '@ember/debug';
 import { DEBUG } from '@glimmer/env';
 import {
@@ -130,7 +130,7 @@ export function _getProp(obj: object, keyName: string) {
     if (isTracking()) {
       consumeTag(tagFor(obj, keyName));
 
-      if (Array.isArray(value)) {
+      if (Array.isArray(value) || isEmberArray(value)) {
         // Add the tag of the returned value if it is an array, since arrays
         // should always cause updates if they are consumed and then changed
         consumeTag(tagFor(value, '[]'));

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

@@ -1,3 +1,4 @@
+import { isEmberArray } from '@ember/-internals/utils';
 import { assert } from '@ember/debug';
 import { DEBUG } from '@glimmer/env';
 import { consumeTag, dirtyTagFor, tagFor, trackedData } from '@glimmer/validator';
@@ -162,7 +163,7 @@ function descriptorForField([_target, key, desc]: [
 
       // Add the tag of the returned value if it is an array, since arrays
       // should always cause updates if they are consumed and then changed
-      if (Array.isArray(value)) {
+      if (Array.isArray(value) || isEmberArray(value)) {
         consumeTag(tagFor(value, '[]'));
       }
 

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

@@ -3,7 +3,7 @@
 */
 import { DEBUG } from '@glimmer/env';
 import { PROXY_CONTENT } from '@ember/-internals/metal';
-import { EMBER_ARRAY, HAS_NATIVE_PROXY, tryInvoke } from '@ember/-internals/utils';
+import { setEmberArray, HAS_NATIVE_PROXY, tryInvoke } from '@ember/-internals/utils';
 import {
   get,
   set,
@@ -216,7 +216,10 @@ function mapBy(key) {
   @public
 */
 const ArrayMixin = Mixin.create(Enumerable, {
-  [EMBER_ARRAY]: true,
+  init() {
+    this._super(...arguments);
+    setEmberArray(this);
+  },
 
   /**
     __Required.__ You must implement this method to apply this mixin.

packages/@ember/-internals/utils/index.ts

@@ -32,7 +32,7 @@ export { HAS_NATIVE_SYMBOL } from './lib/symbol-utils';
 export { HAS_NATIVE_PROXY } from './lib/proxy-utils';
 export { isProxy, setProxy } from './lib/is_proxy';
 export { default as Cache } from './lib/cache';
-export { EMBER_ARRAY, EmberArray, isEmberArray } from './lib/ember-array';
+export { EmberArray, setEmberArray, isEmberArray } from './lib/ember-array';
 export {
   setupMandatorySetter,
   teardownMandatorySetter,

packages/@ember/-internals/utils/lib/ember-array.ts

@@ -1,6 +1,6 @@
-import { symbol } from './symbol';
+import { _WeakSet } from '@glimmer/util';
 
-export const EMBER_ARRAY = symbol('EMBER_ARRAY');
+const EMBER_ARRAYS = new _WeakSet();
 
 export interface EmberArray<T> {
   length: number;
@@ -10,6 +10,10 @@ export interface EmberArray<T> {
   splice(start: number, deleteCount: number, ...items: T[]): void;
 }
 
-export function isEmberArray(obj: any): obj is EmberArray<unknown> {
-  return obj && obj[EMBER_ARRAY];
+export function setEmberArray(obj: object) {
+  EMBER_ARRAYS.add(obj);
+}
+
+export function isEmberArray(obj: unknown): obj is EmberArray<unknown> {
+  return EMBER_ARRAYS.has(obj as object);
 }

packages/@ember/-internals/utils/tests/trackable-object-test.js

@@ -1,12 +1,15 @@
-import { EMBER_ARRAY, isEmberArray } from '..';
+import { setEmberArray, isEmberArray } from '..';
 import { moduleFor, AbstractTestCase } from 'internal-test-helpers';
 
 moduleFor(
   '@ember/-internals/utils Trackable Object',
   class extends AbstractTestCase {
     ['@test classes'](assert) {
-      class Test {}
-      Test.prototype[EMBER_ARRAY] = true;
+      class Test {
+        constructor() {
+          setEmberArray(this);
+        }
+      }
 
       let instance = new Test();