Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove ability to override computed property #19674

Merged
merged 1 commit into from
Jul 23, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 4 additions & 28 deletions packages/@ember/-internals/metal/lib/computed.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,12 @@ import {
} from './decorator';
import expandProperties from './expand_properties';
import { addObserver, setObserverSuspended } from './observer';
import { defineProperty } from './properties';
import {
beginPropertyChanges,
endPropertyChanges,
notifyPropertyChange,
PROPERTY_DID_CHANGE,
} from './property_events';
import { set } from './property_set';

export type ComputedPropertyGetter = (keyName: string) => any;
export type ComputedPropertySetter = (keyName: string, value: any, cachedValue?: any) => any;
Expand Down Expand Up @@ -432,9 +430,10 @@ export class ComputedProperty extends ComputedDescriptor {
this._throwReadOnlyError(obj, keyName);
}

if (!this._setter) {
return this.clobberSet(obj, keyName, value);
}
assert(
`Cannot override the computed property \`${keyName}\` on ${toString(obj)}.`,
this._setter !== undefined
);

if (this._volatile) {
return this.volatileSet(obj, keyName, value);
Expand Down Expand Up @@ -501,29 +500,6 @@ export class ComputedProperty extends ComputedDescriptor {
throw new EmberError(`Cannot set read-only property "${keyName}" on object: ${inspect(obj)}`);
}

clobberSet(obj: object, keyName: string, value: any): any {
deprecate(
`The ${toString(
obj
)}#${keyName} computed property was just overridden. 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://deprecations.emberjs.com/v3.x#toc_computed-property-override',
for: 'ember-source',
since: {
enabled: '3.9.0-beta.1',
},
}
);

let cachedValue = metaFor(obj).valueFor(keyName);
defineProperty(obj, keyName, null, cachedValue);
set(obj, keyName, value);
return value;
}

volatileSet(obj: object, keyName: string, value: any): any {
return this._setter!.call(obj, keyName, value);
}
Expand Down
35 changes: 9 additions & 26 deletions packages/@ember/-internals/metal/tests/computed_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -129,20 +129,6 @@ moduleFor(
assert.equal(count, 1, 'should only call getter once');
}

['@test can override volatile computed property'](assert) {
obj = {};

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.');

expectDeprecation(() => {
set(obj, 'foo', 'boom');
}, /The \[object Object\]#foo computed property was just overridden./);

assert.equal(obj.foo, 'boom');
}

['@test defining computed property should invoke property on set'](assert) {
obj = {};
let count = 0;
Expand Down Expand Up @@ -765,7 +751,7 @@ moduleFor(
assert.ok(testObj.get('aInt') === 123, 'cp has been updated too');
}

['@test setter can be omited'](assert) {
['@test an omitted setter cannot be set later'](assert) {
let testObj = EmberObject.extend({
a: '1',
b: '2',
Expand All @@ -780,11 +766,9 @@ moduleFor(
assert.ok(testObj.get('aInt') === 1, 'getter works');
assert.ok(testObj.get('a') === '1');

expectDeprecation(() => {
expectAssertion(() => {
testObj.set('aInt', '123');
}, /The <\(unknown\):ember\d*>#aInt computed property was just overridden/);

assert.ok(testObj.get('aInt') === '123', 'cp has been updated too');
}, /Cannot override the computed property `aInt` on <\(unknown\):ember\d*>./);
}

['@test getter can be omited'](assert) {
Expand Down Expand Up @@ -979,7 +963,9 @@ moduleFor(
moduleFor(
'computed - default setter',
class extends ComputedTestCase {
async ["@test when setting a value on a computed property that doesn't handle sets"](assert) {
async ["@test raises assertion when setting a value on a computed property that doesn't handle sets"](
assert
) {
obj = {};
let observerFired = false;

Expand All @@ -993,16 +979,13 @@ moduleFor(

addObserver(obj, 'foo', null, () => (observerFired = true));

expectDeprecation(() => {
expectAssertion(() => {
set(obj, 'foo', 'bar');
}, /The \[object Object\]#foo computed property was just overridden./);

assert.equal(get(obj, 'foo'), 'bar', 'The set value is properly returned');
assert.ok(typeof obj.foo === 'string', 'The computed property was removed');
}, /Cannot override the computed property `foo` on \[object Object\]./);

await runLoopSettled();

assert.ok(observerFired, 'The observer was still notified');
assert.notOk(observerFired, 'The observer was not notified');
}
}
);
Expand Down
13 changes: 3 additions & 10 deletions packages/@ember/-internals/metal/tests/mixin/computed_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ moduleFor(
assert.ok(superSetOccurred, 'should pass set to _super after getting');
}

['@test setter behavior works properly when overriding computed properties'](assert) {
['@test setter behavior asserts when overriding computed properties'](assert) {
let obj = {};

let MixinA = Mixin.create({
Expand Down Expand Up @@ -156,16 +156,9 @@ moduleFor(
);
cpWasCalled = false;

expectDeprecation(() => {
expectAssertion(() => {
set(obj, 'cpWithoutSetter', 'test');
}, /The \[object Object\]#cpWithoutSetter computed property was just overridden./);

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');
}, /Cannot override the computed property `cpWithoutSetter` on \[object Object\]./);
}
}
);
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ moduleFor(
assert.equal(result.foo.bar, 'foo');
}

['@test does raise deprecation if descriptor is a computed property without a setter'](assert) {
['@test does raise assertion if descriptor is a computed property without a setter']() {
let owner = buildOwner();

class FooService extends Service {
Expand All @@ -282,16 +282,12 @@ moduleFor(
owner.register('foo:main', FooObject);
owner.inject('foo:main', 'foo', 'service:bar');

expectDeprecation(
/The <.*>#foo computed property was just overridden. 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./
);

expectDeprecation(
/A value was injected implicitly on the 'foo' computed property of an instance of <.*>. Implicit injection is now deprecated, please add an explicit injection for this value/
);

let result = owner.lookup('foo:main');
assert.equal(result.foo.bar, 'bar');
expectAssertion(() => {
expectDeprecation(
/A value was injected implicitly on the 'foo' computed property of an instance of <.*>. Implicit injection is now deprecated, please add an explicit injection for this value/
);
owner.lookup('foo:main');
}, /Cannot override the computed property `foo` on <.*>./);
}

['@test does not raise deprecation if descriptor is a getter and equal to the implicit deprecation'](
Expand Down