From 0070773cf9c1cd1d885b8c9e3620e07d160a1246 Mon Sep 17 00:00:00 2001 From: Chris Garrett Date: Tue, 5 Feb 2019 08:59:02 -0800 Subject: [PATCH 1/4] [FEAT] Updates @tracked to match the Tracked Properties RFC Updates @tracked to be a classic decorator as well as a standard native decorator, removes the ability to decorator native getters, and adds more tests for various situations and updates the existing tests. This PR also exposes the native `descriptor` decorator for use defining getters and setters: ```js import EmberObject, { descriptor } from '@ember/object'; import { tracked } from '@glimmer/tracking'; const Person = EmberObject.extend({ firstName: tracked({ value: 'Tom' }), lastName: tracked({ value: 'Dale' }), fullName: descriptor({ get() { return `${this.firstName} ${this.lastName}`; }, }), }); ``` --- packages/@ember/-internals/metal/index.ts | 15 +- .../@ember/-internals/metal/lib/decorator.ts | 8 +- .../@ember/-internals/metal/lib/properties.ts | 10 +- .../@ember/-internals/metal/lib/tracked.ts | 206 +++++++++--------- .../metal/tests/accessors/set_test.js | 23 ++ .../tests/tracked/classic_classes_test.js | 177 +++++++++++++++ .../metal/tests/tracked/computed_test.ts | 69 ------ .../tracked/{get_test.ts => get_test.js} | 37 +--- .../tracked/{set_test.ts => set_test.js} | 13 +- .../-internals/metal/tests/tracked/support.js | 44 +--- ...{validation_test.ts => validation_test.js} | 155 +++++++++---- packages/ember/index.js | 2 + packages/ember/tests/reexports_test.js | 2 + 13 files changed, 478 insertions(+), 283 deletions(-) create mode 100644 packages/@ember/-internals/metal/tests/tracked/classic_classes_test.js delete mode 100644 packages/@ember/-internals/metal/tests/tracked/computed_test.ts rename packages/@ember/-internals/metal/tests/tracked/{get_test.ts => get_test.js} (71%) rename packages/@ember/-internals/metal/tests/tracked/{set_test.ts => set_test.js} (79%) rename packages/@ember/-internals/metal/tests/tracked/{validation_test.ts => validation_test.js} (58%) diff --git a/packages/@ember/-internals/metal/index.ts b/packages/@ember/-internals/metal/index.ts index 1b9e350a3f9..a5fbd03764c 100644 --- a/packages/@ember/-internals/metal/index.ts +++ b/packages/@ember/-internals/metal/index.ts @@ -49,7 +49,7 @@ export { Mixin, aliasMethod, mixin, observer, applyMixin } from './lib/mixin'; export { default as inject, DEBUG_INJECTION_FUNCTIONS } from './lib/injected_property'; export { setHasViews, tagForProperty, tagFor, markObjectAsDirty } from './lib/tags'; export { default as runInTransaction, didRender, assertNotRendered } from './lib/transaction'; -export { tracked } from './lib/tracked'; +export { tracked, getCurrentTracker, setCurrentTracker } from './lib/tracked'; export { NAMESPACES, @@ -64,3 +64,16 @@ export { isSearchDisabled as isNamespaceSearchDisabled, setSearchDisabled as setNamespaceSearchDisabled, } from './lib/namespace_search'; + +import { DEBUG } from '@glimmer/env'; +import { setComputedDecorator } from './lib/decorator'; +import { tracked } from './lib/tracked'; + +// We have to set this here because there is a cycle of dependencies in tracked +// which causes `setComputedDecorator` to not be resolved before the `tracked` +// module. +if (DEBUG) { + // Normally this isn't a classic decorator, but we want to throw a helpful + // error in development so we need it to treat it like one + setComputedDecorator(tracked); +} diff --git a/packages/@ember/-internals/metal/lib/decorator.ts b/packages/@ember/-internals/metal/lib/decorator.ts index 0a3608c4249..ae0d4a793e7 100644 --- a/packages/@ember/-internals/metal/lib/decorator.ts +++ b/packages/@ember/-internals/metal/lib/decorator.ts @@ -72,8 +72,8 @@ export function isComputedDecorator(dec: Decorator | null | undefined) { @param {function} decorator the value to mark as a decorator @private */ -export function setComputedDecorator(dec: Decorator) { - DECORATOR_DESCRIPTOR_MAP.set(dec, true); +export function setComputedDecorator(dec: Decorator, value: any = true) { + DECORATOR_DESCRIPTOR_MAP.set(dec, value); } // .......................................................... @@ -188,7 +188,7 @@ export function makeComputedDecorator( assert( 'Native decorators are not enabled without the EMBER_NATIVE_DECORATOR_SUPPORT flag', - EMBER_NATIVE_DECORATOR_SUPPORT ? !isClassicDecorator : isClassicDecorator + EMBER_NATIVE_DECORATOR_SUPPORT || isClassicDecorator ); elementDesc.kind = 'method'; @@ -208,7 +208,7 @@ export function makeComputedDecorator( return elementDesc; }; - DECORATOR_DESCRIPTOR_MAP.set(decorator, desc); + setComputedDecorator(decorator, desc); Object.setPrototypeOf(decorator, DecoratorClass.prototype); diff --git a/packages/@ember/-internals/metal/lib/properties.ts b/packages/@ember/-internals/metal/lib/properties.ts index c20febff765..540c5546452 100644 --- a/packages/@ember/-internals/metal/lib/properties.ts +++ b/packages/@ember/-internals/metal/lib/properties.ts @@ -3,7 +3,6 @@ */ import { Meta, meta as metaFor, peekMeta, UNDEFINED } from '@ember/-internals/meta'; -import { EMBER_NATIVE_DECORATOR_SUPPORT } from '@ember/canary-features'; import { assert } from '@ember/debug'; import { DEBUG } from '@glimmer/env'; import { @@ -155,16 +154,19 @@ export function defineProperty( let value; if (isComputedDecorator(desc)) { - let elementDesc: ElementDescriptor = { + let elementDesc = { key: keyName, kind: 'field', placement: 'own', descriptor: { value: undefined, }, - }; + toString() { + return '[object Descriptor]'; + }, + } as ElementDescriptor; - if (DEBUG && !EMBER_NATIVE_DECORATOR_SUPPORT) { + if (DEBUG) { elementDesc = desc!(elementDesc, true); } else { elementDesc = desc!(elementDesc); diff --git a/packages/@ember/-internals/metal/lib/tracked.ts b/packages/@ember/-internals/metal/lib/tracked.ts index 3d21d164eb1..797237435d1 100644 --- a/packages/@ember/-internals/metal/lib/tracked.ts +++ b/packages/@ember/-internals/metal/lib/tracked.ts @@ -1,14 +1,17 @@ +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 { dirty, tagFor, tagForProperty, update } from './tags'; +import { Decorator, ElementDescriptor, setComputedDecorator } from './decorator'; +import { dirty, tagFor, tagForProperty } from './tags'; type Option = T | null; -type unusable = null | undefined | void | {}; /** An object that that tracks @tracked properties that were consumed. @private - */ +*/ class Tracker { private tags = new Set(); private last: Option = null; @@ -95,134 +98,137 @@ class Tracker { ``` @param dependencies Optional dependents to be tracked. - */ -export function tracked(...dependencies: string[]): MethodDecorator; -export function tracked(target: unknown, key: PropertyKey): any; +*/ +export function tracked(propertyDesc: { value: any }): Decorator; +export function tracked(elementDesc: ElementDescriptor): ElementDescriptor; export function tracked( - target: unknown, - key: PropertyKey, - descriptor: PropertyDescriptor -): PropertyDescriptor; -export function tracked(...dependencies: any[]): any { - let [, key, descriptor] = dependencies; - - if (descriptor === undefined || 'initializer' in descriptor) { - return descriptorForDataProperty(key, descriptor); - } else { - return descriptorForAccessor(key, descriptor); - } -} + elementDesc: ElementDescriptor | any, + isClassicDecorator?: boolean +): ElementDescriptor | Decorator { + if ( + elementDesc === undefined || + elementDesc === null || + elementDesc.toString() !== '[object Descriptor]' + ) { + assert( + `tracked() may only receive an options object containing 'value' or 'initializer', received ${elementDesc}`, + elementDesc === undefined || (elementDesc !== null && typeof elementDesc === 'object') + ); -/** - @private + if (DEBUG && elementDesc) { + let keys = Object.keys(elementDesc); - Whenever a tracked computed property is entered, the current tracker is - saved off and a new tracker is replaced. + 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') + ); - Any tracked properties consumed are added to the current tracker. + assert( + `The initializer passed to tracked must be a function. Received ${elementDesc.initializer}`, + !('initializer' in elementDesc) || typeof elementDesc.initializer === 'function' + ); + } - When a tracked computed property is exited, the tracker's tags are - combined and added to the parent tracker. + let initializer = elementDesc ? elementDesc.initializer : undefined; + let value = elementDesc ? elementDesc.value : undefined; - The consequence is that each tracked computed property has a tag - that corresponds to the tracked properties consumed inside of - itself, including child tracked computed properties. - */ -let CURRENT_TRACKER: Option = null; + let decorator = function(elementDesc: ElementDescriptor, isClassicDecorator?: boolean) { + assert( + `You attempted to set a default value for ${ + elementDesc.key + } with the @tracked({ value: 'default' }) syntax. You can only use this syntax with classic classes. For native classes, you can use class initializers: @tracked field = 'default';`, + isClassicDecorator + ); -export function getCurrentTracker(): Option { - return CURRENT_TRACKER; -} + elementDesc.initializer = initializer || (() => value); -export function setCurrentTracker(tracker: Tracker = new Tracker()): Tracker { - return (CURRENT_TRACKER = tracker); -} + return descriptorForField(elementDesc); + }; -function descriptorForAccessor( - key: string | symbol, - descriptor: PropertyDescriptor -): PropertyDescriptor { - let get = descriptor.get as Function; - let set = descriptor.set as Function; + setComputedDecorator(decorator); - function getter(this: any): any { - // Swap the parent tracker for a new tracker - let old = CURRENT_TRACKER; - let tracker = (CURRENT_TRACKER = new Tracker()); + return decorator; + } - // Call the getter - let ret = get.call(this); + assert( + 'Native decorators are not enabled without the EMBER_NATIVE_DECORATOR_SUPPORT flag', + Boolean(EMBER_NATIVE_DECORATOR_SUPPORT) + ); - // Swap back the parent tracker - CURRENT_TRACKER = old; + assert( + `@tracked can only be used directly as a native decorator. If you're using tracked in classic classes, add parenthesis to call it like a function: tracked()`, + !isClassicDecorator + ); - // Combine the tags in the new tracker and add them to the parent tracker - let tag = tracker.combine(); - if (CURRENT_TRACKER) CURRENT_TRACKER.add(tag); + return descriptorForField(elementDesc); +} - // Update the UpdatableTag for this property with the tag for all of the - // consumed dependencies. - update(tagForProperty(this, key), tag); +function descriptorForField(elementDesc: ElementDescriptor): ElementDescriptor { + let { key, kind, initializer } = elementDesc as ElementDescriptor; - return ret; - } + assert( + `You attempted to use @tracked on ${key}, but that element is not a class field. @tracked is only usable on class fields. Native getters and setters will autotrack add any tracked fields they encounter, so there is no need mark getters and setters with @tracked.`, + kind === 'field' + ); - function setter(this: unusable): void { - dirty(tagForProperty(this, key)); - set.apply(this, arguments); - } + let shadowKey = Symbol(key); return { - enumerable: true, - configurable: false, - get: get && getter, - set: set && setter, + key, + kind: 'method', + placement: 'prototype', + descriptor: { + enumerable: true, + configurable: true, + + get(): any { + if (CURRENT_TRACKER) CURRENT_TRACKER.add(tagForProperty(this, key)); + + if (!(shadowKey in this)) { + this[shadowKey] = initializer !== undefined ? initializer.call(this) : undefined; + } + + return this[shadowKey]; + }, + + set(newValue: any): void { + tagFor(this).inner!['dirty'](); + dirty(tagForProperty(this, key)); + this[shadowKey] = newValue; + propertyDidChange(); + }, + }, }; } -export type Key = string; - /** @private - A getter/setter for change tracking for a particular key. The accessor - acts just like a normal property, but it triggers the `propertyDidChange` - hook when written to. - - Values are saved on the object using a "shadow key," or a symbol based on the - tracked property name. Sets write the value to the shadow key, and gets read - from it. - */ - -function descriptorForDataProperty( - key: string, - descriptor: PropertyDescriptor -): PropertyDescriptor { - let shadowKey = Symbol(key); + Whenever a tracked computed property is entered, the current tracker is + saved off and a new tracker is replaced. - return { - enumerable: true, - configurable: true, + Any tracked properties consumed are added to the current tracker. - get(): any { - if (CURRENT_TRACKER) CURRENT_TRACKER.add(tagForProperty(this, key)); + When a tracked computed property is exited, the tracker's tags are + combined and added to the parent tracker. - if (!(shadowKey in this)) { - this[shadowKey] = descriptor.value; - } + The consequence is that each tracked computed property has a tag + that corresponds to the tracked properties consumed inside of + itself, including child tracked computed properties. +*/ +let CURRENT_TRACKER: Option = null; - return this[shadowKey]; - }, +export function getCurrentTracker(): Option { + return CURRENT_TRACKER; +} - set(newValue: any): void { - tagFor(this).inner!['dirty'](); - dirty(tagForProperty(this, key)); - this[shadowKey] = newValue; - propertyDidChange(); - }, - }; +export function setCurrentTracker(tracker: Tracker = new Tracker()): Tracker { + return (CURRENT_TRACKER = tracker); } +export type Key = string; + export interface Interceptors { [key: string]: boolean; } diff --git a/packages/@ember/-internals/metal/tests/accessors/set_test.js b/packages/@ember/-internals/metal/tests/accessors/set_test.js index d159de71c6c..b02020f1893 100644 --- a/packages/@ember/-internals/metal/tests/accessors/set_test.js +++ b/packages/@ember/-internals/metal/tests/accessors/set_test.js @@ -124,5 +124,28 @@ moduleFor( trySet(obj, 'favoriteFood', 'hot dogs'); assert.equal(obj.favoriteFood, undefined, 'does not set and does not error'); } + + ['@test should work with native setters'](assert) { + let count = 0; + + class Foo { + __foo = ''; + + get foo() { + return this.__foo; + } + + set foo(value) { + count++; + this.__foo = `computed ${value}`; + } + } + + let obj = new Foo(); + + assert.equal(set(obj, 'foo', 'bar'), 'bar', 'should return set value'); + assert.equal(count, 1, 'should have native setter'); + assert.equal(get(obj, 'foo'), 'computed bar', 'should return new value'); + } } ); diff --git a/packages/@ember/-internals/metal/tests/tracked/classic_classes_test.js b/packages/@ember/-internals/metal/tests/tracked/classic_classes_test.js new file mode 100644 index 00000000000..e4c0484b232 --- /dev/null +++ b/packages/@ember/-internals/metal/tests/tracked/classic_classes_test.js @@ -0,0 +1,177 @@ +import { AbstractTestCase, moduleFor } from 'internal-test-helpers'; +import { defineProperty, tracked, nativeDescDecorator } from '../..'; + +import { track } from './support'; + +import { + EMBER_METAL_TRACKED_PROPERTIES, + EMBER_NATIVE_DECORATOR_SUPPORT, +} from '@ember/canary-features'; + +if (EMBER_METAL_TRACKED_PROPERTIES) { + moduleFor( + '@tracked decorator - classic classes', + class extends AbstractTestCase { + [`@test validators for tracked getters with dependencies should invalidate when the dependencies invalidate`]( + assert + ) { + let obj = {}; + + defineProperty(obj, 'first', tracked()); + defineProperty(obj, 'last', tracked()); + + defineProperty( + obj, + 'full', + nativeDescDecorator({ + get() { + return `${this.first} ${this.last}`; + }, + + set(value) { + let [first, last] = value.split(' '); + + this.first = first; + this.last = last; + }, + }) + ); + + obj.first = 'Tom'; + obj.last = 'Dale'; + + let tag = track(() => obj.full); + let snapshot = tag.value(); + + assert.equal(obj.full, 'Tom Dale', 'The full name starts correct'); + assert.equal(tag.validate(snapshot), true); + + snapshot = tag.value(); + assert.equal(tag.validate(snapshot), true); + + obj.full = 'Melanie Sumner'; + + assert.equal(tag.validate(snapshot), false); + + assert.equal(obj.full, 'Melanie Sumner'); + assert.equal(obj.first, 'Melanie'); + assert.equal(obj.last, 'Sumner'); + snapshot = tag.value(); + + assert.equal(tag.validate(snapshot), true); + } + + [`@test can pass a default value to the tracked decorator`](assert) { + class Tracked { + get full() { + return `${this.first} ${this.last}`; + } + } + + defineProperty(Tracked.prototype, 'first', tracked({ value: 'Tom' })); + defineProperty(Tracked.prototype, 'last', tracked({ value: 'Dale' })); + + let obj = new Tracked(); + + assert.equal(obj.full, 'Tom Dale', 'Default values are correctly assign'); + } + + [`@test errors if used directly on a classic class`]() { + expectAssertion(() => { + class Tracked { + get full() { + return `${this.first} ${this.last}`; + } + } + + defineProperty(Tracked.prototype, 'first', tracked); + }, "@tracked can only be used directly as a native decorator. If you're using tracked in classic classes, add parenthesis to call it like a function: tracked()"); + } + + [`@test errors on any keys besides 'value', 'get', or 'set' being passed`]() { + expectAssertion(() => { + class Tracked { + get full() { + return `${this.first} ${this.last}`; + } + } + + defineProperty( + Tracked.prototype, + 'first', + tracked({ + foo() {}, + }) + ); + }, "The options object passed to tracked() may only contain a 'value' or 'initializer' property, not both. Received: [foo]"); + } + + [`@test errors if 'value' and 'get'/'set' are passed together`]() { + expectAssertion(() => { + class Tracked { + get full() { + return `${this.first} ${this.last}`; + } + } + + defineProperty( + Tracked.prototype, + 'first', + tracked({ + value: 123, + initializer: () => 123, + }) + ); + }, "The options object passed to tracked() may only contain a 'value' or 'initializer' property, not both. Received: [value,initializer]"); + } + + [`@test errors on anything besides an options object being passed`]() { + expectAssertion(() => { + class Tracked { + get full() { + return `${this.first} ${this.last}`; + } + } + + defineProperty(Tracked.prototype, 'first', tracked(null)); + }, "tracked() may only receive an options object containing 'value' or 'initializer', received null"); + } + } + ); + + if (EMBER_NATIVE_DECORATOR_SUPPORT) { + moduleFor( + '@tracked decorator - native decorator behavior', + class extends AbstractTestCase { + [`@test errors if options are passed to native decorator`]() { + expectAssertion(() => { + class Tracked { + @tracked() first; + + get full() { + return `${this.first} ${this.last}`; + } + } + + new Tracked(); + }, "You attempted to set a default value for first with the @tracked({ value: 'default' }) syntax. You can only use this syntax with classic classes. For native classes, you can use class initializers: @tracked field = 'default';"); + } + } + ); + } else { + moduleFor( + '@tracked decorator - native decorator behavior', + class extends AbstractTestCase { + [`@test errors if used as a native decorator`]() { + expectAssertion(() => { + class Tracked { + @tracked first; + } + + new Tracked(); + }, 'Native decorators are not enabled without the EMBER_NATIVE_DECORATOR_SUPPORT flag'); + } + } + ); + } +} diff --git a/packages/@ember/-internals/metal/tests/tracked/computed_test.ts b/packages/@ember/-internals/metal/tests/tracked/computed_test.ts deleted file mode 100644 index 88172798da0..00000000000 --- a/packages/@ember/-internals/metal/tests/tracked/computed_test.ts +++ /dev/null @@ -1,69 +0,0 @@ -import { get, set, tracked } from '../..'; - -import { EMBER_METAL_TRACKED_PROPERTIES } from '@ember/canary-features'; -import { AbstractTestCase, moduleFor } from 'internal-test-helpers'; - -if (EMBER_METAL_TRACKED_PROPERTIES) { - moduleFor( - '@tracked getters', - class extends AbstractTestCase { - ['@test works without get'](assert: Assert) { - let count = 0; - - class Count { - @tracked - get foo() { - count++; - return `computed foo`; - } - } - - let obj = new Count(); - - assert.equal(obj.foo, 'computed foo', 'should return value'); - assert.equal(count, 1, 'should have invoked computed property'); - } - - ['@test defining computed property should invoke property on get'](assert: Assert) { - let count = 0; - - class Count { - @tracked - get foo() { - count++; - return `computed foo`; - } - } - - let obj = new Count(); - - assert.equal(get(obj, 'foo'), 'computed foo', 'should return value'); - assert.equal(count, 1, 'should have invoked computed property'); - } - - ['@test defining computed property should invoke property on set'](assert: Assert) { - let count = 0; - - class Foo { - __foo = ''; - - @tracked - get foo() { - return this.__foo; - } - - set foo(value) { - count++; - this.__foo = `computed ${value}`; - } - } - - let obj = new Foo(); - - assert.equal(set(obj, 'foo', 'bar'), 'bar', 'should return set value'); - assert.equal(count, 1, 'should have invoked computed property'); - assert.equal(get(obj, 'foo'), 'computed bar', 'should return new value'); - } - } - ); -} diff --git a/packages/@ember/-internals/metal/tests/tracked/get_test.ts b/packages/@ember/-internals/metal/tests/tracked/get_test.js similarity index 71% rename from packages/@ember/-internals/metal/tests/tracked/get_test.ts rename to packages/@ember/-internals/metal/tests/tracked/get_test.js index 250f6dc9c75..5f06f9ad7cd 100644 --- a/packages/@ember/-internals/metal/tests/tracked/get_test.ts +++ b/packages/@ember/-internals/metal/tests/tracked/get_test.js @@ -1,9 +1,12 @@ -import { EMBER_METAL_TRACKED_PROPERTIES } from '@ember/canary-features'; +import { + EMBER_METAL_TRACKED_PROPERTIES, + EMBER_NATIVE_DECORATOR_SUPPORT, +} from '@ember/canary-features'; import { AbstractTestCase, moduleFor } from 'internal-test-helpers'; import { get, getWithDefault, tracked } from '../..'; -if (EMBER_METAL_TRACKED_PROPERTIES) { - const createObj = function() { +if (EMBER_METAL_TRACKED_PROPERTIES && EMBER_NATIVE_DECORATOR_SUPPORT) { + let createObj = function() { class Obj { @tracked string = 'string'; @tracked number = 23; @@ -64,37 +67,15 @@ if (EMBER_METAL_TRACKED_PROPERTIES) { this.assert.equal(get(obj, 'path.key.value'), 'value'); } - '@test should not access a property more than once'() { - let count = 20; - - class Count { - @tracked - get id() { - return ++count; - } - } - - let obj = new Count(); - - get(obj, 'id'); - - this.assert.equal(count, 21); - } - } - ); - - moduleFor( - '@tracked decorator: getWithDefault', - class extends AbstractTestCase { ['@test should get arbitrary properties on an object']() { let obj = createObj(); for (let key in obj) { - this.assert.equal(getWithDefault(obj, key as any, 'fail'), obj[key], key); + this.assert.equal(getWithDefault(obj, key, 'fail'), obj[key], key); } class Obj { - @tracked undef: string | undefined = undefined; + @tracked undef = undefined; } let obj2 = new Obj(); @@ -105,7 +86,7 @@ if (EMBER_METAL_TRACKED_PROPERTIES) { 'explicit undefined retrieves the default' ); this.assert.equal( - getWithDefault(obj2, 'not-present' as any, 'default'), + getWithDefault(obj2, 'not-present', 'default'), 'default', 'non-present key retrieves the default' ); diff --git a/packages/@ember/-internals/metal/tests/tracked/set_test.ts b/packages/@ember/-internals/metal/tests/tracked/set_test.js similarity index 79% rename from packages/@ember/-internals/metal/tests/tracked/set_test.ts rename to packages/@ember/-internals/metal/tests/tracked/set_test.js index 4458c005595..9ee38d7d51b 100644 --- a/packages/@ember/-internals/metal/tests/tracked/set_test.ts +++ b/packages/@ember/-internals/metal/tests/tracked/set_test.js @@ -1,10 +1,13 @@ import { AbstractTestCase, moduleFor } from 'internal-test-helpers'; import { get, set, tracked } from '../..'; -import { EMBER_METAL_TRACKED_PROPERTIES } from '@ember/canary-features'; +import { + EMBER_METAL_TRACKED_PROPERTIES, + EMBER_NATIVE_DECORATOR_SUPPORT, +} from '@ember/canary-features'; -if (EMBER_METAL_TRACKED_PROPERTIES) { - const createObj = () => { +if (EMBER_METAL_TRACKED_PROPERTIES && EMBER_NATIVE_DECORATOR_SUPPORT) { + let createObj = () => { class Obj { @tracked string = 'string'; @tracked number = 23; @@ -20,7 +23,7 @@ if (EMBER_METAL_TRACKED_PROPERTIES) { moduleFor( '@tracked set', class extends AbstractTestCase { - ['@test should set arbitrary properties on an object'](assert: Assert) { + ['@test should set arbitrary properties on an object'](assert) { let obj = createObj(); class Obj { @@ -35,7 +38,7 @@ if (EMBER_METAL_TRACKED_PROPERTIES) { } } - ['@test should set a number key on an object'](assert: Assert) { + ['@test should set a number key on an object'](assert) { class Obj { @tracked 1 = 'original'; } diff --git a/packages/@ember/-internals/metal/tests/tracked/support.js b/packages/@ember/-internals/metal/tests/tracked/support.js index 6d4bc761a78..e58a979df9f 100644 --- a/packages/@ember/-internals/metal/tests/tracked/support.js +++ b/packages/@ember/-internals/metal/tests/tracked/support.js @@ -1,37 +1,17 @@ -import { tracked } from '../..'; +import { getCurrentTracker, setCurrentTracker } from '../..'; -export function createTracked(values, proto = {}) { - function Class() { - for (let prop in values) { - this[prop] = values[prop]; - } - } +/** + Creates an autotrack stack so we can test field changes as they flow through + getters/setters, and through the system overall - for (let prop in values) { - Object.defineProperty( - proto, - prop, - tracked(proto, prop, { - enumerable: true, - configurable: true, - writable: true, - value: values[prop], - }) - ); - } + @private +*/ +export function track(fn) { + let parent = getCurrentTracker(); + let tracker = setCurrentTracker(); - Class.prototype = proto; + fn(); - return new Class(); -} - -export function createWithDescriptors(values) { - function Class() {} - - for (let prop in values) { - let descriptor = Object.getOwnPropertyDescriptor(values, prop); - Object.defineProperty(Class.prototype, prop, tracked(Class.prototype, prop, descriptor)); - } - - return new Class(); + setCurrentTracker(parent); + return tracker.combine(); } diff --git a/packages/@ember/-internals/metal/tests/tracked/validation_test.ts b/packages/@ember/-internals/metal/tests/tracked/validation_test.js similarity index 58% rename from packages/@ember/-internals/metal/tests/tracked/validation_test.ts rename to packages/@ember/-internals/metal/tests/tracked/validation_test.js index b8380f93b75..3fc071dcd22 100644 --- a/packages/@ember/-internals/metal/tests/tracked/validation_test.ts +++ b/packages/@ember/-internals/metal/tests/tracked/validation_test.js @@ -1,24 +1,54 @@ import { computed, defineProperty, get, set, tagForProperty, tracked } from '../..'; -import { EMBER_METAL_TRACKED_PROPERTIES } from '@ember/canary-features'; +import { + EMBER_METAL_TRACKED_PROPERTIES, + EMBER_NATIVE_DECORATOR_SUPPORT, +} from '@ember/canary-features'; import { AbstractTestCase, moduleFor } from 'internal-test-helpers'; +import { track } from './support'; -if (EMBER_METAL_TRACKED_PROPERTIES) { +if (EMBER_METAL_TRACKED_PROPERTIES && EMBER_NATIVE_DECORATOR_SUPPORT) { moduleFor( '@tracked get validation', class extends AbstractTestCase { - [`@test validators for tracked getters with dependencies should invalidate when the dependencies invalidate`]( - assert: Assert - ) { + [`@test autotracking should work with tracked fields`](assert) { + class Tracked { + @tracked first = undefined; + constructor(first) { + this.first = first; + } + } + + let obj = new Tracked('Tom', 'Dale'); + + let tag = track(() => obj.first); + let snapshot = tag.value(); + + assert.equal(obj.first, 'Tom', 'The full name starts correct'); + assert.equal(tag.validate(snapshot), true); + + snapshot = tag.value(); + assert.equal(tag.validate(snapshot), true); + + obj.first = 'Thomas'; + + assert.equal(tag.validate(snapshot), false); + + assert.equal(obj.first, 'Thomas'); + snapshot = tag.value(); + + assert.equal(tag.validate(snapshot), true); + } + + [`@test autotracking should work with native getters`](assert) { class Tracked { - @tracked first?: string = undefined; - @tracked last?: string = undefined; - constructor(first: string, last: string) { + @tracked first = undefined; + @tracked last = undefined; + constructor(first, last) { this.first = first; this.last = last; } - @tracked get full() { return `${this.first} ${this.last}`; } @@ -26,11 +56,10 @@ if (EMBER_METAL_TRACKED_PROPERTIES) { let obj = new Tracked('Tom', 'Dale'); - let tag = tagForProperty(obj, 'full'); + let tag = track(() => obj.full); let snapshot = tag.value(); - let full = obj.full; - assert.equal(full, 'Tom Dale', 'The full name starts correct'); + assert.equal(obj.full, 'Tom Dale', 'The full name starts correct'); assert.equal(tag.validate(snapshot), true); snapshot = tag.value(); @@ -46,22 +75,60 @@ if (EMBER_METAL_TRACKED_PROPERTIES) { assert.equal(tag.validate(snapshot), true); } + [`@test autotracking should work with native setters`](assert) { + class Tracked { + @tracked first = undefined; + @tracked last = undefined; + constructor(first, last) { + this.first = first; + this.last = last; + } + + get full() { + return `${this.first} ${this.last}`; + } + + set full(value) { + let [first, last] = value.split(' '); + + this.first = first; + this.last = last; + } + } + + let obj = new Tracked('Tom', 'Dale'); + + let tag = track(() => obj.full); + let snapshot = tag.value(); + + assert.equal(obj.full, 'Tom Dale', 'The full name starts correct'); + assert.equal(tag.validate(snapshot), true); + + snapshot = tag.value(); + assert.equal(tag.validate(snapshot), true); + + obj.full = 'Melanie Sumner'; + + assert.equal(tag.validate(snapshot), false); + + assert.equal(obj.full, 'Melanie Sumner'); + assert.equal(obj.first, 'Melanie'); + assert.equal(obj.last, 'Sumner'); + snapshot = tag.value(); + + assert.equal(tag.validate(snapshot), true); + } + [`@test interaction with Ember object model (tracked property depending on Ember property)`]( - assert: Assert + assert ) { - interface NameInterface { - first: string; - last: string; - } class Tracked { - @tracked name: NameInterface; - constructor(name: NameInterface) { + constructor(name) { this.name = name; } - @tracked get full() { - return `${get(this.name, 'first')} ${get(this.name, 'last')}`; + return `${get(this, 'name.first')} ${get(this, 'name.last')}`; } } @@ -69,11 +136,10 @@ if (EMBER_METAL_TRACKED_PROPERTIES) { let obj = new Tracked(tom); - let tag = tagForProperty(obj, 'full'); + let tag = track(() => obj.full); let snapshot = tag.value(); - let full = obj.full; - assert.equal(full, 'Tom Dale'); + assert.equal(obj.full, 'Tom Dale'); assert.equal(tag.validate(snapshot), true); snapshot = tag.value(); @@ -86,14 +152,22 @@ if (EMBER_METAL_TRACKED_PROPERTIES) { snapshot = tag.value(); assert.equal(tag.validate(snapshot), true); + + set(obj, 'name', { first: 'Ricardo', last: 'Mendes' }); + + assert.equal(tag.validate(snapshot), false, 'invalid after setting with Ember set'); + + assert.equal(obj.full, 'Ricardo Mendes'); + snapshot = tag.value(); + + assert.equal(tag.validate(snapshot), true); } [`@test interaction with Ember object model (Ember computed property depending on tracked property)`]( - assert: Assert + assert ) { class EmberObject { - name: Name; - constructor(name: Name) { + constructor(name) { this.name = name; } } @@ -101,16 +175,17 @@ if (EMBER_METAL_TRACKED_PROPERTIES) { defineProperty( EmberObject.prototype, 'full', - computed('name', function(this: EmberObject) { + computed('name', function() { let name = get(this, 'name'); return `${name.first} ${name.last}`; }) ); class Name { - @tracked first: string; - @tracked last: string; - constructor(first: string, last: string) { + @tracked first; + @tracked last; + + constructor(first, last) { this.first = first; this.last = last; } @@ -143,12 +218,12 @@ if (EMBER_METAL_TRACKED_PROPERTIES) { } ['@test interaction with the Ember object model (paths going through tracked properties)']( - assert: Assert + assert ) { - let self: EmberObject; + let self; class EmberObject { - contact: Contact; - constructor(contact: Contact) { + contact; + constructor(contact) { this.contact = contact; self = this; } @@ -164,16 +239,16 @@ if (EMBER_METAL_TRACKED_PROPERTIES) { ); class Contact { - @tracked name?: EmberName = undefined; - constructor(name: EmberName) { + @tracked name = undefined; + constructor(name) { this.name = name; } } class EmberName { - first: string; - last: string; - constructor(first: string, last: string) { + first; + last; + constructor(first, last) { this.first = first; this.last = last; } diff --git a/packages/ember/index.js b/packages/ember/index.js index 34a22d63a8f..b186fe246b9 100644 --- a/packages/ember/index.js +++ b/packages/ember/index.js @@ -272,6 +272,8 @@ Object.defineProperty(Ember.run, 'currentRunLoop', { // in globals builds const computed = metal._globalsComputed; Ember.computed = computed; +Ember._descriptor = metal.nativeDescDecorator; +Ember._tracked = metal.tracked; computed.alias = metal.alias; Ember.cacheFor = metal.getCachedValueFor; Ember.ComputedProperty = metal.ComputedProperty; diff --git a/packages/ember/tests/reexports_test.js b/packages/ember/tests/reexports_test.js index 1e8b6a5acaf..882914b7167 100644 --- a/packages/ember/tests/reexports_test.js +++ b/packages/ember/tests/reexports_test.js @@ -109,6 +109,8 @@ let allExports = [ // @ember/-internals/metal ['computed', '@ember/-internals/metal', '_globalsComputed'], + ['_descriptor', '@ember/-internals/metal', 'nativeDescDecorator'], + ['_tracked', '@ember/-internals/metal', 'tracked'], ['computed.alias', '@ember/-internals/metal', 'alias'], ['ComputedProperty', '@ember/-internals/metal'], ['_setComputedDecorator', '@ember/-internals/metal', 'setComputedDecorator'], From b7c7eaf792545c2dcbc87b8ba7453d48f32f840f Mon Sep 17 00:00:00 2001 From: Chris Garrett Date: Fri, 8 Feb 2019 15:52:37 -0800 Subject: [PATCH 2/4] remove cycle, switch from symbol to weakmap --- packages/@ember/-internals/metal/index.ts | 17 +----- packages/@ember/-internals/metal/lib/alias.ts | 2 +- .../@ember/-internals/metal/lib/chains.ts | 2 +- .../@ember/-internals/metal/lib/computed.ts | 2 +- .../@ember/-internals/metal/lib/decorator.ts | 60 +------------------ .../-internals/metal/lib/descriptor_map.ts | 58 ++++++++++++++++++ packages/@ember/-internals/metal/lib/mixin.ts | 4 +- .../@ember/-internals/metal/lib/properties.ts | 8 +-- .../-internals/metal/lib/property_events.ts | 2 +- .../-internals/metal/lib/property_get.ts | 2 +- .../-internals/metal/lib/property_set.ts | 2 +- .../@ember/-internals/metal/lib/tracked.ts | 48 ++++++++++++--- .../@ember/-internals/metal/lib/watch_key.ts | 2 +- 13 files changed, 112 insertions(+), 97 deletions(-) create mode 100644 packages/@ember/-internals/metal/lib/descriptor_map.ts diff --git a/packages/@ember/-internals/metal/index.ts b/packages/@ember/-internals/metal/index.ts index a5fbd03764c..145300fe99c 100644 --- a/packages/@ember/-internals/metal/index.ts +++ b/packages/@ember/-internals/metal/index.ts @@ -29,12 +29,12 @@ export { PROPERTY_DID_CHANGE, } from './lib/property_events'; export { defineProperty } from './lib/properties'; +export { nativeDescDecorator } from './lib/decorator'; export { descriptorForProperty, isComputedDecorator, setComputedDecorator, - nativeDescDecorator, -} from './lib/decorator'; +} from './lib/descriptor_map'; export { watchKey, unwatchKey } from './lib/watch_key'; export { ChainNode, finishChains, removeChainWatcher } from './lib/chains'; export { watchPath, unwatchPath } from './lib/watch_path'; @@ -64,16 +64,3 @@ export { isSearchDisabled as isNamespaceSearchDisabled, setSearchDisabled as setNamespaceSearchDisabled, } from './lib/namespace_search'; - -import { DEBUG } from '@glimmer/env'; -import { setComputedDecorator } from './lib/decorator'; -import { tracked } from './lib/tracked'; - -// We have to set this here because there is a cycle of dependencies in tracked -// which causes `setComputedDecorator` to not be resolved before the `tracked` -// module. -if (DEBUG) { - // Normally this isn't a classic decorator, but we want to throw a helpful - // error in development so we need it to treat it like one - setComputedDecorator(tracked); -} diff --git a/packages/@ember/-internals/metal/lib/alias.ts b/packages/@ember/-internals/metal/lib/alias.ts index 0cbea3a9efa..a985f8c5429 100644 --- a/packages/@ember/-internals/metal/lib/alias.ts +++ b/packages/@ember/-internals/metal/lib/alias.ts @@ -7,10 +7,10 @@ import { addDependentKeys, ComputedDescriptor, Decorator, - descriptorForDecorator, makeComputedDecorator, removeDependentKeys, } from './decorator'; +import { descriptorForDecorator } from './descriptor_map'; import { defineProperty } from './properties'; import { get } from './property_get'; import { set } from './property_set'; diff --git a/packages/@ember/-internals/metal/lib/chains.ts b/packages/@ember/-internals/metal/lib/chains.ts index 3dbb5b74872..feb1cc5ae92 100644 --- a/packages/@ember/-internals/metal/lib/chains.ts +++ b/packages/@ember/-internals/metal/lib/chains.ts @@ -1,6 +1,6 @@ import { Meta, meta as metaFor, peekMeta } from '@ember/-internals/meta'; import { getCachedValueFor } from './computed_cache'; -import { descriptorForProperty } from './decorator'; +import { descriptorForProperty } from './descriptor_map'; import { eachProxyFor } from './each_proxy'; import { get } from './property_get'; import { unwatchKey, watchKey } from './watch_key'; diff --git a/packages/@ember/-internals/metal/lib/computed.ts b/packages/@ember/-internals/metal/lib/computed.ts index 433ef371471..642841588df 100644 --- a/packages/@ember/-internals/metal/lib/computed.ts +++ b/packages/@ember/-internals/metal/lib/computed.ts @@ -14,10 +14,10 @@ import { addDependentKeys, ComputedDescriptor, Decorator, - descriptorForDecorator, makeComputedDecorator, removeDependentKeys, } from './decorator'; +import { descriptorForDecorator } from './descriptor_map'; import expandProperties from './expand_properties'; import { defineProperty } from './properties'; import { notifyPropertyChange } from './property_events'; diff --git a/packages/@ember/-internals/metal/lib/decorator.ts b/packages/@ember/-internals/metal/lib/decorator.ts index ae0d4a793e7..4a80d704523 100644 --- a/packages/@ember/-internals/metal/lib/decorator.ts +++ b/packages/@ember/-internals/metal/lib/decorator.ts @@ -1,11 +1,10 @@ -import { Meta, meta as metaFor, peekMeta } from '@ember/-internals/meta'; +import { Meta, meta as metaFor } from '@ember/-internals/meta'; import { EMBER_NATIVE_DECORATOR_SUPPORT } from '@ember/canary-features'; import { assert } from '@ember/debug'; import { DEBUG } from '@glimmer/env'; +import { setComputedDecorator } from './descriptor_map'; import { unwatch, watch } from './watching'; -const DECORATOR_DESCRIPTOR_MAP: WeakMap = new WeakMap(); - // https://tc39.github.io/proposal-decorators/#sec-elementdescriptor-specification-type export interface ElementDescriptor { descriptor: PropertyDescriptor & { initializer?: any }; @@ -21,61 +20,6 @@ export type Decorator = ( isClassicDecorator?: boolean ) => ElementDescriptor; -// .......................................................... -// DESCRIPTOR -// - -/** - Returns the CP descriptor assocaited with `obj` and `keyName`, if any. - - @method descriptorFor - @param {Object} obj the object to check - @param {String} keyName the key to check - @return {Descriptor} - @private -*/ -export function descriptorForProperty(obj: object, keyName: string, _meta?: Meta | null) { - assert('Cannot call `descriptorFor` on null', obj !== null); - assert('Cannot call `descriptorFor` on undefined', obj !== undefined); - assert( - `Cannot call \`descriptorFor\` on ${typeof obj}`, - typeof obj === 'object' || typeof obj === 'function' - ); - - let meta = _meta === undefined ? peekMeta(obj) : _meta; - - if (meta !== null) { - return meta.peekDescriptors(keyName); - } -} - -export function descriptorForDecorator(dec: Decorator) { - return DECORATOR_DESCRIPTOR_MAP.get(dec); -} - -/** - Check whether a value is a decorator - - @method isComputedDecorator - @param {any} possibleDesc the value to check - @return {boolean} - @private -*/ -export function isComputedDecorator(dec: Decorator | null | undefined) { - return dec !== null && dec !== undefined && DECORATOR_DESCRIPTOR_MAP.has(dec); -} - -/** - Set a value as a decorator - - @method setComputedDecorator - @param {function} decorator the value to mark as a decorator - @private -*/ -export function setComputedDecorator(dec: Decorator, value: any = true) { - DECORATOR_DESCRIPTOR_MAP.set(dec, value); -} - // .......................................................... // DEPENDENT KEYS // diff --git a/packages/@ember/-internals/metal/lib/descriptor_map.ts b/packages/@ember/-internals/metal/lib/descriptor_map.ts new file mode 100644 index 00000000000..91fbc94a717 --- /dev/null +++ b/packages/@ember/-internals/metal/lib/descriptor_map.ts @@ -0,0 +1,58 @@ +import { Meta, peekMeta } from '@ember/-internals/meta'; +import { assert } from '@ember/debug'; + +const DECORATOR_DESCRIPTOR_MAP: WeakMap< + import('./decorator').Decorator, + import('./decorator').ComputedDescriptor | boolean +> = new WeakMap(); + +/** + Returns the CP descriptor assocaited with `obj` and `keyName`, if any. + + @method descriptorFor + @param {Object} obj the object to check + @param {String} keyName the key to check + @return {Descriptor} + @private +*/ +export function descriptorForProperty(obj: object, keyName: string, _meta?: Meta | null) { + assert('Cannot call `descriptorFor` on null', obj !== null); + assert('Cannot call `descriptorFor` on undefined', obj !== undefined); + assert( + `Cannot call \`descriptorFor\` on ${typeof obj}`, + typeof obj === 'object' || typeof obj === 'function' + ); + + let meta = _meta === undefined ? peekMeta(obj) : _meta; + + if (meta !== null) { + return meta.peekDescriptors(keyName); + } +} + +export function descriptorForDecorator(dec: import('./decorator').Decorator) { + return DECORATOR_DESCRIPTOR_MAP.get(dec); +} + +/** + Check whether a value is a decorator + + @method isComputedDecorator + @param {any} possibleDesc the value to check + @return {boolean} + @private +*/ +export function isComputedDecorator(dec: import('./decorator').Decorator | null | undefined) { + return dec !== null && dec !== undefined && DECORATOR_DESCRIPTOR_MAP.has(dec); +} + +/** + Set a value as a decorator + + @method setComputedDecorator + @param {function} decorator the value to mark as a decorator + @private +*/ +export function setComputedDecorator(dec: import('./decorator').Decorator, value: any = true) { + DECORATOR_DESCRIPTOR_MAP.set(dec, value); +} diff --git a/packages/@ember/-internals/metal/lib/mixin.ts b/packages/@ember/-internals/metal/lib/mixin.ts index df7837b8512..8d1326c0e1c 100644 --- a/packages/@ember/-internals/metal/lib/mixin.ts +++ b/packages/@ember/-internals/metal/lib/mixin.ts @@ -22,12 +22,12 @@ import { ComputedPropertyGetter, ComputedPropertySetter, } from './computed'; +import { makeComputedDecorator } from './decorator'; import { descriptorForDecorator, descriptorForProperty, isComputedDecorator, - makeComputedDecorator, -} from './decorator'; +} from './descriptor_map'; import { addListener, removeListener } from './events'; import expandProperties from './expand_properties'; import { classToString, setUnprocessedMixins } from './namespace_search'; diff --git a/packages/@ember/-internals/metal/lib/properties.ts b/packages/@ember/-internals/metal/lib/properties.ts index 540c5546452..298d004cc1f 100644 --- a/packages/@ember/-internals/metal/lib/properties.ts +++ b/packages/@ember/-internals/metal/lib/properties.ts @@ -5,12 +5,8 @@ import { Meta, meta as metaFor, peekMeta, UNDEFINED } from '@ember/-internals/meta'; import { assert } from '@ember/debug'; import { DEBUG } from '@glimmer/env'; -import { - Decorator, - descriptorForProperty, - ElementDescriptor, - isComputedDecorator, -} from './decorator'; +import { Decorator, ElementDescriptor } from './decorator'; +import { descriptorForProperty, isComputedDecorator } from './descriptor_map'; import { overrideChains } from './property_events'; export type MandatorySetterFunction = ((this: object, value: any) => void) & { diff --git a/packages/@ember/-internals/metal/lib/property_events.ts b/packages/@ember/-internals/metal/lib/property_events.ts index da9ebf2013e..985e7c54b8b 100644 --- a/packages/@ember/-internals/metal/lib/property_events.ts +++ b/packages/@ember/-internals/metal/lib/property_events.ts @@ -2,7 +2,7 @@ import { Meta, peekMeta } from '@ember/-internals/meta'; import { symbol } from '@ember/-internals/utils'; import { DEBUG } from '@glimmer/env'; import changeEvent from './change_event'; -import { descriptorForProperty } from './decorator'; +import { descriptorForProperty } from './descriptor_map'; import { sendEvent } from './events'; import ObserverSet from './observer_set'; import { markObjectAsDirty } from './tags'; diff --git a/packages/@ember/-internals/metal/lib/property_get.ts b/packages/@ember/-internals/metal/lib/property_get.ts index 89290818fcc..a86567d06d2 100644 --- a/packages/@ember/-internals/metal/lib/property_get.ts +++ b/packages/@ember/-internals/metal/lib/property_get.ts @@ -5,7 +5,7 @@ import { HAS_NATIVE_PROXY, 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 './decorator'; +import { descriptorForProperty } from './descriptor_map'; import { isPath } from './path_cache'; import { tagForProperty } from './tags'; import { getCurrentTracker } from './tracked'; diff --git a/packages/@ember/-internals/metal/lib/property_set.ts b/packages/@ember/-internals/metal/lib/property_set.ts index 77f9c585d0c..2e3d6a420aa 100644 --- a/packages/@ember/-internals/metal/lib/property_set.ts +++ b/packages/@ember/-internals/metal/lib/property_set.ts @@ -3,7 +3,7 @@ import { HAS_NATIVE_PROXY, lookupDescriptor, toString } from '@ember/-internals/ import { assert } from '@ember/debug'; import EmberError from '@ember/error'; import { DEBUG } from '@glimmer/env'; -import { descriptorForProperty } from './decorator'; +import { descriptorForProperty } from './descriptor_map'; import { isPath } from './path_cache'; import { MandatorySetterFunction } from './properties'; import { notifyPropertyChange } from './property_events'; diff --git a/packages/@ember/-internals/metal/lib/tracked.ts b/packages/@ember/-internals/metal/lib/tracked.ts index 797237435d1..3cba94fae4b 100644 --- a/packages/@ember/-internals/metal/lib/tracked.ts +++ b/packages/@ember/-internals/metal/lib/tracked.ts @@ -2,7 +2,8 @@ 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, ElementDescriptor, setComputedDecorator } from './decorator'; +import { Decorator, ElementDescriptor } from './decorator'; +import { setComputedDecorator } from './descriptor_map'; import { dirty, tagFor, tagForProperty } from './tags'; type Option = T | null; @@ -164,6 +165,29 @@ export function tracked( return descriptorForField(elementDesc); } +if (DEBUG) { + // Normally this isn't a classic decorator, but we want to throw a helpful + // error in development so we need it to treat it like one + setComputedDecorator(tracked); +} + +const TRACKED_FIELDS_SHAPE: WeakMap any) | undefined][]> = new WeakMap(); +const TRACKED_FIELDS_VALUES: WeakMap = new WeakMap(); + +function getTrackedFieldValues(obj: any) { + let values = TRACKED_FIELDS_VALUES.get(obj); + + if (values === undefined) { + values = {}; + TRACKED_FIELDS_SHAPE.get(obj.constructor)!.forEach( + ([key, initializer]) => (values![key] = initializer === undefined ? undefined : initializer()) + ); + TRACKED_FIELDS_VALUES.set(obj, values); + } + + return values; +} + function descriptorForField(elementDesc: ElementDescriptor): ElementDescriptor { let { key, kind, initializer } = elementDesc as ElementDescriptor; @@ -172,8 +196,6 @@ function descriptorForField(elementDesc: ElementDescriptor): ElementDescriptor { kind === 'field' ); - let shadowKey = Symbol(key); - return { key, kind: 'method', @@ -185,20 +207,28 @@ function descriptorForField(elementDesc: ElementDescriptor): ElementDescriptor { get(): any { if (CURRENT_TRACKER) CURRENT_TRACKER.add(tagForProperty(this, key)); - if (!(shadowKey in this)) { - this[shadowKey] = initializer !== undefined ? initializer.call(this) : undefined; - } - - return this[shadowKey]; + return getTrackedFieldValues(this)[key]; }, set(newValue: any): void { tagFor(this).inner!['dirty'](); dirty(tagForProperty(this, key)); - this[shadowKey] = newValue; + + getTrackedFieldValues(this)[key] = newValue; + propertyDidChange(); }, }, + finisher(target) { + let shape = TRACKED_FIELDS_SHAPE.get(target); + + if (shape === undefined) { + shape = []; + TRACKED_FIELDS_SHAPE.set(target, shape); + } + + shape.push([key, initializer]); + }, }; } diff --git a/packages/@ember/-internals/metal/lib/watch_key.ts b/packages/@ember/-internals/metal/lib/watch_key.ts index 9191dda3ff2..062681e9c99 100644 --- a/packages/@ember/-internals/metal/lib/watch_key.ts +++ b/packages/@ember/-internals/metal/lib/watch_key.ts @@ -1,7 +1,7 @@ import { Meta, meta as metaFor, peekMeta, UNDEFINED } from '@ember/-internals/meta'; import { lookupDescriptor } from '@ember/-internals/utils'; import { DEBUG } from '@glimmer/env'; -import { descriptorForProperty, isComputedDecorator } from './decorator'; +import { descriptorForProperty, isComputedDecorator } from './descriptor_map'; import { DEFAULT_GETTER_FUNCTION, INHERITING_GETTER_FUNCTION, From 687ec2c90ebb144d38b6954e333dc26104600d4f Mon Sep 17 00:00:00 2001 From: Chris Garrett Date: Fri, 8 Feb 2019 16:21:17 -0800 Subject: [PATCH 3/4] remove shaping --- packages/@ember/-internals/metal/lib/tracked.ts | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/packages/@ember/-internals/metal/lib/tracked.ts b/packages/@ember/-internals/metal/lib/tracked.ts index 3cba94fae4b..edf4a683b41 100644 --- a/packages/@ember/-internals/metal/lib/tracked.ts +++ b/packages/@ember/-internals/metal/lib/tracked.ts @@ -171,7 +171,6 @@ if (DEBUG) { setComputedDecorator(tracked); } -const TRACKED_FIELDS_SHAPE: WeakMap any) | undefined][]> = new WeakMap(); const TRACKED_FIELDS_VALUES: WeakMap = new WeakMap(); function getTrackedFieldValues(obj: any) { @@ -179,9 +178,6 @@ function getTrackedFieldValues(obj: any) { if (values === undefined) { values = {}; - TRACKED_FIELDS_SHAPE.get(obj.constructor)!.forEach( - ([key, initializer]) => (values![key] = initializer === undefined ? undefined : initializer()) - ); TRACKED_FIELDS_VALUES.set(obj, values); } @@ -219,16 +215,6 @@ function descriptorForField(elementDesc: ElementDescriptor): ElementDescriptor { propertyDidChange(); }, }, - finisher(target) { - let shape = TRACKED_FIELDS_SHAPE.get(target); - - if (shape === undefined) { - shape = []; - TRACKED_FIELDS_SHAPE.set(target, shape); - } - - shape.push([key, initializer]); - }, }; } From 287afaee33d069981bc49f0f9b8bf0c05dbb419f Mon Sep 17 00:00:00 2001 From: Chris Garrett Date: Fri, 8 Feb 2019 16:31:37 -0800 Subject: [PATCH 4/4] add initializer back --- packages/@ember/-internals/metal/lib/tracked.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/@ember/-internals/metal/lib/tracked.ts b/packages/@ember/-internals/metal/lib/tracked.ts index edf4a683b41..0cb532de6b5 100644 --- a/packages/@ember/-internals/metal/lib/tracked.ts +++ b/packages/@ember/-internals/metal/lib/tracked.ts @@ -203,7 +203,13 @@ function descriptorForField(elementDesc: ElementDescriptor): ElementDescriptor { get(): any { if (CURRENT_TRACKER) CURRENT_TRACKER.add(tagForProperty(this, key)); - return getTrackedFieldValues(this)[key]; + let values = getTrackedFieldValues(this); + + if (!(key in values)) { + values[key] = initializer !== undefined ? initializer.call(this) : undefined; + } + + return values[key]; }, set(newValue: any): void {