From 8c4c3e09c8bd6da57c5c9677d3ad4899d3ef97ae Mon Sep 17 00:00:00 2001 From: Chris Garrett Date: Sun, 17 Mar 2019 11:42:59 -0700 Subject: [PATCH] [FEATURE] Adds Array Tracking This PR adds autotracking of arrays and Ember arrays. This is useful for arrays in particular because they have an indeterminate number of tracked values, and it would be impossible to actually track all values manually. The strategy is to check any value that we get back from a tracked getter method and see if it's an array - if so, we push its tag directly onto the stack. Later on, if the array is marked dirty via `pushObject` or `shiftObject` or some other method, it'll invalidate the autotrack stack that included the array. The one scenario this strategy does _not_ work with current is _creating_ an array in a getter dynamically: ```js class Foo { get bar() { if (!this._bar) { this._bar = []; } return this._bar; } } ``` In practice, these cases should be fairly rare, since getters should typically represent derived state from some source value, which would be tracked. Computed properties/cached getters _do_ add the tag for the object as well, since their state can be more long lived, so this won't be an issue for those. --- .../-internals/glimmer/lib/utils/iterable.ts | 4 +- .../integration/components/tracked-test.js | 29 +++++++++- .../tests/integration/helpers/tracked-test.js | 33 ++++++++++- .../@ember/-internals/metal/lib/computed.ts | 14 ++++- .../-internals/metal/lib/property_get.ts | 22 +++++-- packages/@ember/-internals/metal/lib/tags.ts | 9 ++- .../@ember/-internals/metal/lib/tracked.ts | 18 ++++-- .../metal/tests/tracked/validation_test.js | 57 ++++++++++++++++++- packages/@ember/-internals/runtime/index.d.ts | 2 - packages/@ember/-internals/runtime/index.js | 1 - .../-internals/runtime/lib/mixins/array.js | 9 +-- packages/@ember/-internals/utils/index.ts | 1 + .../-internals/utils/lib/ember-array.ts | 7 +++ .../utils/tests/trackable-object-test.js | 16 ++++++ 14 files changed, 193 insertions(+), 29 deletions(-) create mode 100644 packages/@ember/-internals/utils/lib/ember-array.ts create mode 100644 packages/@ember/-internals/utils/tests/trackable-object-test.js diff --git a/packages/@ember/-internals/glimmer/lib/utils/iterable.ts b/packages/@ember/-internals/glimmer/lib/utils/iterable.ts index 75d4636c060..9c1ff036637 100644 --- a/packages/@ember/-internals/glimmer/lib/utils/iterable.ts +++ b/packages/@ember/-internals/glimmer/lib/utils/iterable.ts @@ -1,6 +1,6 @@ import { get, objectAt, tagFor, tagForProperty } from '@ember/-internals/metal'; -import { _contentFor, isEmberArray } from '@ember/-internals/runtime'; -import { guidFor, HAS_NATIVE_SYMBOL, isProxy } from '@ember/-internals/utils'; +import { _contentFor } from '@ember/-internals/runtime'; +import { guidFor, HAS_NATIVE_SYMBOL, isEmberArray, isProxy } from '@ember/-internals/utils'; import { assert } from '@ember/debug'; import { AbstractIterable, diff --git a/packages/@ember/-internals/glimmer/tests/integration/components/tracked-test.js b/packages/@ember/-internals/glimmer/tests/integration/components/tracked-test.js index e30ed94a58b..1ff10494c87 100644 --- a/packages/@ember/-internals/glimmer/tests/integration/components/tracked-test.js +++ b/packages/@ember/-internals/glimmer/tests/integration/components/tracked-test.js @@ -1,5 +1,5 @@ import { EMBER_METAL_TRACKED_PROPERTIES } from '@ember/canary-features'; -import { Object as EmberObject } from '@ember/-internals/runtime'; +import { Object as EmberObject, A } from '@ember/-internals/runtime'; import { tracked, nativeDescDecorator as descriptor } from '@ember/-internals/metal'; import { moduleFor, RenderingTestCase, strip, runTask } from 'internal-test-helpers'; import GlimmerishComponent from '../../utils/glimmerish-component'; @@ -151,6 +151,33 @@ if (EMBER_METAL_TRACKED_PROPERTIES) { this.assertText('1'); } + '@test array properties rerender when updated'() { + let NumListComponent = Component.extend({ + numbers: tracked({ initializer: () => A([1, 2, 3]) }), + + addNumber() { + this.numbers.pushObject(4); + }, + }); + + this.registerComponent('num-list', { + ComponentClass: NumListComponent, + template: strip` + + `, + }); + + this.render(''); + + this.assertText('123'); + + runTask(() => this.$('button').click()); + + this.assertText('1234'); + } + '@test getters update when dependent properties are invalidated'() { let CountComponent = Component.extend({ count: tracked({ value: 0 }), 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 f1fb5212eb6..195103a79ee 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,5 @@ import { EMBER_METAL_TRACKED_PROPERTIES } from '@ember/canary-features'; -import { Object as EmberObject } from '@ember/-internals/runtime'; +import { Object as EmberObject, A } from '@ember/-internals/runtime'; import { tracked, nativeDescDecorator as descriptor } from '@ember/-internals/metal'; import { moduleFor, RenderingTestCase, strip, runTask } from 'internal-test-helpers'; @@ -138,6 +138,37 @@ if (EMBER_METAL_TRACKED_PROPERTIES) { assert.strictEqual(computeCount, 2, 'compute is called exactly 2 times'); } + '@test array properties rerender when updated'() { + let NumListComponent = Component.extend({ + numbers: tracked({ initializer: () => A([1, 2, 3]) }), + + 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/computed.ts b/packages/@ember/-internals/metal/lib/computed.ts index d3a94dc50ba..02a2d2d25c4 100644 --- a/packages/@ember/-internals/metal/lib/computed.ts +++ b/packages/@ember/-internals/metal/lib/computed.ts @@ -1,5 +1,5 @@ import { Meta, meta as metaFor, peekMeta } from '@ember/-internals/meta'; -import { inspect, toString } from '@ember/-internals/utils'; +import { inspect, isEmberArray, toString } from '@ember/-internals/utils'; import { EMBER_METAL_TRACKED_PROPERTIES, EMBER_NATIVE_DECORATOR_SUPPORT, @@ -27,7 +27,7 @@ import expandProperties from './expand_properties'; import { defineProperty } from './properties'; import { notifyPropertyChange } from './property_events'; import { set } from './property_set'; -import { tagForProperty, update } from './tags'; +import { tagFor, tagForProperty, update } from './tags'; import { getCurrentTracker, setCurrentTracker } from './tracked'; export type ComputedPropertyGetter = (keyName: string) => any; @@ -553,7 +553,15 @@ export class ComputedProperty extends ComputedDescriptor { if (EMBER_METAL_TRACKED_PROPERTIES) { setCurrentTracker(parent!); let tag = tracker!.combine(); - if (parent) parent.add(tag); + if (parent) { + parent.add(tag); + + // 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(ret) || isEmberArray(ret)) { + parent.add(tagFor(ret)); + } + } update(propertyTag as any, tag); setLastRevisionFor(obj, keyName, (propertyTag as any).value()); diff --git a/packages/@ember/-internals/metal/lib/property_get.ts b/packages/@ember/-internals/metal/lib/property_get.ts index a86567d06d2..fe9975a7c3a 100644 --- a/packages/@ember/-internals/metal/lib/property_get.ts +++ b/packages/@ember/-internals/metal/lib/property_get.ts @@ -1,13 +1,13 @@ /** @module @ember/object */ -import { HAS_NATIVE_PROXY, symbol } from '@ember/-internals/utils'; +import { HAS_NATIVE_PROXY, isEmberArray, symbol } from '@ember/-internals/utils'; import { EMBER_METAL_TRACKED_PROPERTIES } from '@ember/canary-features'; import { assert } from '@ember/debug'; import { DEBUG } from '@glimmer/env'; import { descriptorForProperty } from './descriptor_map'; import { isPath } from './path_cache'; -import { tagForProperty } from './tags'; +import { tagFor, tagForProperty } from './tags'; import { getCurrentTracker } from './tracked'; export const PROXY_CONTENT = symbol('PROXY_CONTENT'); @@ -103,9 +103,13 @@ export function get(obj: object, keyName: string): any { let value: any; if (isObjectLike) { + let tracker = null; + if (EMBER_METAL_TRACKED_PROPERTIES) { - let tracker = getCurrentTracker(); - if (tracker) tracker.add(tagForProperty(obj, keyName)); + tracker = getCurrentTracker(); + if (tracker !== null) { + tracker.add(tagForProperty(obj, keyName)); + } } let descriptor = descriptorForProperty(obj, keyName); @@ -118,6 +122,16 @@ export function get(obj: object, keyName: string): any { } else { value = obj[keyName]; } + + // 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 ( + EMBER_METAL_TRACKED_PROPERTIES && + tracker !== null && + (Array.isArray(value) || isEmberArray(value)) + ) { + tracker.add(tagFor(value)); + } } else { value = obj[keyName]; } diff --git a/packages/@ember/-internals/metal/lib/tags.ts b/packages/@ember/-internals/metal/lib/tags.ts index 9150f85b484..d23b689dc66 100644 --- a/packages/@ember/-internals/metal/lib/tags.ts +++ b/packages/@ember/-internals/metal/lib/tags.ts @@ -43,10 +43,13 @@ export function tagForProperty(object: any, propertyKey: string | symbol, _meta? export function tagFor(object: any | null, _meta?: Meta): Tag { if (typeof object === 'object' && object !== null) { let meta = _meta === undefined ? metaFor(object) : _meta; - return meta.writableTag(makeTag); - } else { - return CONSTANT_TAG; + + if (!meta.isMetaDestroyed()) { + return meta.writableTag(makeTag); + } } + + return CONSTANT_TAG; } export let dirty: (tag: Tag) => void; diff --git a/packages/@ember/-internals/metal/lib/tracked.ts b/packages/@ember/-internals/metal/lib/tracked.ts index 7b5752e42c7..07d52c9b31f 100644 --- a/packages/@ember/-internals/metal/lib/tracked.ts +++ b/packages/@ember/-internals/metal/lib/tracked.ts @@ -1,11 +1,11 @@ -import { HAS_NATIVE_SYMBOL, symbol as emberSymbol } from '@ember/-internals/utils'; +import { HAS_NATIVE_SYMBOL, isEmberArray, symbol as emberSymbol } from '@ember/-internals/utils'; import { EMBER_NATIVE_DECORATOR_SUPPORT } from '@ember/canary-features'; import { assert } from '@ember/debug'; import { DEBUG } from '@glimmer/env'; import { combine, CONSTANT_TAG, Tag } from '@glimmer/reference'; import { Decorator, DecoratorPropertyDescriptor, isElementDescriptor } from './decorator'; import { setClassicDecorator } from './descriptor_map'; -import { dirty, ensureRunloop, tagFor, tagForProperty } from './tags'; +import { dirty, ensureRunloop, tagFor, tagForProperty, update } from './tags'; type Option = T | null; @@ -131,7 +131,7 @@ export function tracked(...args: any[]): Decorator | DecoratorPropertyDescriptor assert( `The options object passed to tracked() may only contain a 'value' or 'initializer' property, not both. Received: [${keys}]`, keys.length <= 1 && - (keys[0] === undefined || keys[0] === 'value' || keys[0] === 'undefined') + (keys[0] === undefined || keys[0] === 'value' || keys[0] === 'initializer') ); assert( @@ -201,13 +201,23 @@ function descriptorForField([_target, key, desc]: [ configurable: true, get(): any { - if (CURRENT_TRACKER) CURRENT_TRACKER.add(tagForProperty(this, key)); + let propertyTag = tagForProperty(this, key); + + if (CURRENT_TRACKER) CURRENT_TRACKER.add(propertyTag); // If the field has never been initialized, we should initialize it if (!(secretKey in this)) { this[secretKey] = typeof initializer === 'function' ? initializer.call(this) : undefined; } + let value = this[secretKey]; + + // 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) || isEmberArray(value)) { + update(propertyTag, tagFor(value)); + } + return this[secretKey]; }, diff --git a/packages/@ember/-internals/metal/tests/tracked/validation_test.js b/packages/@ember/-internals/metal/tests/tracked/validation_test.js index 3fc071dcd22..9047877ed74 100644 --- a/packages/@ember/-internals/metal/tests/tracked/validation_test.js +++ b/packages/@ember/-internals/metal/tests/tracked/validation_test.js @@ -1,9 +1,18 @@ -import { computed, defineProperty, get, set, tagForProperty, tracked } from '../..'; +import { + computed, + defineProperty, + get, + set, + tagForProperty, + tracked, + notifyPropertyChange, +} from '../..'; import { EMBER_METAL_TRACKED_PROPERTIES, EMBER_NATIVE_DECORATOR_SUPPORT, } from '@ember/canary-features'; +import { EMBER_ARRAY } from '@ember/-internals/utils'; import { AbstractTestCase, moduleFor } from 'internal-test-helpers'; import { track } from './support'; @@ -285,6 +294,52 @@ if (EMBER_METAL_TRACKED_PROPERTIES && EMBER_NATIVE_DECORATOR_SUPPORT) { assert.equal(get(obj, 'full'), 'Tizzle Dale'); } + + ['@test interaction with arrays'](assert) { + class EmberObject { + @tracked array = []; + } + + let obj = new EmberObject(); + + let tag = tagForProperty(obj, 'array'); + let snapshot = tag.value(); + + let array = get(obj, 'array'); + assert.deepEqual(array, []); + assert.equal(tag.validate(snapshot), true); + + array.push(1); + notifyPropertyChange(array, '[]'); + assert.equal( + tag.validate(snapshot), + false, + 'invalid after pushing an object and notifying on the array' + ); + } + + ['@test interaction with ember arrays'](assert) { + class EmberObject { + @tracked emberArray = { + [EMBER_ARRAY]: true, + }; + } + + let obj = new EmberObject(); + + let tag = tagForProperty(obj, 'emberArray'); + let snapshot = tag.value(); + + let emberArray = get(obj, 'emberArray'); + assert.equal(tag.validate(snapshot), true); + + set(emberArray, 'foo', 123); + assert.equal( + tag.validate(snapshot), + false, + 'invalid after setting a property on the object' + ); + } } ); } diff --git a/packages/@ember/-internals/runtime/index.d.ts b/packages/@ember/-internals/runtime/index.d.ts index 7c6be78ec76..b3a044c855c 100644 --- a/packages/@ember/-internals/runtime/index.d.ts +++ b/packages/@ember/-internals/runtime/index.d.ts @@ -13,8 +13,6 @@ export function deprecatingAlias( export const FrameworkObject: any; export const Object: any; -export function isEmberArray(arr: any): boolean; - export function _contentFor(proxy: any): any; export const A: any; diff --git a/packages/@ember/-internals/runtime/index.js b/packages/@ember/-internals/runtime/index.js index a4396c92b96..6558895952b 100644 --- a/packages/@ember/-internals/runtime/index.js +++ b/packages/@ember/-internals/runtime/index.js @@ -6,7 +6,6 @@ export { default as compare } from './lib/compare'; export { default as isEqual } from './lib/is-equal'; export { default as Array, - isEmberArray, NativeArray, A, MutableArray, diff --git a/packages/@ember/-internals/runtime/lib/mixins/array.js b/packages/@ember/-internals/runtime/lib/mixins/array.js index 3fb3400d802..2d6089c4c2c 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 { symbol, HAS_NATIVE_PROXY, tryInvoke } from '@ember/-internals/utils'; +import { EMBER_ARRAY, HAS_NATIVE_PROXY, tryInvoke } from '@ember/-internals/utils'; import { get, set, @@ -29,11 +29,6 @@ import MutableEnumerable from './mutable_enumerable'; import { typeOf } from '../type-of'; const EMPTY_ARRAY = Object.freeze([]); -const EMBER_ARRAY = symbol('EMBER_ARRAY'); - -export function isEmberArray(obj) { - return obj && obj[EMBER_ARRAY]; -} const identityFunction = item => item; @@ -722,7 +717,7 @@ const ArrayMixin = Mixin.create(Enumerable, { Returns an array with just the items with the matched property. You can pass an optional second argument with the target value. Otherwise this will match any property that evaluates to `true`. - + Example Usage: ```javascript diff --git a/packages/@ember/-internals/utils/index.ts b/packages/@ember/-internals/utils/index.ts index 8c09ac3e8eb..f9cfce25d82 100644 --- a/packages/@ember/-internals/utils/index.ts +++ b/packages/@ember/-internals/utils/index.ts @@ -32,6 +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, isEmberArray } from './lib/ember-array'; import symbol from './lib/symbol'; export const NAME_KEY = symbol('NAME_KEY'); diff --git a/packages/@ember/-internals/utils/lib/ember-array.ts b/packages/@ember/-internals/utils/lib/ember-array.ts new file mode 100644 index 00000000000..4efbff614e4 --- /dev/null +++ b/packages/@ember/-internals/utils/lib/ember-array.ts @@ -0,0 +1,7 @@ +import symbol from './symbol'; + +export const EMBER_ARRAY = symbol('EMBER_ARRAY'); + +export function isEmberArray(obj: any) { + return obj && obj[EMBER_ARRAY]; +} diff --git a/packages/@ember/-internals/utils/tests/trackable-object-test.js b/packages/@ember/-internals/utils/tests/trackable-object-test.js new file mode 100644 index 00000000000..ee8542b6e28 --- /dev/null +++ b/packages/@ember/-internals/utils/tests/trackable-object-test.js @@ -0,0 +1,16 @@ +import { EMBER_ARRAY, 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; + + let instance = new Test(); + + assert.equal(isEmberArray(instance), true); + } + } +);