From ef26b1fa43edcce2bd04a78ef087ff5615683af2 Mon Sep 17 00:00:00 2001 From: Chris Garrett Date: Mon, 28 Sep 2020 12:58:27 -0700 Subject: [PATCH] [BUGFIX lts] Entangles custom EmberArray implementations when accessed Restores the previous logic that existed for entangling custom EmberArray implementations when they are accessed, along with a test for the functionality. --- .../tests/integration/helpers/tracked-test.js | 65 ++++++++++++++++++- .../-internals/metal/lib/property_get.ts | 4 +- .../@ember/-internals/metal/lib/tracked.ts | 3 +- .../-internals/runtime/lib/mixins/array.js | 7 +- packages/@ember/-internals/utils/index.ts | 2 +- .../-internals/utils/lib/ember-array.ts | 12 ++-- .../utils/tests/trackable-object-test.js | 9 ++- 7 files changed, 87 insertions(+), 15 deletions(-) diff --git a/packages/@ember/-internals/glimmer/tests/integration/helpers/tracked-test.js b/packages/@ember/-internals/glimmer/tests/integration/helpers/tracked-test.js index fe81f543951..cba9cd63f49 100644 --- a/packages/@ember/-internals/glimmer/tests/integration/helpers/tracked-test.js +++ b/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` + + `, + }); + + this.registerHelper('join', ([value]) => { + return value.join(', '); + }); + + this.render(''); + + 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; diff --git a/packages/@ember/-internals/metal/lib/property_get.ts b/packages/@ember/-internals/metal/lib/property_get.ts index bc1f8e08c0c..b5de81e4554 100644 --- a/packages/@ember/-internals/metal/lib/property_get.ts +++ b/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, '[]')); diff --git a/packages/@ember/-internals/metal/lib/tracked.ts b/packages/@ember/-internals/metal/lib/tracked.ts index df9a47025f5..951b88f7ddd 100644 --- a/packages/@ember/-internals/metal/lib/tracked.ts +++ b/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, '[]')); } diff --git a/packages/@ember/-internals/runtime/lib/mixins/array.js b/packages/@ember/-internals/runtime/lib/mixins/array.js index f5baade158f..22c6d8d49e6 100644 --- a/packages/@ember/-internals/runtime/lib/mixins/array.js +++ b/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. diff --git a/packages/@ember/-internals/utils/index.ts b/packages/@ember/-internals/utils/index.ts index 0a6b3815bc6..12504123e60 100644 --- a/packages/@ember/-internals/utils/index.ts +++ b/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, diff --git a/packages/@ember/-internals/utils/lib/ember-array.ts b/packages/@ember/-internals/utils/lib/ember-array.ts index 81f43362eff..f6b3d68a2b5 100644 --- a/packages/@ember/-internals/utils/lib/ember-array.ts +++ b/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 { length: number; @@ -10,6 +10,10 @@ export interface EmberArray { splice(start: number, deleteCount: number, ...items: T[]): void; } -export function isEmberArray(obj: any): obj is EmberArray { - return obj && obj[EMBER_ARRAY]; +export function setEmberArray(obj: object) { + EMBER_ARRAYS.add(obj); +} + +export function isEmberArray(obj: unknown): obj is EmberArray { + return EMBER_ARRAYS.has(obj as object); } diff --git a/packages/@ember/-internals/utils/tests/trackable-object-test.js b/packages/@ember/-internals/utils/tests/trackable-object-test.js index ee8542b6e28..7355dfad3d7 100644 --- a/packages/@ember/-internals/utils/tests/trackable-object-test.js +++ b/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();