Skip to content

Commit

Permalink
[BUGFIX] Asserts when users attempt to set a shadowed value with Embe…
Browse files Browse the repository at this point in the history
…r.set
  • Loading branch information
Chris Garrett committed Oct 13, 2020
1 parent b8502c0 commit fdb1296
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 4 deletions.
19 changes: 18 additions & 1 deletion packages/@ember/-internals/metal/lib/property_set.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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;
}
Expand Down
6 changes: 3 additions & 3 deletions packages/@ember/-internals/metal/lib/tracked.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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;
}
Expand Down
16 changes: 16 additions & 0 deletions packages/@ember/-internals/metal/tests/tracked/set_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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/);
}
}
);

0 comments on commit fdb1296

Please sign in to comment.