Merge pull request #19232 from emberjs/bugfix/restore-previous-set-be…

…havior [BUGFIX beta] Restores the shadowed property set behavior
emberjs · Oct 28, 2020 · 8635659 · 8635659
2 parents b6c8a37 + 5e7e543
commit 8635659
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 38 deletions.
diff --git a/packages/@ember/-internals/metal/lib/decorator.ts b/packages/@ember/-internals/metal/lib/decorator.ts
@@ -71,12 +71,10 @@ export abstract class ComputedDescriptor {
   abstract set(obj: object, keyName: string, value: any | null | undefined): any | null | undefined;
 }
 
-export let CPGETTERS: WeakSet<() => unknown>;
-export let CPSETTERS: WeakSet<(value: unknown) => void>;
+export let COMPUTED_GETTERS: WeakSet<() => unknown>;
 
 if (DEBUG) {
-  CPGETTERS = new WeakSet();
-  CPSETTERS = new WeakSet();
+  COMPUTED_GETTERS = new WeakSet();
 }
 
 function DESCRIPTOR_GETTER_FUNCTION(name: string, descriptor: ComputedDescriptor): () => unknown {
@@ -85,7 +83,7 @@ function DESCRIPTOR_GETTER_FUNCTION(name: string, descriptor: ComputedDescriptor
   }
 
   if (DEBUG) {
-    CPGETTERS.add(getter);
+    COMPUTED_GETTERS.add(getter);
   }
 
   return getter;
@@ -94,18 +92,18 @@ function DESCRIPTOR_GETTER_FUNCTION(name: string, descriptor: ComputedDescriptor
 function DESCRIPTOR_SETTER_FUNCTION(
   name: string,
   descriptor: ComputedDescriptor
-): (value: unknown) => void {
-  function setter(this: object, value: unknown): void {
+): (value: any) => void {
+  let set = function CPSETTER_FUNCTION(this: object, value: any): void {
     return descriptor.set(this, name, value);
-  }
+  };
 
-  if (DEBUG) {
-    CPSETTERS.add(setter);
-  }
+  COMPUTED_SETTERS.add(set);
 
-  return setter;
+  return set;
 }
 
+export const COMPUTED_SETTERS = new WeakSet();
+
 export function makeComputedDecorator(
   desc: ComputedDescriptor,
   DecoratorClass: { prototype: object }
@@ -119,7 +117,10 @@ export function makeComputedDecorator(
   ): DecoratorPropertyDescriptor {
     assert(
       `Only one computed property decorator can be applied to a class field or accessor, but '${key}' was decorated twice. You may have added the decorator to both a getter and setter, which is unnecessary.`,
-      isClassicDecorator || !propertyDesc || !propertyDesc.get || !CPGETTERS.has(propertyDesc.get)
+      isClassicDecorator ||
+        !propertyDesc ||
+        !propertyDesc.get ||
+        !COMPUTED_GETTERS.has(propertyDesc.get)
     );
 
     let meta = arguments.length === 3 ? metaFor(target) : maybeMeta;

diff --git a/packages/@ember/-internals/metal/lib/property_set.ts b/packages/@ember/-internals/metal/lib/property_set.ts
@@ -7,7 +7,7 @@ import {
 import { assert } from '@ember/debug';
 import EmberError from '@ember/error';
 import { DEBUG } from '@glimmer/env';
-import { CPSETTERS, descriptorForProperty } from './decorator';
+import { COMPUTED_SETTERS } from './decorator';
 import { isPath } from './path_cache';
 import { notifyPropertyChange } from './property_events';
 import { _getPath as getPath, getPossibleMandatoryProxyValue } from './property_get';
@@ -72,21 +72,10 @@ export function set<T = unknown>(obj: object, keyName: string, value: T, toleran
     return setPath(obj, keyName, value, tolerant);
   }
 
-  let descriptor = descriptorForProperty(obj, keyName);
+  let descriptor = lookupDescriptor(obj, keyName);
 
-  if (descriptor !== undefined) {
-    if (DEBUG) {
-      let instanceDesc = lookupDescriptor(obj, keyName);
-
-      assert(
-        `Attempted to set \`${toString(
-          obj
-        )}.${keyName}\` using Ember.set(), but the property was a computed or tracked property that was shadowed by another property declaration. This can happen if you defined a tracked or computed property on a parent class, and then redefined it on a subclass.`,
-        instanceDesc && instanceDesc.set && CPSETTERS.has(instanceDesc.set)
-      );
-    }
-
-    descriptor.set(obj, keyName, value);
+  if (descriptor !== null && COMPUTED_SETTERS.has(descriptor.set!)) {
+    obj[keyName] = value;
     return value;
   }
 

diff --git a/packages/@ember/-internals/metal/lib/tracked.ts b/packages/@ember/-internals/metal/lib/tracked.ts
@@ -5,8 +5,7 @@ import { DEBUG } from '@glimmer/env';
 import { consumeTag, dirtyTagFor, tagFor, trackedData } from '@glimmer/validator';
 import { CHAIN_PASS_THROUGH } from './chain-tags';
 import {
-  CPGETTERS,
-  CPSETTERS,
+  COMPUTED_SETTERS,
   Decorator,
   DecoratorPropertyDescriptor,
   isElementDescriptor,
@@ -184,10 +183,7 @@ function descriptorForField([target, key, desc]: [
     set,
   };
 
-  if (DEBUG) {
-    CPGETTERS.add(get);
-    CPSETTERS.add(set);
-  }
+  COMPUTED_SETTERS.add(set);
 
   metaFor(target).writeDescriptors(key, new TrackedDescriptor(get, set));
 

diff --git a/packages/@ember/-internals/metal/tests/tracked/set_test.js b/packages/@ember/-internals/metal/tests/tracked/set_test.js
@@ -32,7 +32,7 @@ moduleFor(
       }
     }
 
-    ['@test set should throw an error when setting on shadowed properties']() {
+    ['@test set should not throw an error when setting on shadowed properties'](assert) {
       class Obj {
         @tracked value = 'emberjs';
 
@@ -43,9 +43,9 @@ moduleFor(
 
       let newObj = new Obj();
 
-      expectAssertion(() => {
-        set(newObj, 'value', 123);
-      }, /Attempted to set `\[object Object\].value` using Ember.set\(\), but the property was a computed or tracked property that was shadowed by another property declaration. This can happen if you defined a tracked or computed property on a parent class, and then redefined it on a subclass/);
+      set(newObj, 'value', 123);
+
+      assert.equal(newObj.value, 123, 'it worked');
     }
   }
 );