Skip to content

Commit

Permalink
Merge pull request #17470 from pzuraq/deprecation/computed-modifiers
Browse files Browse the repository at this point in the history 
[FEAT] Implements the Computed Property Modifier deprecation RFCs
  • Loading branch information
@rwjblue
rwjblue committed Jan 17, 2019
2 parents ecf605a + 95973cf commit 52ddee8
Show file tree
Hide file tree
Showing 16 changed files with 662 additions and 256 deletions.
43 changes: 40 additions & 3 deletions packages/@ember/-internals/metal/lib/computed.ts
View file
@@ -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,
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down
43 changes: 25 additions & 18 deletions packages/@ember/-internals/metal/lib/injected_property.ts
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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);
},
};
60 changes: 35 additions & 25 deletions packages/@ember/-internals/metal/tests/computed_test.js
View file
Expand Up @@ -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');
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -438,10 +442,10 @@ moduleFor(
defineProperty(
obj,
'foo',
computed({
computed('bar', {
get: getterAndSetter,
set: getterAndSetter,
}).property('bar')
})
);
}

Expand Down Expand Up @@ -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');
Expand Down Expand Up @@ -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');
Expand All @@ -543,10 +547,10 @@ moduleFor(
defineProperty(
obj,
'foo',
computed(function() {
computed('baz', function() {
count++;
return 'baz ' + count;
}).property('baz')
})
);

assert.equal(
Expand All @@ -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');
Expand All @@ -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/);
}
Expand Down Expand Up @@ -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');

Expand Down Expand Up @@ -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');
}
}
Expand Down Expand Up @@ -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');
}

Expand Down Expand Up @@ -851,7 +859,7 @@ moduleFor(
defineProperty(
obj,
'fullName',
computed({
computed('firstName', 'lastName', {
get() {
return get(this, 'firstName') + ' ' + get(this, 'lastName');
},
Expand All @@ -861,7 +869,7 @@ moduleFor(
set(this, 'lastName', values[1]);
return value;
},
}).property('firstName', 'lastName')
})
);

let fullNameDidChange = 0;
Expand Down Expand Up @@ -900,15 +908,15 @@ moduleFor(
defineProperty(
obj,
'plusOne',
computed({
computed('foo', {
get() {
return get(this, 'foo') + 1;
},
set(key, value) {
set(this, 'foo', value);
return value + 1;
},
}).property('foo')
})
);

let plusOneDidChange = 0;
Expand Down Expand Up @@ -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');
Expand Down
5 changes: 4 additions & 1 deletion packages/@ember/-internals/metal/tests/mixin/computed_test.js
View file
Expand Up @@ -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',
Expand Down

0 comments on commit 52ddee8

Please sign in to comment.