Skip to content

Commit

Permalink
[FEATURE] Adds Array Tracking
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Chris Garrett committed Apr 19, 2019
1 parent f0816d4 commit 8c4c3e0
Show file tree
Hide file tree
Showing 14 changed files with 193 additions and 29 deletions.
4 changes: 2 additions & 2 deletions packages/@ember/-internals/glimmer/lib/utils/iterable.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand Down
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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`
<button {{action this.addNumber}}>
{{#each this.numbers as |num|}}{{num}}{{/each}}
</button>
`,
});

this.render('<NumList />');

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 }),
Expand Down
Original file line number Diff line number Diff line change
@@ -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';

Expand Down Expand Up @@ -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`
<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;

Expand Down
14 changes: 11 additions & 3 deletions packages/@ember/-internals/metal/lib/computed.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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());
Expand Down
22 changes: 18 additions & 4 deletions packages/@ember/-internals/metal/lib/property_get.ts
Original file line number Diff line number Diff line change
@@ -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');
Expand Down Expand Up @@ -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);
Expand All @@ -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];
}
Expand Down
9 changes: 6 additions & 3 deletions packages/@ember/-internals/metal/lib/tags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
18 changes: 14 additions & 4 deletions packages/@ember/-internals/metal/lib/tracked.ts
Original file line number Diff line number Diff line change
@@ -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> = T | null;

Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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];
},

Expand Down
Original file line number Diff line number Diff line change
@@ -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';

Expand Down Expand Up @@ -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'
);
}
}
);
}
2 changes: 0 additions & 2 deletions packages/@ember/-internals/runtime/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 0 additions & 1 deletion packages/@ember/-internals/runtime/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
9 changes: 2 additions & 7 deletions packages/@ember/-internals/runtime/lib/mixins/array.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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;

Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions packages/@ember/-internals/utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
7 changes: 7 additions & 0 deletions packages/@ember/-internals/utils/lib/ember-array.ts
Original file line number Diff line number Diff line change
@@ -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];
}
16 changes: 16 additions & 0 deletions packages/@ember/-internals/utils/tests/trackable-object-test.js
Original file line number Diff line number Diff line change
@@ -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);
}
}
);

0 comments on commit 8c4c3e0

Please sign in to comment.