diff --git a/packages/@ember/-internals/metal/lib/computed.ts b/packages/@ember/-internals/metal/lib/computed.ts index cf522637c3c..9b9b50dc03a 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 { inspect, toString } 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,18 @@ class ComputedProperty extends Descriptor implements DescriptorWithDependentKeys } clobberSet(obj: object, keyName: string, value: any): any { + deprecate( + `The ${toString( + obj + )}#${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 +651,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..d524a067d5b 100644 --- a/packages/@ember/-internals/metal/tests/computed_test.js +++ b/packages/@ember/-internals/metal/tests/computed_test.js @@ -79,9 +79,13 @@ moduleFor( ['@test can override volatile computed property'](assert) { let obj = {}; - defineProperty(obj, 'foo', computed(function() {}).volatile()); + expectDeprecation(() => { + defineProperty(obj, 'foo', computed(function() {}).volatile()); + }, 'Setting a computed property as volatile has been deprecated. Instead, consider using a native getter with native class syntax.'); - set(obj, 'foo', 'boom'); + expectDeprecation(() => { + set(obj, 'foo', 'boom'); + }, /The \[object Object\]#foo computed property was just overriden./); assert.equal(obj.foo, 'boom'); } @@ -399,13 +403,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 +442,10 @@ moduleFor( defineProperty( obj, 'foo', - computed({ + computed('bar', { get: getterAndSetter, set: getterAndSetter, - }).property('bar') + }) ); } @@ -478,11 +482,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 +519,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 +547,10 @@ moduleFor( defineProperty( obj, 'foo', - computed(function() { + computed('baz', function() { count++; return 'baz ' + count; - }).property('baz') + }) ); assert.equal( @@ -570,10 +574,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 +602,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 +660,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 +703,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'); } } @@ -751,7 +755,11 @@ moduleFor( assert.ok(testObj.get('aInt') === 1, 'getter works'); assert.ok(testObj.get('a') === '1'); - testObj.set('aInt', '123'); + + expectDeprecation(() => { + testObj.set('aInt', '123'); + }, /The <\(unknown\):ember\d*>#aInt computed property was just overriden/); + assert.ok(testObj.get('aInt') === '123', 'cp has been updated too'); } @@ -851,7 +859,7 @@ moduleFor( defineProperty( obj, 'fullName', - computed({ + computed('firstName', 'lastName', { get() { return get(this, 'firstName') + ' ' + get(this, 'lastName'); }, @@ -861,7 +869,7 @@ moduleFor( set(this, 'lastName', values[1]); return value; }, - }).property('firstName', 'lastName') + }) ); let fullNameDidChange = 0; @@ -900,7 +908,7 @@ moduleFor( defineProperty( obj, 'plusOne', - computed({ + computed('foo', { get() { return get(this, 'foo') + 1; }, @@ -908,7 +916,7 @@ moduleFor( set(this, 'foo', value); return value + 1; }, - }).property('foo') + }) ); let plusOneDidChange = 0; @@ -949,7 +957,9 @@ moduleFor( addObserver(obj, 'foo', null, () => (observerFired = true)); - set(obj, 'foo', 'bar'); + expectDeprecation(() => { + set(obj, 'foo', 'bar'); + }, /The \[object Object\]#foo computed property was just overriden./); assert.equal(get(obj, 'foo'), 'bar', 'The set value is properly returned'); assert.ok(typeof obj.foo === 'string', 'The computed property was removed'); diff --git a/packages/@ember/-internals/metal/tests/mixin/computed_test.js b/packages/@ember/-internals/metal/tests/mixin/computed_test.js index b094bcd0aa5..071464c5d86 100644 --- a/packages/@ember/-internals/metal/tests/mixin/computed_test.js +++ b/packages/@ember/-internals/metal/tests/mixin/computed_test.js @@ -154,7 +154,10 @@ moduleFor( ); cpWasCalled = false; - set(obj, 'cpWithoutSetter', 'test'); + expectDeprecation(() => { + set(obj, 'cpWithoutSetter', 'test'); + }, /The \[object Object\]#cpWithoutSetter computed property was just overriden./); + assert.equal( get(obj, 'cpWithoutSetter'), 'test', 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..392247e6836 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); + }, }), /** diff --git a/packages/@ember/-internals/routing/tests/system/route_test.js b/packages/@ember/-internals/routing/tests/system/route_test.js index 90d5a29cd4a..870db3fbfea 100644 --- a/packages/@ember/-internals/routing/tests/system/route_test.js +++ b/packages/@ember/-internals/routing/tests/system/route_test.js @@ -3,6 +3,7 @@ import { runDestroy, buildOwner, moduleFor, AbstractTestCase } from 'internal-te import Service, { inject as injectService } from '@ember/service'; import { Object as EmberObject } from '@ember/-internals/runtime'; import EmberRoute from '../../lib/system/route'; +import { defineProperty } from '../../../metal'; let route, routeOne, routeTwo, lookupHash; @@ -53,7 +54,8 @@ moduleFor( let owner = buildOwner(ownerOptions); setOwner(route, owner); - route.set('_qp', null); + // Override the computed property by redefining it + defineProperty(route, '_qp', null, null); assert.equal(route.model({ post_id: 1 }), post); assert.equal(route.findModel('post', 1), post, '#findModel returns the correct post'); 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..a672cdd8754 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,19 +112,23 @@ 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, - }); + let a; + + expectDeprecation(() => { + a = ObservableObject.extend({ + b: computed({ + get() { + return this._b; + }, + set(key, value) { + this._b = value; + return this; + }, + }).volatile(), + }).create({ + _b: null, + }); + }, /Setting a computed property as volatile has been deprecated/); a.set('b', 'foo'); assert.equal(a.get('b'), 'foo', 'should have set the correct value for property b'); 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..7bbf03b4bfd 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,25 @@ moduleFor( assert.equal(obj1.get('name'), '1'); assert.equal(obj2.get('name'), '2'); } + + ['@test can declare dependent keys with .property()'](assert) { + let Obj; + + expectDeprecation(() => { + Obj = EmberObject.extend({ + foo: computed(function() { + return this.bar; + }).property('bar'), + }); + }, /Setting dependency keys using the `.property\(\)` modifier has been deprecated/); + + let obj = Obj.create({ bar: 1 }); + + assert.equal(obj.get('foo'), 1); + + obj.set('bar', 2); + + assert.equal(obj.get('foo'), 2); + } } ); 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..4f5a864bd40 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) { @@ -213,16 +213,60 @@ export function min(dependentKey) { hamster.get('excitingChores'); // ['CLEAN!', 'WRITE MORE UNIT TESTS!'] ``` + You can optionally pass an array of additional dependent keys as the second + parameter to the macro, if your map function relies on any external values: + + ```javascript + import { map } from '@ember/object/computed'; + import EmberObject from '@ember/object'; + + let Hamster = EmberObject.extend({ + excitingChores: map('chores', ['shouldUpperCase'], function(chore, index) { + if (this.shouldUpperCase) { + return chore.toUpperCase() + '!'; + } else { + return chore + '!'; + } + }) + }); + + let hamster = Hamster.create({ + shouldUpperCase: false, + + chores: ['clean', 'write more unit tests'] + }); + + hamster.get('excitingChores'); // ['clean!', 'write more unit tests!'] + hamster.set('shouldUpperCase', true); + hamster.get('excitingChores'); // ['CLEAN!', 'WRITE MORE UNIT TESTS!'] + ``` + @method map @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); }); } @@ -333,17 +377,59 @@ export function mapBy(dependentKey, propertyKey) { hamster.get('remainingChores'); // [] ``` + Finally, you can optionally pass an array of additional dependent keys as the + second parameter to the macro, if your filter function relies on any external + values: + + ```javascript + import { filter } from '@ember/object/computed'; + import EmberObject from '@ember/object'; + + let Hamster = EmberObject.extend({ + remainingChores: filter('chores', ['doneKey'], function(chore, index, array) { + return !chore[this.doneKey]; + }) + }); + + let hamster = Hamster.create({ + doneKey: 'finished' + + chores: [ + { name: 'cook', finished: true }, + { name: 'clean', finished: true }, + { name: 'write more unit tests', finished: false } + ] + }); + + hamster.get('remainingChores'); // [{name: 'write more unit tests', finished: false}] + ``` @method filter @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); }); } @@ -722,9 +808,13 @@ export function collect(...dependentKeys) { /** A computed property which returns a new array with all the properties from the first dependent array sorted based on a property - or sort function. + or sort function. The sort macro can be used in two different ways: + + 1. By providing a sort callback function + 2. By providing an array of keys to sort the array - The callback method you provide should have the following signature: + In the first form, the callback method you provide should have the following + signature: ```javascript function(itemA, itemB); @@ -733,12 +823,14 @@ export function collect(...dependentKeys) { - `itemA` the first item to compare. - `itemB` the second item to compare. - This function should return negative number (e.g. `-1`) when `itemA` should come before - `itemB`. It should return positive number (e.g. `1`) when `itemA` should come after - `itemB`. If the `itemA` and `itemB` are equal this function should return `0`. + This function should return negative number (e.g. `-1`) when `itemA` should + come before `itemB`. It should return positive number (e.g. `1`) when `itemA` + should come after `itemB`. If the `itemA` and `itemB` are equal this function + should return `0`. - Therefore, if this function is comparing some numeric values, simple `itemA - itemB` or - `itemA.get( 'foo' ) - itemB.get( 'foo' )` can be used instead of series of `if`. + Therefore, if this function is comparing some numeric values, simple `itemA - + itemB` or `itemA.get( 'foo' ) - itemB.get( 'foo' )` can be used instead of + series of `if`. Example @@ -747,14 +839,6 @@ export function collect(...dependentKeys) { import EmberObject from '@ember/object'; let ToDoList = EmberObject.extend({ - // using standard ascending sort - todosSorting: Object.freeze(['name']), - sortedTodos: sort('todos', 'todosSorting'), - - // using descending sort - todosSortingDesc: Object.freeze(['name:desc']), - sortedTodosDesc: sort('todos', 'todosSortingDesc'), - // using a custom sort function priorityTodos: sort('todos', function(a, b){ if (a.priority > b.priority) { @@ -767,43 +851,129 @@ export function collect(...dependentKeys) { }) }); - let todoList = ToDoList.create({todos: [ - { name: 'Unit Test', priority: 2 }, - { name: 'Documentation', priority: 3 }, - { name: 'Release', priority: 1 } - ]}); + let todoList = ToDoList.create({ + todos: [ + { name: 'Unit Test', priority: 2 }, + { name: 'Documentation', priority: 3 }, + { name: 'Release', priority: 1 } + ] + }); + + todoList.get('priorityTodos'); // [{ name:'Release', priority:1 }, { name:'Unit Test', priority:2 }, { name:'Documentation', priority:3 }] + ``` + + You can also optionally pass an array of additional dependent keys as the + second parameter, if your sort function is dependent on additional values that + could changes: + + ```js + import { sort } from '@ember/object/computed'; + import EmberObject from '@ember/object'; + + let ToDoList = EmberObject.extend({ + // using a custom sort function + sortedTodos: sort('todos', ['sortKey'] function(a, b){ + if (a[this.sortKey] > b[this.sortKey]) { + return 1; + } else if (a[this.sortKey] < b[this.sortKey]) { + return -1; + } + + return 0; + }) + }); + + let todoList = ToDoList.create({ + sortKey: 'priority', + + todos: [ + { name: 'Unit Test', priority: 2 }, + { name: 'Documentation', priority: 3 }, + { name: 'Release', priority: 1 } + ] + }); + + todoList.get('priorityTodos'); // [{ name:'Release', priority:1 }, { name:'Unit Test', priority:2 }, { name:'Documentation', priority:3 }] + ``` + + In the second form, you should provide the key of the array of sort values as + the second parameter: + + ```javascript + import { sort } from '@ember/object/computed'; + import EmberObject from '@ember/object'; + + let ToDoList = EmberObject.extend({ + // using standard ascending sort + todosSorting: Object.freeze(['name']), + sortedTodos: sort('todos', 'todosSorting'), + + // using descending sort + todosSortingDesc: Object.freeze(['name:desc']), + sortedTodosDesc: sort('todos', 'todosSortingDesc'), + }); + + let todoList = ToDoList.create({ + todos: [ + { name: 'Unit Test', priority: 2 }, + { name: 'Documentation', priority: 3 }, + { name: 'Release', priority: 1 } + ] + }); todoList.get('sortedTodos'); // [{ name:'Documentation', priority:3 }, { name:'Release', priority:1 }, { name:'Unit Test', priority:2 }] todoList.get('sortedTodosDesc'); // [{ name:'Unit Test', priority:2 }, { name:'Release', priority:1 }, { name:'Documentation', priority:3 }] - todoList.get('priorityTodos'); // [{ name:'Release', priority:1 }, { name:'Unit Test', priority:2 }, { name:'Documentation', priority:3 }] ``` @method sort @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..ec5fef112ad 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,62 @@ 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) => { + if (a.age > b.age) { + return -1; + } else if (a.age < b.age) { + return 1; + } + + return 0; + }); + + 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/); + } } );