diff --git a/packages/@ember/-internals/metal/lib/computed.ts b/packages/@ember/-internals/metal/lib/computed.ts index 08405fd2c81..3d7a14cd9e6 100644 --- a/packages/@ember/-internals/metal/lib/computed.ts +++ b/packages/@ember/-internals/metal/lib/computed.ts @@ -630,6 +630,8 @@ export class ComputedProperty extends ComputedDescriptor { } finally { endPropertyChanges(); } + + return ret; } else { return this.setWithSuspend(obj, keyName, value); } diff --git a/packages/@ember/-internals/metal/lib/decorator.ts b/packages/@ember/-internals/metal/lib/decorator.ts index a130691606f..9f7bafd17aa 100644 --- a/packages/@ember/-internals/metal/lib/decorator.ts +++ b/packages/@ember/-internals/metal/lib/decorator.ts @@ -1,6 +1,7 @@ import { Meta, meta as metaFor } from '@ember/-internals/meta'; import { EMBER_METAL_TRACKED_PROPERTIES } from '@ember/canary-features'; import { assert } from '@ember/debug'; +import { _WeakSet as WeakSet } from '@ember/polyfills'; import { setClassicDecorator } from './descriptor_map'; import { unwatch, watch } from './watching'; @@ -140,11 +141,17 @@ function DESCRIPTOR_SETTER_FUNCTION( name: string, descriptor: ComputedDescriptor ): (value: any) => void { - return function CPSETTER_FUNCTION(this: object, value: any): void { + let func = function CPSETTER_FUNCTION(this: object, value: any): void { return descriptor.set(this, name, value); }; + + CP_SETTER_FUNCS.add(func); + + return func; } +export const CP_SETTER_FUNCS = new WeakSet(); + export function makeComputedDecorator( desc: ComputedDescriptor, DecoratorClass: { prototype: object } diff --git a/packages/@ember/-internals/metal/lib/property_get.ts b/packages/@ember/-internals/metal/lib/property_get.ts index e0b6f37ef0b..0251d4f4670 100644 --- a/packages/@ember/-internals/metal/lib/property_get.ts +++ b/packages/@ember/-internals/metal/lib/property_get.ts @@ -111,9 +111,11 @@ export function get(obj: object, keyName: string): any { } } - let descriptor = descriptorForProperty(obj, keyName); - if (descriptor !== undefined) { - return descriptor.get(obj, keyName); + if (!EMBER_METAL_TRACKED_PROPERTIES) { + let descriptor = descriptorForProperty(obj, keyName); + if (descriptor !== undefined) { + return descriptor.get(obj, keyName); + } } if (DEBUG && HAS_NATIVE_PROXY) { diff --git a/packages/@ember/-internals/metal/lib/property_set.ts b/packages/@ember/-internals/metal/lib/property_set.ts index 2b21a846296..909128a7a2b 100644 --- a/packages/@ember/-internals/metal/lib/property_set.ts +++ b/packages/@ember/-internals/metal/lib/property_set.ts @@ -9,6 +9,7 @@ import { EMBER_METAL_TRACKED_PROPERTIES } from '@ember/canary-features'; import { assert } from '@ember/debug'; import EmberError from '@ember/error'; import { DEBUG } from '@glimmer/env'; +import { CP_SETTER_FUNCS } from './decorator'; import { descriptorForProperty } from './descriptor_map'; import { isPath } from './path_cache'; import { MandatorySetterFunction } from './properties'; @@ -85,11 +86,22 @@ export function set(obj: object, keyName: string, value: any, tolerant?: boolean } let meta = peekMeta(obj); - let descriptor = descriptorForProperty(obj, keyName, meta); - if (descriptor !== undefined) { - descriptor.set(obj, keyName, value); - return value; + if (!EMBER_METAL_TRACKED_PROPERTIES) { + let descriptor = descriptorForProperty(obj, keyName, meta); + + if (descriptor !== undefined) { + descriptor.set(obj, keyName, value); + return value; + } + } else { + let descriptor = lookupDescriptor(obj, keyName); + let setter = descriptor === null ? undefined : descriptor.set; + + if (setter !== undefined && CP_SETTER_FUNCS.has(setter)) { + obj[keyName] = value; + return value; + } } let currentValue: any; diff --git a/packages/@ember/-internals/metal/tests/accessors/get_test.js b/packages/@ember/-internals/metal/tests/accessors/get_test.js index f121499e91d..b3ce67788b3 100644 --- a/packages/@ember/-internals/metal/tests/accessors/get_test.js +++ b/packages/@ember/-internals/metal/tests/accessors/get_test.js @@ -1,6 +1,6 @@ import { ENV } from '@ember/-internals/environment'; import { Object as EmberObject } from '@ember/-internals/runtime'; -import { get, getWithDefault, Mixin, observer } from '../..'; +import { get, getWithDefault, Mixin, observer, computed } from '../..'; import { moduleFor, AbstractTestCase } from 'internal-test-helpers'; import { run } from '@ember/runloop'; @@ -313,5 +313,50 @@ moduleFor( 'should return the set value, not false' ); } + + ['@test should respect prototypical inheritance when subclasses override CPs'](assert) { + let ParentClass = EmberObject.extend({ + prop: computed({ + get() { + assert.ok(false, 'incorrect getter called'); + return 123; + }, + }), + }); + + let SubClass = ParentClass.extend({ + get prop() { + assert.ok(true, 'correct getter called'); + return 456; + }, + }); + + let instance = SubClass.create(); + + instance.prop; + } + + ['@test should respect prototypical inheritance when subclasses override CPs with native classes']( + assert + ) { + class ParentClass extends EmberObject { + @computed + get prop() { + assert.ok(false, 'incorrect getter called'); + return 123; + } + } + + class SubClass extends ParentClass { + get prop() { + assert.ok(true, 'correct getter called'); + return 456; + } + } + + let instance = SubClass.create(); + + instance.prop; + } } ); diff --git a/packages/@ember/-internals/metal/tests/accessors/set_test.js b/packages/@ember/-internals/metal/tests/accessors/set_test.js index d10006cd246..4b80cbef6ff 100644 --- a/packages/@ember/-internals/metal/tests/accessors/set_test.js +++ b/packages/@ember/-internals/metal/tests/accessors/set_test.js @@ -1,4 +1,5 @@ -import { get, set, trySet } from '../..'; +import { Object as EmberObject } from '@ember/-internals/runtime'; +import { get, set, trySet, computed } from '../..'; import { moduleFor, AbstractTestCase } from 'internal-test-helpers'; moduleFor( @@ -142,5 +143,50 @@ moduleFor( assert.equal(count, 1, 'should have native setter'); assert.equal(get(obj, 'foo'), 'computed bar', 'should return new value'); } + + ['@test should respect prototypical inheritance when subclasses override CPs'](assert) { + let ParentClass = EmberObject.extend({ + prop: computed({ + set(key, val) { + assert.ok(false, 'incorrect setter called'); + this._val = val; + }, + }), + }); + + let SubClass = ParentClass.extend({ + set prop(val) { + assert.ok(true, 'correct setter called'); + this._val = val; + }, + }); + + let instance = SubClass.create(); + + instance.prop = 123; + } + + ['@test should respect prototypical inheritance when subclasses override CPs with native classes']( + assert + ) { + class ParentClass extends EmberObject { + @computed + set prop(val) { + assert.ok(false, 'incorrect setter called'); + this._val = val; + } + } + + class SubClass extends ParentClass { + set prop(val) { + assert.ok(true, 'correct setter called'); + this._val = val; + } + } + + let instance = SubClass.create(); + + instance.prop = 123; + } } ); diff --git a/packages/@ember/-internals/utils/lib/lookup-descriptor.ts b/packages/@ember/-internals/utils/lib/lookup-descriptor.ts index c450b34fa49..395dea6ebde 100644 --- a/packages/@ember/-internals/utils/lib/lookup-descriptor.ts +++ b/packages/@ember/-internals/utils/lib/lookup-descriptor.ts @@ -1,4 +1,4 @@ -export default function lookupDescriptor(obj: object, keyName: string) { +export default function lookupDescriptor(obj: object, keyName: string | symbol) { let current: object | null = obj; do { let descriptor = Object.getOwnPropertyDescriptor(current, keyName); diff --git a/packages/@ember/-internals/utils/lib/mandatory-setter.ts b/packages/@ember/-internals/utils/lib/mandatory-setter.ts index b9ce32bdc64..7b856eedb73 100644 --- a/packages/@ember/-internals/utils/lib/mandatory-setter.ts +++ b/packages/@ember/-internals/utils/lib/mandatory-setter.ts @@ -1,6 +1,7 @@ import { EMBER_METAL_TRACKED_PROPERTIES } from '@ember/canary-features'; import { assert } from '@ember/debug'; import { DEBUG } from '@glimmer/env'; +import lookupDescriptor from './lookup-descriptor'; export let setupMandatorySetter: ((obj: object, keyName: string | symbol) => void) | undefined; export let teardownMandatorySetter: ((obj: object, keyName: string | symbol) => void) | undefined; @@ -17,31 +18,12 @@ if (DEBUG && EMBER_METAL_TRACKED_PROPERTIES) { { [key: string | symbol]: PropertyDescriptorWithMeta } > = new WeakMap(); - let getPropertyDescriptor = function( - obj: object, - keyName: string | symbol - ): PropertyDescriptorWithMeta | undefined { - let current = obj; - - while (current !== null) { - let desc = Object.getOwnPropertyDescriptor(current, keyName); - - if (desc !== undefined) { - return desc; - } - - current = Object.getPrototypeOf(current); - } - - return; - }; - let propertyIsEnumerable = function(obj: object, key: string | symbol) { return Object.prototype.propertyIsEnumerable.call(obj, key); }; setupMandatorySetter = function(obj: object, keyName: string | symbol) { - let desc = getPropertyDescriptor(obj, keyName) || {}; + let desc = (lookupDescriptor(obj, keyName) as PropertyDescriptorWithMeta) || {}; if (desc.get || desc.set) { // if it has a getter or setter, we can't install the mandatory setter. @@ -113,7 +95,7 @@ if (DEBUG && EMBER_METAL_TRACKED_PROPERTIES) { // If the object didn't have own property before, it would have changed // the enumerability after setting the value the first time. if (!setter.hadOwnProperty) { - let desc = getPropertyDescriptor(obj, keyName); + let desc = lookupDescriptor(obj, keyName); desc!.enumerable = true; Object.defineProperty(obj, keyName, desc!);