From 5e7e54347488a7c2f25c3bc39256affb0ba52048 Mon Sep 17 00:00:00 2001 From: Chris Garrett Date: Tue, 13 Oct 2020 14:36:21 -0700 Subject: [PATCH] [BUGFIX lts] Restores the shadowed property set behavior Restores the previous behavior that would happen when setting a shadowed property. --- .../@ember/-internals/metal/lib/decorator.ts | 27 ++++++++++--------- .../-internals/metal/lib/property_set.ts | 19 +++---------- .../@ember/-internals/metal/lib/tracked.ts | 8 ++---- .../metal/tests/tracked/set_test.js | 8 +++--- 4 files changed, 24 insertions(+), 38 deletions(-) diff --git a/packages/@ember/-internals/metal/lib/decorator.ts b/packages/@ember/-internals/metal/lib/decorator.ts index bd172ec2271..29e7b338ed1 100644 --- 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 index 3de6304a8bb..e78e2b22037 100644 --- 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(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 index 9469a6110d7..6a375adacf7 100644 --- 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 index 6bed7598dea..389bcf0ec6a 100644 --- 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'); } } );