From fdb1296eb573f628739194b0f0e481c66177c581 Mon Sep 17 00:00:00 2001 From: Chris Garrett Date: Tue, 13 Oct 2020 14:43:16 -0700 Subject: [PATCH] [BUGFIX] Asserts when users attempt to set a shadowed value with Ember.set --- .../-internals/metal/lib/property_set.ts | 19 ++++++++++++++++++- .../@ember/-internals/metal/lib/tracked.ts | 6 +++--- .../metal/tests/tracked/set_test.js | 16 ++++++++++++++++ 3 files changed, 37 insertions(+), 4 deletions(-) diff --git a/packages/@ember/-internals/metal/lib/property_set.ts b/packages/@ember/-internals/metal/lib/property_set.ts index aef085266ed..ee757f76a2f 100644 --- a/packages/@ember/-internals/metal/lib/property_set.ts +++ b/packages/@ember/-internals/metal/lib/property_set.ts @@ -1,4 +1,9 @@ -import { HAS_NATIVE_PROXY, setWithMandatorySetter, toString } from '@ember/-internals/utils'; +import { + HAS_NATIVE_PROXY, + lookupDescriptor, + setWithMandatorySetter, + toString, +} from '@ember/-internals/utils'; import { assert } from '@ember/debug'; import EmberError from '@ember/error'; import { DEBUG } from '@glimmer/env'; @@ -70,6 +75,18 @@ export function set(obj: object, keyName: string, value: any, tolerant?: boolean let descriptor = descriptorForProperty(obj, keyName); if (descriptor !== undefined) { + if (DEBUG) { + let instanceDesc = lookupDescriptor(obj, keyName); + let name = instanceDesc?.set?.toString(); + + 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.`, + name && (name.indexOf('CPSETTER_FUNCTION') !== -1 || name.indexOf('TRACKED_SETTER') !== -1) + ); + } + 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 e7399ead097..a0b450647e4 100644 --- a/packages/@ember/-internals/metal/lib/tracked.ts +++ b/packages/@ember/-internals/metal/lib/tracked.ts @@ -168,7 +168,7 @@ function descriptorForField([target, key, desc]: [ return value; } - function set(this: object, newValue: unknown): void { + function TRACKED_SETTER(this: object, newValue: unknown): void { setter(this, newValue); dirtyTagFor(this, SELF_TAG); } @@ -179,10 +179,10 @@ function descriptorForField([target, key, desc]: [ isTracked: true, get, - set, + set: TRACKED_SETTER, }; - metaFor(target).writeDescriptors(key, new TrackedDescriptor(get, set)); + metaFor(target).writeDescriptors(key, new TrackedDescriptor(get, TRACKED_SETTER)); return newDesc; } diff --git a/packages/@ember/-internals/metal/tests/tracked/set_test.js b/packages/@ember/-internals/metal/tests/tracked/set_test.js index 5b9eaa9a84d..dde2fb3fdc7 100644 --- a/packages/@ember/-internals/metal/tests/tracked/set_test.js +++ b/packages/@ember/-internals/metal/tests/tracked/set_test.js @@ -31,5 +31,21 @@ moduleFor( assert.equal(get(newObj, key), obj[key], 'should set value'); } } + + ['@test set should throw an error when setting on shadowed properties'](assert) { + class Obj { + @tracked value = 'emberjs'; + + constructor() { + Object.defineProperty(this, 'value', { writable: true, value: 'emberjs' }); + } + } + + let newObj = new Obj(); + + assert.throws(() => { + 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/); + } } );