Merge pull request #19674 from nlfurniss/remove-computed-property-ove…

…rride

Remove ability to override computed property

packages/@ember/-internals/metal/lib/computed.ts

@@ -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;
@@ -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);
@@ -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);
   }

packages/@ember/-internals/metal/tests/computed_test.js

@@ -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;
@@ -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',
@@ -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) {
@@ -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;
 
@@ -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');
     }
   }
 );

packages/@ember/-internals/metal/tests/mixin/computed_test.js

@@ -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({
@@ -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\]./);
     }
   }
 );

packages/@ember/-internals/runtime/tests/system/object/create_test.js

@@ -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 {
@@ -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'](