diff --git a/packages/@ember/-internals/metal/lib/computed.ts b/packages/@ember/-internals/metal/lib/computed.ts index cf522637c3c..6142029941a 100644 --- a/packages/@ember/-internals/metal/lib/computed.ts +++ b/packages/@ember/-internals/metal/lib/computed.ts @@ -1,7 +1,7 @@ import { meta as metaFor, peekMeta } from '@ember/-internals/meta'; import { inspect } from '@ember/-internals/utils'; import { EMBER_METAL_TRACKED_PROPERTIES } from '@ember/canary-features'; -import { assert, warn } from '@ember/debug'; +import { assert, deprecate, warn } from '@ember/debug'; import EmberError from '@ember/error'; import { getCachedValueFor, @@ -224,6 +224,16 @@ class ComputedProperty extends Descriptor implements DescriptorWithDependentKeys @public */ volatile(): ComputedProperty { + deprecate( + 'Setting a computed property as volatile has been deprecated. Instead, consider using a native getter with native class syntax.', + false, + { + id: 'computed-property.volatile', + until: '4.0.0', + url: 'https://emberjs.com/deprecations/v3.x#toc_computed-property-volatile', + } + ); + this._volatile = true; return this; } @@ -291,6 +301,21 @@ class ComputedProperty extends Descriptor implements DescriptorWithDependentKeys @public */ property(...passedArgs: string[]): ComputedProperty { + deprecate( + 'Setting dependency keys using the `.property()` modifier has been deprecated. Pass the dependency keys directly to computed as arguments instead. If you are using `.property()` on a computed property macro, consider refactoring your macro to receive additional dependent keys in its initial declaration.', + false, + { + id: 'computed-property.property', + until: '4.0.0', + url: 'https://emberjs.com/deprecations/v3.x#toc_computed-property-property', + } + ); + + this._property(...passedArgs); + return this; + } + + _property(...passedArgs: string[]): ComputedProperty { let args: string[] = []; function addArg(property: string): void { @@ -450,6 +475,16 @@ class ComputedProperty extends Descriptor implements DescriptorWithDependentKeys } clobberSet(obj: object, keyName: string, value: any): any { + deprecate( + `The ${keyName} computed property was just overriden. This removes the computed property and replaces it with a plain value, and has been deprecated. If you want this behavior, consider defining a setter which does it manually.`, + false, + { + id: 'computed-property.override', + until: '4.0.0', + url: 'https://emberjs.com/deprecations/v3.x#toc_computed-property-override', + } + ); + let cachedValue = getCachedValueFor(obj, keyName); defineProperty(obj, keyName, null, cachedValue); set(obj, keyName, value); @@ -614,7 +649,7 @@ export default function computed(...args: (string | ComputedPropertyConfig)[]): let cp = new ComputedProperty(func as ComputedPropertyConfig); if (args.length > 0) { - cp.property(...(args as string[])); + cp._property(...(args as string[])); } return cp; diff --git a/packages/@ember/-internals/metal/lib/injected_property.ts b/packages/@ember/-internals/metal/lib/injected_property.ts index cbd720b0ded..ee8cc64a90b 100644 --- a/packages/@ember/-internals/metal/lib/injected_property.ts +++ b/packages/@ember/-internals/metal/lib/injected_property.ts @@ -3,6 +3,7 @@ import { getOwner } from '@ember/-internals/owner'; import { EMBER_MODULE_UNIFICATION } from '@ember/canary-features'; import { assert } from '@ember/debug'; import { ComputedProperty } from './computed'; +import { defineProperty } from './properties'; export interface InjectedPropertyOptions { source: string; @@ -31,7 +32,7 @@ export default class InjectedProperty extends ComputedProperty { readonly namespace: string | undefined; constructor(type: string, name: string, options?: InjectedPropertyOptions) { - super(injectedPropertyGet); + super(injectedPropertyDesc); this.type = type; this.name = name; @@ -54,22 +55,28 @@ export default class InjectedProperty extends ComputedProperty { } } -function injectedPropertyGet(this: any, keyName: string): any { - let desc = descriptorFor(this, keyName); - let owner = getOwner(this) || this.container; // fallback to `container` for backwards compat +const injectedPropertyDesc = { + get(this: any, keyName: string): any { + let desc = descriptorFor(this, keyName); + let owner = getOwner(this) || this.container; // fallback to `container` for backwards compat - assert( - `InjectedProperties should be defined with the inject computed property macros.`, - desc && desc.type - ); - assert( - `Attempting to lookup an injected property on an object without a container, ensure that the object was instantiated via a container.`, - Boolean(owner) - ); + assert( + `InjectedProperties should be defined with the inject computed property macros.`, + desc && desc.type + ); + assert( + `Attempting to lookup an injected property on an object without a container, ensure that the object was instantiated via a container.`, + Boolean(owner) + ); - let specifier = `${desc.type}:${desc.name || keyName}`; - return owner.lookup(specifier, { - source: desc.source, - namespace: desc.namespace, - }); -} + let specifier = `${desc.type}:${desc.name || keyName}`; + return owner.lookup(specifier, { + source: desc.source, + namespace: desc.namespace, + }); + }, + + set(this: any, keyName: string, value: any) { + defineProperty(this, keyName, null, value); + }, +}; diff --git a/packages/@ember/-internals/metal/tests/computed_test.js b/packages/@ember/-internals/metal/tests/computed_test.js index 44034631676..f37e707710f 100644 --- a/packages/@ember/-internals/metal/tests/computed_test.js +++ b/packages/@ember/-internals/metal/tests/computed_test.js @@ -77,13 +77,15 @@ moduleFor( } ['@test can override volatile computed property'](assert) { - let obj = {}; + expectDeprecation(() => { + let obj = {}; - defineProperty(obj, 'foo', computed(function() {}).volatile()); + defineProperty(obj, 'foo', computed(function() {}).volatile()); - set(obj, 'foo', 'boom'); + set(obj, 'foo', 'boom'); - assert.equal(obj.foo, 'boom'); + assert.equal(obj.foo, 'boom'); + }, 'Setting a computed property as volatile has been deprecated. Instead, consider using a native getter with native class syntax.'); } ['@test defining computed property should invoke property on set'](assert) { @@ -399,13 +401,13 @@ moduleFor( defineProperty( obj, 'plusOne', - computed({ + computed('foo', { get() {}, set(key, value, oldValue) { receivedOldValue = oldValue; return value; }, - }).property('foo') + }) ); set(obj, 'plusOne', 1); @@ -438,10 +440,10 @@ moduleFor( defineProperty( obj, 'foo', - computed({ + computed('bar', { get: getterAndSetter, set: getterAndSetter, - }).property('bar') + }) ); } @@ -478,11 +480,11 @@ moduleFor( defineProperty( obj, 'bar', - computed(function() { + computed('baz', function() { count++; get(this, 'baz'); return 'baz ' + count; - }).property('baz') + }) ); assert.equal(isWatching(obj, 'bar'), false, 'precond not watching dependent key'); @@ -515,15 +517,15 @@ moduleFor( count++; return 'bar ' + count; }; - defineProperty(obj, 'bar', computed({ get: func, set: func }).property('foo')); + defineProperty(obj, 'bar', computed('foo', { get: func, set: func })); defineProperty( obj, 'foo', - computed(function() { + computed('bar', function() { count++; return 'foo ' + count; - }).property('bar') + }) ); assert.equal(get(obj, 'foo'), 'foo 1', 'get once'); @@ -543,10 +545,10 @@ moduleFor( defineProperty( obj, 'foo', - computed(function() { + computed('baz', function() { count++; return 'baz ' + count; - }).property('baz') + }) ); assert.equal( @@ -570,10 +572,10 @@ moduleFor( defineProperty( obj, 'foo', - computed(function() { + computed('qux.{bar,baz}', function() { count++; return 'foo ' + count; - }).property('qux.{bar,baz}') + }) ); assert.equal(get(obj, 'foo'), 'foo 1', 'get once'); @@ -598,10 +600,10 @@ moduleFor( defineProperty( obj, 'roo', - computed(function() { + computed('fee.{bar, baz,bop , }', function() { count++; return 'roo ' + count; - }).property('fee.{bar, baz,bop , }') + }) ); }, /cannot contain spaces/); } @@ -656,7 +658,7 @@ moduleFor( ['@test depending on simple chain'](assert) { // assign computed property - defineProperty(obj, 'prop', computed(func).property('foo.bar.baz.biff')); + defineProperty(obj, 'prop', computed('foo.bar.baz.biff', func)); assert.equal(get(obj, 'prop'), 'BIFF 1'); @@ -699,7 +701,7 @@ moduleFor( ['@test chained dependent keys should evaluate computed properties lazily'](assert) { defineProperty(obj.foo.bar, 'b', computed(func)); - defineProperty(obj.foo, 'c', computed(function() {}).property('bar.b')); + defineProperty(obj.foo, 'c', computed('bar.b', function() {})); assert.equal(count, 0, 'b should not run'); } } @@ -738,21 +740,23 @@ moduleFor( } ['@test setter can be omited'](assert) { - let testObj = EmberObject.extend({ - a: '1', - b: '2', - aInt: computed('a', { - get(keyName) { - assert.equal(keyName, 'aInt', 'getter receives the keyName'); - return parseInt(this.get('a')); - }, - }), - }).create(); + expectDeprecation(() => { + let testObj = EmberObject.extend({ + a: '1', + b: '2', + aInt: computed('a', { + get(keyName) { + assert.equal(keyName, 'aInt', 'getter receives the keyName'); + return parseInt(this.get('a')); + }, + }), + }).create(); - assert.ok(testObj.get('aInt') === 1, 'getter works'); - assert.ok(testObj.get('a') === '1'); - testObj.set('aInt', '123'); - assert.ok(testObj.get('aInt') === '123', 'cp has been updated too'); + assert.ok(testObj.get('aInt') === 1, 'getter works'); + assert.ok(testObj.get('a') === '1'); + testObj.set('aInt', '123'); + assert.ok(testObj.get('aInt') === '123', 'cp has been updated too'); + }, /The aInt computed property was just overriden/); } ['@test getter can be omited'](assert) { @@ -851,7 +855,7 @@ moduleFor( defineProperty( obj, 'fullName', - computed({ + computed('firstName', 'lastName', { get() { return get(this, 'firstName') + ' ' + get(this, 'lastName'); }, @@ -861,7 +865,7 @@ moduleFor( set(this, 'lastName', values[1]); return value; }, - }).property('firstName', 'lastName') + }) ); let fullNameDidChange = 0; @@ -900,7 +904,7 @@ moduleFor( defineProperty( obj, 'plusOne', - computed({ + computed('foo', { get() { return get(this, 'foo') + 1; }, @@ -908,7 +912,7 @@ moduleFor( set(this, 'foo', value); return value + 1; }, - }).property('foo') + }) ); let plusOneDidChange = 0; @@ -936,24 +940,26 @@ moduleFor( 'computed - default setter', class extends AbstractTestCase { ["@test when setting a value on a computed property that doesn't handle sets"](assert) { - let obj = {}; - let observerFired = false; + expectDeprecation(() => { + let obj = {}; + let observerFired = false; - defineProperty( - obj, - 'foo', - computed(function() { - return 'foo'; - }) - ); + defineProperty( + obj, + 'foo', + computed(function() { + return 'foo'; + }) + ); - addObserver(obj, 'foo', null, () => (observerFired = true)); + addObserver(obj, 'foo', null, () => (observerFired = true)); - set(obj, 'foo', 'bar'); + set(obj, 'foo', 'bar'); - assert.equal(get(obj, 'foo'), 'bar', 'The set value is properly returned'); - assert.ok(typeof obj.foo === 'string', 'The computed property was removed'); - assert.ok(observerFired, 'The observer was still notified'); + assert.equal(get(obj, 'foo'), 'bar', 'The set value is properly returned'); + assert.ok(typeof obj.foo === 'string', 'The computed property was removed'); + assert.ok(observerFired, 'The observer was still notified'); + }, /The foo computed property was just overriden./); } } ); diff --git a/packages/@ember/-internals/metal/tests/mixin/computed_test.js b/packages/@ember/-internals/metal/tests/mixin/computed_test.js index b094bcd0aa5..1a9a07afd6c 100644 --- a/packages/@ember/-internals/metal/tests/mixin/computed_test.js +++ b/packages/@ember/-internals/metal/tests/mixin/computed_test.js @@ -110,57 +110,62 @@ moduleFor( } ['@test setter behavior works properly when overriding computed properties'](assert) { - let obj = {}; - - let MixinA = Mixin.create({ - cpWithSetter2: computed(K), - cpWithSetter3: computed(K), - cpWithoutSetter: computed(K), - }); - - let cpWasCalled = false; - - let MixinB = Mixin.create({ - cpWithSetter2: computed({ - get: K, - set() { - cpWasCalled = true; - }, - }), - - cpWithSetter3: computed({ - get: K, - set() { + expectDeprecation(() => { + let obj = {}; + + let MixinA = Mixin.create({ + cpWithSetter2: computed(K), + cpWithSetter3: computed(K), + cpWithoutSetter: computed(K), + }); + + let cpWasCalled = false; + + let MixinB = Mixin.create({ + cpWithSetter2: computed({ + get: K, + set() { + cpWasCalled = true; + }, + }), + + cpWithSetter3: computed({ + get: K, + set() { + cpWasCalled = true; + }, + }), + + cpWithoutSetter: computed(function() { cpWasCalled = true; - }, - }), - - cpWithoutSetter: computed(function() { - cpWasCalled = true; - }), - }); - - MixinA.apply(obj); - MixinB.apply(obj); - - set(obj, 'cpWithSetter2', 'test'); - assert.ok(cpWasCalled, 'The computed property setter was called when defined with two args'); - cpWasCalled = false; - - set(obj, 'cpWithSetter3', 'test'); - assert.ok( - cpWasCalled, - 'The computed property setter was called when defined with three args' - ); - cpWasCalled = false; - - set(obj, 'cpWithoutSetter', 'test'); - assert.equal( - get(obj, 'cpWithoutSetter'), - 'test', - 'The default setter was called, the value is correct' - ); - assert.ok(!cpWasCalled, 'The default setter was called, not the CP itself'); + }), + }); + + MixinA.apply(obj); + MixinB.apply(obj); + + set(obj, 'cpWithSetter2', 'test'); + assert.ok( + cpWasCalled, + 'The computed property setter was called when defined with two args' + ); + cpWasCalled = false; + + set(obj, 'cpWithSetter3', 'test'); + assert.ok( + cpWasCalled, + 'The computed property setter was called when defined with three args' + ); + cpWasCalled = false; + + set(obj, 'cpWithoutSetter', 'test'); + assert.equal( + get(obj, 'cpWithoutSetter'), + 'test', + 'The default setter was called, the value is correct' + ); + assert.ok(!cpWasCalled, 'The default setter was called, not the CP itself'); + }, /The cpWithoutSetter computed property was just overriden./); } } ); diff --git a/packages/@ember/-internals/metal/tests/observer_test.js b/packages/@ember/-internals/metal/tests/observer_test.js index 17e83ee48b2..ffa4ed59cf3 100644 --- a/packages/@ember/-internals/metal/tests/observer_test.js +++ b/packages/@ember/-internals/metal/tests/observer_test.js @@ -54,9 +54,9 @@ moduleFor( defineProperty( obj, 'foo', - computed(function() { + computed('bar', function() { return get(this, 'bar').toUpperCase(); - }).property('bar') + }) ); get(obj, 'foo'); @@ -139,17 +139,17 @@ moduleFor( defineProperty( obj, 'foo', - computed(function() { + computed('bar', function() { return get(this, 'bar').toLowerCase(); - }).property('bar') + }) ); defineProperty( obj, 'bar', - computed(function() { + computed('baz', function() { return get(this, 'baz').toUpperCase(); - }).property('baz') + }) ); mixin(obj, { @@ -205,17 +205,17 @@ moduleFor( defineProperty( obj, 'foo', - computed(function() { + computed('bar', function() { return get(this, 'bar').toLowerCase(); - }).property('bar') + }) ); defineProperty( obj, 'bar', - computed(function() { + computed('baz', function() { return get(this, 'baz').toUpperCase(); - }).property('baz') + }) ); mixin(obj, { @@ -690,14 +690,14 @@ moduleFor( defineProperty( obj, 'foo', - computed({ + computed('baz', { get: function() { return get(this, 'baz'); }, set: function(key, value) { return value; }, - }).property('baz') + }) ); let count = 0; diff --git a/packages/@ember/-internals/metal/tests/performance_test.js b/packages/@ember/-internals/metal/tests/performance_test.js index f88f074495a..a820ffeb8a0 100644 --- a/packages/@ember/-internals/metal/tests/performance_test.js +++ b/packages/@ember/-internals/metal/tests/performance_test.js @@ -30,10 +30,10 @@ moduleFor( defineProperty( obj, 'abc', - computed(function(key) { + computed('a', 'b', 'c', function(key) { cpCount++; return 'computed ' + key; - }).property('a', 'b', 'c') + }) ); get(obj, 'abc'); diff --git a/packages/@ember/-internals/metal/tests/watching/is_watching_test.js b/packages/@ember/-internals/metal/tests/watching/is_watching_test.js index 77321839554..19cbbed261b 100644 --- a/packages/@ember/-internals/metal/tests/watching/is_watching_test.js +++ b/packages/@ember/-internals/metal/tests/watching/is_watching_test.js @@ -61,7 +61,7 @@ moduleFor( testObserver( assert, (obj, key, fn) => { - defineProperty(obj, fn, computed(function() {}).property(key)); + defineProperty(obj, fn, computed(key, function() {})); get(obj, fn); }, (obj, key, fn) => defineProperty(obj, fn, null) @@ -72,7 +72,7 @@ moduleFor( testObserver( assert, (obj, key, fn) => { - defineProperty(obj, fn, computed(function() {}).property(key + '.bar')); + defineProperty(obj, fn, computed(key + '.bar', function() {})); get(obj, fn); }, (obj, key, fn) => defineProperty(obj, fn, null) diff --git a/packages/@ember/-internals/routing/lib/system/route.ts b/packages/@ember/-internals/routing/lib/system/route.ts index 2234211fb6a..babf671acce 100644 --- a/packages/@ember/-internals/routing/lib/system/route.ts +++ b/packages/@ember/-internals/routing/lib/system/route.ts @@ -1,4 +1,12 @@ -import { computed, get, getProperties, isEmpty, set, setProperties } from '@ember/-internals/metal'; +import { + computed, + defineProperty, + get, + getProperties, + isEmpty, + set, + setProperties, +} from '@ember/-internals/metal'; import { getOwner, Owner } from '@ember/-internals/owner'; import { A as emberA, @@ -2134,33 +2142,42 @@ Route.reopen(ActionHandler, Evented, { @type {Object} @private */ - store: computed(function(this: Route) { - let owner = getOwner(this); - let routeName = this.routeName; - let namespace = get(this, '_router.namespace'); - - return { - find(name: string, value: unknown) { - let modelClass: any = owner.factoryFor(`model:${name}`); - - assert( - `You used the dynamic segment ${name}_id in your route ${routeName}, but ${namespace}.${classify( - name - )} did not exist and you did not override your route's \`model\` hook.`, - Boolean(modelClass) - ); + store: computed({ + get(this: Route) { + let owner = getOwner(this); + let routeName = this.routeName; + let namespace = get(this, '_router.namespace'); + + return { + find(name: string, value: unknown) { + let modelClass: any = owner.factoryFor(`model:${name}`); + + assert( + `You used the dynamic segment ${name}_id in your route ${routeName}, but ${namespace}.${classify( + name + )} did not exist and you did not override your route's \`model\` hook.`, + Boolean(modelClass) + ); + + if (!modelClass) { + return; + } - if (!modelClass) { - return; - } + modelClass = modelClass.class; - modelClass = modelClass.class; + assert( + `${classify(name)} has no method \`find\`.`, + typeof modelClass.find === 'function' + ); - assert(`${classify(name)} has no method \`find\`.`, typeof modelClass.find === 'function'); + return modelClass.find(value); + }, + }; + }, - return modelClass.find(value); - }, - }; + set(key, value) { + defineProperty(this, key, null, value); + }, }), /** @@ -2168,128 +2185,134 @@ Route.reopen(ActionHandler, Evented, { @property _qp */ - _qp: computed(function(this: Route) { - let combinedQueryParameterConfiguration; + _qp: computed({ + get(this: Route) { + let combinedQueryParameterConfiguration; + + let controllerName = this.controllerName || this.routeName; + let owner = getOwner(this); + let controller = owner.lookup(`controller:${controllerName}`); + let queryParameterConfiguraton = get(this, 'queryParams'); + let hasRouterDefinedQueryParams = Object.keys(queryParameterConfiguraton).length > 0; + + if (controller) { + // the developer has authored a controller class in their application for + // this route find its query params and normalize their object shape them + // merge in the query params for the route. As a mergedProperty, + // Route#queryParams is always at least `{}` + + let controllerDefinedQueryParameterConfiguration = get(controller, 'queryParams') || {}; + let normalizedControllerQueryParameterConfiguration = normalizeControllerQueryParams( + controllerDefinedQueryParameterConfiguration + ); + combinedQueryParameterConfiguration = mergeEachQueryParams( + normalizedControllerQueryParameterConfiguration, + queryParameterConfiguraton + ); + } else if (hasRouterDefinedQueryParams) { + // the developer has not defined a controller but *has* supplied route query params. + // Generate a class for them so we can later insert default values + controller = generateController(owner, controllerName); + combinedQueryParameterConfiguration = queryParameterConfiguraton; + } - let controllerName = this.controllerName || this.routeName; - let owner = getOwner(this); - let controller = owner.lookup(`controller:${controllerName}`); - let queryParameterConfiguraton = get(this, 'queryParams'); - let hasRouterDefinedQueryParams = Object.keys(queryParameterConfiguraton).length > 0; - - if (controller) { - // the developer has authored a controller class in their application for - // this route find its query params and normalize their object shape them - // merge in the query params for the route. As a mergedProperty, - // Route#queryParams is always at least `{}` - - let controllerDefinedQueryParameterConfiguration = get(controller, 'queryParams') || {}; - let normalizedControllerQueryParameterConfiguration = normalizeControllerQueryParams( - controllerDefinedQueryParameterConfiguration - ); - combinedQueryParameterConfiguration = mergeEachQueryParams( - normalizedControllerQueryParameterConfiguration, - queryParameterConfiguraton - ); - } else if (hasRouterDefinedQueryParams) { - // the developer has not defined a controller but *has* supplied route query params. - // Generate a class for them so we can later insert default values - controller = generateController(owner, controllerName); - combinedQueryParameterConfiguration = queryParameterConfiguraton; - } + let qps = []; + let map = {}; + let propertyNames = []; - let qps = []; - let map = {}; - let propertyNames = []; + for (let propName in combinedQueryParameterConfiguration) { + if (!combinedQueryParameterConfiguration.hasOwnProperty(propName)) { + continue; + } - for (let propName in combinedQueryParameterConfiguration) { - if (!combinedQueryParameterConfiguration.hasOwnProperty(propName)) { - continue; - } + // to support the dubious feature of using unknownProperty + // on queryParams configuration + if (propName === 'unknownProperty' || propName === '_super') { + // possible todo: issue deprecation warning? + continue; + } - // to support the dubious feature of using unknownProperty - // on queryParams configuration - if (propName === 'unknownProperty' || propName === '_super') { - // possible todo: issue deprecation warning? - continue; - } + let desc = combinedQueryParameterConfiguration[propName]; + let scope = desc.scope || 'model'; + let parts; - let desc = combinedQueryParameterConfiguration[propName]; - let scope = desc.scope || 'model'; - let parts; + if (scope === 'controller') { + parts = []; + } - if (scope === 'controller') { - parts = []; - } + let urlKey = desc.as || this.serializeQueryParamKey(propName); + let defaultValue = get(controller, propName); - let urlKey = desc.as || this.serializeQueryParamKey(propName); - let defaultValue = get(controller, propName); + if (Array.isArray(defaultValue)) { + defaultValue = emberA(defaultValue.slice()); + } - if (Array.isArray(defaultValue)) { - defaultValue = emberA(defaultValue.slice()); + let type = desc.type || typeOf(defaultValue); + + let defaultValueSerialized = this.serializeQueryParam(defaultValue, urlKey, type); + let scopedPropertyName = `${controllerName}:${propName}`; + let qp = { + undecoratedDefaultValue: get(controller, propName), + defaultValue, + serializedDefaultValue: defaultValueSerialized, + serializedValue: defaultValueSerialized, + + type, + urlKey, + prop: propName, + scopedPropertyName, + controllerName, + route: this, + parts, // provided later when stashNames is called if 'model' scope + values: null, // provided later when setup is called. no idea why. + scope, + }; + + map[propName] = map[urlKey] = map[scopedPropertyName] = qp; + qps.push(qp); + propertyNames.push(propName); } - let type = desc.type || typeOf(defaultValue); - - let defaultValueSerialized = this.serializeQueryParam(defaultValue, urlKey, type); - let scopedPropertyName = `${controllerName}:${propName}`; - let qp = { - undecoratedDefaultValue: get(controller, propName), - defaultValue, - serializedDefaultValue: defaultValueSerialized, - serializedValue: defaultValueSerialized, - - type, - urlKey, - prop: propName, - scopedPropertyName, - controllerName, - route: this, - parts, // provided later when stashNames is called if 'model' scope - values: null, // provided later when setup is called. no idea why. - scope, + return { + qps, + map, + propertyNames, + states: { + /* + Called when a query parameter changes in the URL, this route cares + about that query parameter, but the route is not currently + in the active route hierarchy. + */ + inactive: (prop: string, value: unknown) => { + let qp = map[prop]; + this._qpChanged(prop, value, qp); + }, + /* + Called when a query parameter changes in the URL, this route cares + about that query parameter, and the route is currently + in the active route hierarchy. + */ + active: (prop: string, value: unknown) => { + let qp = map[prop]; + this._qpChanged(prop, value, qp); + return this._activeQPChanged(qp, value); + }, + /* + Called when a value of a query parameter this route handles changes in a controller + and the route is currently in the active route hierarchy. + */ + allowOverrides: (prop: string, value: unknown) => { + let qp = map[prop]; + this._qpChanged(prop, value, qp); + return this._updatingQPChanged(qp); + }, + }, }; + }, - map[propName] = map[urlKey] = map[scopedPropertyName] = qp; - qps.push(qp); - propertyNames.push(propName); - } - - return { - qps, - map, - propertyNames, - states: { - /* - Called when a query parameter changes in the URL, this route cares - about that query parameter, but the route is not currently - in the active route hierarchy. - */ - inactive: (prop: string, value: unknown) => { - let qp = map[prop]; - this._qpChanged(prop, value, qp); - }, - /* - Called when a query parameter changes in the URL, this route cares - about that query parameter, and the route is currently - in the active route hierarchy. - */ - active: (prop: string, value: unknown) => { - let qp = map[prop]; - this._qpChanged(prop, value, qp); - return this._activeQPChanged(qp, value); - }, - /* - Called when a value of a query parameter this route handles changes in a controller - and the route is currently in the active route hierarchy. - */ - allowOverrides: (prop: string, value: unknown) => { - let qp = map[prop]; - this._qpChanged(prop, value, qp); - return this._updatingQPChanged(qp); - }, - }, - }; + set(key, value) { + defineProperty(this, key, null, value); + }, }), /** diff --git a/packages/@ember/-internals/runtime/tests/legacy_1x/mixins/observable/observable_test.js b/packages/@ember/-internals/runtime/tests/legacy_1x/mixins/observable/observable_test.js index 220e4c3dce2..3adc1c920eb 100644 --- a/packages/@ember/-internals/runtime/tests/legacy_1x/mixins/observable/observable_test.js +++ b/packages/@ember/-internals/runtime/tests/legacy_1x/mixins/observable/observable_test.js @@ -48,7 +48,7 @@ moduleFor( object = ObservableObject.extend(Observable, { computed: computed(function() { return 'value'; - }).volatile(), + }), method() { return 'value'; }, @@ -97,7 +97,7 @@ moduleFor( objectA = ObservableObject.extend({ computed: computed(function() { return 'value'; - }).volatile(), + }), method() { return 'value'; }, @@ -164,7 +164,7 @@ moduleFor( bar: ObservableObject.extend({ baz: computed(function() { return 'blargh'; - }).volatile(), + }), }).create(), }); @@ -202,7 +202,7 @@ moduleFor( this._computed = value; return this._computed; }, - }).volatile(), + }), method(key, value) { if (value !== undefined) { @@ -284,99 +284,95 @@ moduleFor( beforeEach() { lookup = context.lookup = {}; - object = ObservableObject.extend({ - computed: computed({ - get() { - this.computedCalls.push('getter-called'); - return 'computed'; - }, - set(key, value) { - this.computedCalls.push(value); - }, - }).volatile(), + expectDeprecation(() => { + object = ObservableObject.extend({ + computed: computed({ + get() { + this.computedCalls.push('getter-called'); + return 'computed'; + }, + set(key, value) { + this.computedCalls.push(value); + }, + }).volatile(), - computedCached: computed({ - get() { - this.computedCachedCalls.push('getter-called'); - return 'computedCached'; - }, - set: function(key, value) { - this.computedCachedCalls.push(value); - }, - }), + computedCached: computed({ + get() { + this.computedCachedCalls.push('getter-called'); + return 'computedCached'; + }, + set: function(key, value) { + this.computedCachedCalls.push(value); + }, + }), - dependent: computed({ - get() { - this.dependentCalls.push('getter-called'); - return 'dependent'; - }, - set(key, value) { - this.dependentCalls.push(value); - }, - }) - .property('changer') - .volatile(), - dependentFront: computed('changer', { - get() { - this.dependentFrontCalls.push('getter-called'); - return 'dependentFront'; - }, - set(key, value) { - this.dependentFrontCalls.push(value); - }, - }).volatile(), - dependentCached: computed({ - get() { - this.dependentCachedCalls.push('getter-called!'); - return 'dependentCached'; - }, - set(key, value) { - this.dependentCachedCalls.push(value); - }, - }).property('changer'), + dependent: computed('changer', { + get() { + this.dependentCalls.push('getter-called'); + return 'dependent'; + }, + set(key, value) { + this.dependentCalls.push(value); + }, + }).volatile(), + dependentFront: computed('changer', { + get() { + this.dependentFrontCalls.push('getter-called'); + return 'dependentFront'; + }, + set(key, value) { + this.dependentFrontCalls.push(value); + }, + }).volatile(), + dependentCached: computed('changer', { + get() { + this.dependentCachedCalls.push('getter-called!'); + return 'dependentCached'; + }, + set(key, value) { + this.dependentCachedCalls.push(value); + }, + }), - inc: computed('changer', function() { - return this.incCallCount++; - }), + inc: computed('changer', function() { + return this.incCallCount++; + }), - nestedInc: computed(function() { - get(this, 'inc'); - return this.nestedIncCallCount++; - }).property('inc'), + nestedInc: computed('inc', function() { + get(this, 'inc'); + return this.nestedIncCallCount++; + }), - isOn: computed({ - get() { - return this.get('state') === 'on'; - }, - set() { - this.set('state', 'on'); - return this.get('state') === 'on'; - }, - }) - .property('state') - .volatile(), + isOn: computed('state', { + get() { + return this.get('state') === 'on'; + }, + set() { + this.set('state', 'on'); + return this.get('state') === 'on'; + }, + }).volatile(), - isOff: computed({ - get() { - return this.get('state') === 'off'; - }, - set() { - this.set('state', 'off'); - return this.get('state') === 'off'; - }, - }) - .property('state') - .volatile(), - }).create({ - computedCalls: [], - computedCachedCalls: [], - changer: 'foo', - dependentCalls: [], - dependentFrontCalls: [], - dependentCachedCalls: [], - incCallCount: 0, - nestedIncCallCount: 0, - state: 'on', + isOff: computed('state', { + get() { + return this.get('state') === 'off'; + }, + set() { + this.set('state', 'off'); + return this.get('state') === 'off'; + }, + }).volatile(), + }).create({ + computedCalls: [], + computedCachedCalls: [], + changer: 'foo', + dependentCalls: [], + dependentFrontCalls: [], + dependentCachedCalls: [], + incCallCount: 0, + nestedIncCallCount: 0, + state: 'on', + }); }); } afterEach() { @@ -542,9 +538,9 @@ moduleFor( ['@test dependent keys should be able to be specified as property paths'](assert) { var depObj = ObservableObject.extend({ - menuPrice: computed(function() { + menuPrice: computed('menu.price', function() { return this.get('menu.price'); - }).property('menu.price'), + }), }).create({ menu: ObservableObject.create({ price: 5, diff --git a/packages/@ember/-internals/runtime/tests/legacy_1x/mixins/observable/propertyChanges_test.js b/packages/@ember/-internals/runtime/tests/legacy_1x/mixins/observable/propertyChanges_test.js index fa70f6cb17e..d73e9faa7bf 100644 --- a/packages/@ember/-internals/runtime/tests/legacy_1x/mixins/observable/propertyChanges_test.js +++ b/packages/@ember/-internals/runtime/tests/legacy_1x/mixins/observable/propertyChanges_test.js @@ -112,31 +112,33 @@ moduleFor( ['@test should invalidate function property cache when notifyPropertyChange is called']( assert ) { - let a = ObservableObject.extend({ - b: computed({ - get() { - return this._b; - }, - set(key, value) { - this._b = value; - return this; - }, - }).volatile(), - }).create({ - _b: null, - }); - - a.set('b', 'foo'); - assert.equal(a.get('b'), 'foo', 'should have set the correct value for property b'); - - a._b = 'bar'; - a.notifyPropertyChange('b'); - a.set('b', 'foo'); - assert.equal( - a.get('b'), - 'foo', - 'should have invalidated the cache so that the newly set value is actually set' - ); + expectDeprecation(() => { + let a = ObservableObject.extend({ + b: computed({ + get() { + return this._b; + }, + set(key, value) { + this._b = value; + return this; + }, + }).volatile(), + }).create({ + _b: null, + }); + + a.set('b', 'foo'); + assert.equal(a.get('b'), 'foo', 'should have set the correct value for property b'); + + a._b = 'bar'; + a.notifyPropertyChange('b'); + a.set('b', 'foo'); + assert.equal( + a.get('b'), + 'foo', + 'should have invalidated the cache so that the newly set value is actually set' + ); + }, /Setting a computed property as volatile has been deprecated/); } } ); diff --git a/packages/@ember/-internals/runtime/tests/system/array_proxy/arranged_content_test.js b/packages/@ember/-internals/runtime/tests/system/array_proxy/arranged_content_test.js index 07048f0ae3b..c5a91b7de73 100644 --- a/packages/@ember/-internals/runtime/tests/system/array_proxy/arranged_content_test.js +++ b/packages/@ember/-internals/runtime/tests/system/array_proxy/arranged_content_test.js @@ -146,7 +146,7 @@ moduleFor( beforeEach() { run(function() { array = ArrayProxy.extend({ - arrangedContent: computed(function() { + arrangedContent: computed('content.[]', function() { let content = this.get('content'); return ( content && @@ -162,7 +162,7 @@ moduleFor( }) ) ); - }).property('content.[]'), + }), objectAtContent(idx) { let obj = objectAt(this.get('arrangedContent'), idx); diff --git a/packages/@ember/-internals/runtime/tests/system/object/computed_test.js b/packages/@ember/-internals/runtime/tests/system/object/computed_test.js index 9ea2737f735..d48ae6f9d7d 100644 --- a/packages/@ember/-internals/runtime/tests/system/object/computed_test.js +++ b/packages/@ember/-internals/runtime/tests/system/object/computed_test.js @@ -73,10 +73,10 @@ moduleFor( count: 0, - foo: computed(function() { + foo: computed('bar.baz', function() { set(this, 'count', get(this, 'count') + 1); return get(get(this, 'bar'), 'baz') + ' ' + get(this, 'count'); - }).property('bar.baz'), + }), }); let Subclass = MyClass.extend({ @@ -109,10 +109,10 @@ moduleFor( count: 0, - foo: computed(function() { + foo: computed('bar.baz', function() { set(this, 'count', get(this, 'count') + 1); return get(get(this, 'bar'), 'baz') + ' ' + get(this, 'count'); - }).property('bar.baz'), + }), }); let Subclass = MyClass.extend({ @@ -123,10 +123,10 @@ moduleFor( count: 0, - foo: computed(function() { + foo: computed('bar2.baz', function() { set(this, 'count', get(this, 'count') + 1); return get(get(this, 'bar2'), 'baz') + ' ' + get(this, 'count'); - }).property('bar2.baz'), + }), }); let obj2 = Subclass.create(); @@ -152,7 +152,7 @@ moduleFor( ); let ClassWithNoMetadata = EmberObject.extend({ - computedProperty: computed(function() {}).volatile(), + computedProperty: computed(function() {}), staticProperty: 12, }); @@ -356,5 +356,23 @@ moduleFor( assert.equal(obj1.get('name'), '1'); assert.equal(obj2.get('name'), '2'); } + + ['@test can declare dependent keys with .property()'](assert) { + expectDeprecation(() => { + let Obj = EmberObject.extend({ + foo: computed(function() { + return this.bar; + }).property('bar'), + }); + + let obj = Obj.create({ bar: 1 }); + + assert.equal(obj.get('foo'), 1); + + obj.set('bar', 2); + + assert.equal(obj.get('foo'), 2); + }, /Setting dependency keys using the `.property\(\)` modifier has been deprecated/); + } } ); diff --git a/packages/@ember/-internals/runtime/tests/system/object_proxy_test.js b/packages/@ember/-internals/runtime/tests/system/object_proxy_test.js index 3a4c5a53ed4..0a73fc39a1b 100644 --- a/packages/@ember/-internals/runtime/tests/system/object_proxy_test.js +++ b/packages/@ember/-internals/runtime/tests/system/object_proxy_test.js @@ -180,7 +180,7 @@ moduleFor( let last; let Proxy = ObjectProxy.extend({ - fullName: computed(function() { + fullName: computed('firstName', 'lastName', function() { let firstName = this.get('firstName'); let lastName = this.get('lastName'); @@ -188,7 +188,7 @@ moduleFor( return firstName + ' ' + lastName; } return firstName || lastName; - }).property('firstName', 'lastName'), + }), }); let proxy = Proxy.create(); diff --git a/packages/@ember/object/lib/computed/reduce_computed_macros.js b/packages/@ember/object/lib/computed/reduce_computed_macros.js index 8f650e13f52..c039665ade1 100644 --- a/packages/@ember/object/lib/computed/reduce_computed_macros.js +++ b/packages/@ember/object/lib/computed/reduce_computed_macros.js @@ -1,8 +1,15 @@ /** @module @ember/object */ +import { DEBUG } from '@glimmer/env'; import { assert } from '@ember/debug'; -import { get, ComputedProperty, addObserver, removeObserver } from '@ember/-internals/metal'; +import { + get, + computed, + ComputedProperty, + addObserver, + removeObserver, +} from '@ember/-internals/metal'; import { compare, isArray, A as emberA, uniqBy as uniqByArray } from '@ember/-internals/runtime'; function reduceMacro(dependentKey, callback, initialValue, name) { @@ -25,7 +32,7 @@ function reduceMacro(dependentKey, callback, initialValue, name) { return cp; } -function arrayMacro(dependentKey, callback) { +function arrayMacro(dependentKey, additionalDependentKeys, callback) { // This is a bit ugly let propertyName; if (/@each/.test(dependentKey)) { @@ -35,21 +42,14 @@ function arrayMacro(dependentKey, callback) { dependentKey += '.[]'; } - let cp = new ComputedProperty( - function() { - let value = get(this, propertyName); - if (isArray(value)) { - return emberA(callback.call(this, value)); - } else { - return emberA(); - } - }, - { readOnly: true } - ); - - cp.property(dependentKey); // this forces to expand properties GH #15855 - - return cp; + return computed(dependentKey, ...additionalDependentKeys, function() { + let value = get(this, propertyName); + if (isArray(value)) { + return emberA(callback.call(this, value)); + } else { + return emberA(); + } + }).readOnly(); } function multiArrayMacro(_dependentKeys, callback, name) { @@ -217,12 +217,28 @@ export function min(dependentKey) { @for @ember/object/computed @static @param {String} dependentKey + @param {Array} additionalDependentKeys optional array of additional dependent keys @param {Function} callback @return {ComputedProperty} an array mapped via the callback @public */ -export function map(dependentKey, callback) { - return arrayMacro(dependentKey, function(value) { +export function map(dependentKey, additionalDependentKeys, callback) { + if (callback === undefined && typeof additionalDependentKeys === 'function') { + callback = additionalDependentKeys; + additionalDependentKeys = []; + } + + assert( + 'The final parameter provided to map must be a callback function', + typeof callback === 'function' + ); + + assert( + 'The second parameter provided to map must either be the callback or an array of additional dependent keys', + Array.isArray(additionalDependentKeys) + ); + + return arrayMacro(dependentKey, additionalDependentKeys, function(value) { return value.map(callback, this); }); } @@ -338,12 +354,28 @@ export function mapBy(dependentKey, propertyKey) { @for @ember/object/computed @static @param {String} dependentKey + @param {Array} additionalDependentKeys optional array of additional dependent keys @param {Function} callback @return {ComputedProperty} the filtered array @public */ -export function filter(dependentKey, callback) { - return arrayMacro(dependentKey, function(value) { +export function filter(dependentKey, additionalDependentKeys, callback) { + if (callback === undefined && typeof additionalDependentKeys === 'function') { + callback = additionalDependentKeys; + additionalDependentKeys = []; + } + + assert( + 'The final parameter provided to filter must be a callback function', + typeof callback === 'function' + ); + + assert( + 'The second parameter provided to filter must either be the callback or an array of additional dependent keys', + Array.isArray(additionalDependentKeys) + ); + + return arrayMacro(dependentKey, additionalDependentKeys, function(value) { return value.filter(callback, this); }); } @@ -782,28 +814,51 @@ export function collect(...dependentKeys) { @for @ember/object/computed @static @param {String} itemsKey + @param {Array} additionalDependentKeys optional array of additional dependent keys @param {String or Function} sortDefinition a dependent key to an array of sort properties (add `:desc` to the arrays sort properties to sort descending) or a function to use when sorting @return {ComputedProperty} computes a new sorted array based on the sort property array or callback function @public */ -export function sort(itemsKey, sortDefinition) { - assert( - '`computed.sort` requires two arguments: an array key to sort and ' + - 'either a sort properties key or sort function', - arguments.length === 2 - ); +export function sort(itemsKey, additionalDependentKeys, sortDefinition) { + if (DEBUG) { + let argumentsValid = false; + + if (arguments.length === 2) { + argumentsValid = + typeof itemsKey === 'string' && + (typeof additionalDependentKeys === 'string' || + typeof additionalDependentKeys === 'function'); + } + + if (arguments.length === 3) { + argumentsValid = + typeof itemsKey === 'string' && + Array.isArray(additionalDependentKeys) && + typeof sortDefinition === 'function'; + } + + assert( + '`computed.sort` can either be used with an array of sort properties or with a sort function. If used with an array of sort properties, it must receive exactly two arguments: the key of the array to sort, and the key of the array of sort properties. If used with a sort function, it may recieve up to three arguments: the key of the array to sort, an optional additional array of dependent keys for the computed property, and the sort function.', + argumentsValid + ); + } + + if (sortDefinition === undefined && !Array.isArray(additionalDependentKeys)) { + sortDefinition = additionalDependentKeys; + additionalDependentKeys = []; + } if (typeof sortDefinition === 'function') { - return customSort(itemsKey, sortDefinition); + return customSort(itemsKey, additionalDependentKeys, sortDefinition); } else { return propertySort(itemsKey, sortDefinition); } } -function customSort(itemsKey, comparator) { - return arrayMacro(itemsKey, function(value) { +function customSort(itemsKey, additionalDependentKeys, comparator) { + return arrayMacro(itemsKey, additionalDependentKeys, function(value) { return value.slice().sort((x, y) => comparator.call(this, x, y)); }); } diff --git a/packages/@ember/object/tests/computed/reduce_computed_macros_test.js b/packages/@ember/object/tests/computed/reduce_computed_macros_test.js index d9b9878cee2..3b2aaf9106e 100644 --- a/packages/@ember/object/tests/computed/reduce_computed_macros_test.js +++ b/packages/@ember/object/tests/computed/reduce_computed_macros_test.js @@ -172,6 +172,48 @@ moduleFor( 'properties unshifted in sequence are mapped correctly' ); } + + ['@test it updates if additional dependent keys are modified'](assert) { + obj = EmberObject.extend({ + mapped: map('array', ['key'], function(item) { + return item[this.key]; + }), + }).create({ + key: 'name', + array: emberA([{ name: 'Cercei', house: 'Lannister' }]), + }); + + assert.deepEqual( + obj.get('mapped'), + ['Cercei'], + 'precond - mapped array is initially correct' + ); + + obj.set('key', 'house'); + assert.deepEqual( + obj.get('mapped'), + ['Lannister'], + 'mapped prop updates correctly when additional dependency is updated' + ); + } + + ['@test it throws on bad inputs']() { + expectAssertion(() => { + map('items.@each.{prop}', 'foo'); + }, /The final parameter provided to map must be a callback function/); + + expectAssertion(() => { + map('items.@each.{prop}', 'foo', function() {}); + }, /The second parameter provided to map must either be the callback or an array of additional dependent keys/); + + expectAssertion(() => { + map('items.@each.{prop}', function() {}, ['foo']); + }, /The final parameter provided to map must be a callback function/); + + expectAssertion(() => { + map('items.@each.{prop}', ['foo']); + }, /The final parameter provided to map must be a callback function/); + } } ); @@ -401,6 +443,48 @@ moduleFor( assert.deepEqual(obj.get('filtered'), []); } + + ['@test it updates if additional dependent keys are modified'](assert) { + obj = EmberObject.extend({ + filtered: filter('array', ['modulo'], function(item) { + return item % this.modulo === 0; + }), + }).create({ + modulo: 2, + array: emberA([1, 2, 3, 4, 5, 6, 7, 8]), + }); + + assert.deepEqual( + obj.get('filtered'), + [2, 4, 6, 8], + 'precond - filtered array is initially correct' + ); + + obj.set('modulo', 3); + assert.deepEqual( + obj.get('filtered'), + [3, 6], + 'filtered prop updates correctly when additional dependency is updated' + ); + } + + ['@test it throws on bad inputs']() { + expectAssertion(() => { + filter('items.@each.{prop}', 'foo'); + }, /The final parameter provided to filter must be a callback function/); + + expectAssertion(() => { + filter('items.@each.{prop}', 'foo', function() {}); + }, /The second parameter provided to filter must either be the callback or an array of additional dependent keys/); + + expectAssertion(() => { + filter('items.@each.{prop}', function() {}, ['foo']); + }, /The final parameter provided to filter must be a callback function/); + + expectAssertion(() => { + filter('items.@each.{prop}', ['foo']); + }, /The final parameter provided to filter must be a callback function/); + } } ); @@ -1726,6 +1810,54 @@ moduleFor( assert.expect(0); } } + + ['@test sort updates if additional dependent keys are present'](assert) { + obj = EmberObject.extend({ + sortedItems: sort('items', ['sortFunction'], function() { + return this.sortFunction(...arguments); + }), + }).create({ + sortFunction: sortByLnameFname, + items: emberA([ + { fname: 'Jaime', lname: 'Lannister', age: 34 }, + { fname: 'Cersei', lname: 'Lannister', age: 34 }, + { fname: 'Robb', lname: 'Stark', age: 16 }, + { fname: 'Bran', lname: 'Stark', age: 8 }, + ]), + }); + + assert.deepEqual( + obj.get('sortedItems').mapBy('fname'), + ['Cersei', 'Jaime', 'Bran', 'Robb'], + 'array is initially sorted' + ); + + obj.set('sortFunction', (a, b) => a.age > b.age); + + assert.deepEqual( + obj.get('sortedItems').mapBy('fname'), + ['Jaime', 'Cersei', 'Robb', 'Bran'], + 'array is updated when dependent key changes' + ); + } + + ['@test it throws on bad inputs']() { + expectAssertion(() => { + sort('foo', 'bar', 'baz'); + }, /`computed.sort` can either be used with an array of sort properties or with a sort function/); + + expectAssertion(() => { + sort('foo', ['bar'], 'baz'); + }, /`computed.sort` can either be used with an array of sort properties or with a sort function/); + + expectAssertion(() => { + sort('foo', 'bar', function() {}); + }, /`computed.sort` can either be used with an array of sort properties or with a sort function/); + + expectAssertion(() => { + sort('foo', ['bar'], function() {}, 'baz'); + }, /`computed.sort` can either be used with an array of sort properties or with a sort function/); + } } );