From 166d3959f9a81a5c1dc8e2256fb79c19aee9fb33 Mon Sep 17 00:00:00 2001 From: Kris Selden Date: Wed, 2 May 2018 10:49:50 +0500 Subject: [PATCH] [BUGFIX release] Fix #16427 Setup a non setup descriptor on access. --- packages/ember-meta/lib/meta.ts | 9 ++- packages/ember-metal/lib/property_events.js | 3 +- packages/ember-metal/lib/property_get.js | 63 +++++++++++++-------- packages/ember-metal/lib/property_set.js | 38 +++++++++++-- packages/ember-metal/tests/computed_test.js | 31 +++++++++- 5 files changed, 110 insertions(+), 34 deletions(-) diff --git a/packages/ember-meta/lib/meta.ts b/packages/ember-meta/lib/meta.ts index f7da8a963d3..6ea8eb34c96 100644 --- a/packages/ember-meta/lib/meta.ts +++ b/packages/ember-meta/lib/meta.ts @@ -802,8 +802,13 @@ export function descriptorFor(obj: object, keyName: string, _meta?: Meta) { @private */ export function isDescriptor(possibleDesc: any | undefined | null): boolean { - // TODO make this return possibleDesc is Descriptor - return possibleDesc !== null && typeof possibleDesc === 'object' && possibleDesc.isDescriptor; + // TODO make this return `possibleDesc is Descriptor` + return ( + possibleDesc !== undefined && + possibleDesc !== null && + typeof possibleDesc === 'object' && + possibleDesc.isDescriptor === true + ); } export { counters }; diff --git a/packages/ember-metal/lib/property_events.js b/packages/ember-metal/lib/property_events.js index 3b2bb6e1e64..3e177daf040 100644 --- a/packages/ember-metal/lib/property_events.js +++ b/packages/ember-metal/lib/property_events.js @@ -86,8 +86,7 @@ function notifyPropertyChange(obj, keyName, _meta) { let possibleDesc = descriptorFor(obj, keyName, meta); - // shouldn't this mean that we're watching this key? - if (possibleDesc !== undefined && possibleDesc.didChange) { + if (possibleDesc !== undefined && typeof possibleDesc.didChange === 'function') { possibleDesc.didChange(obj, keyName); } diff --git a/packages/ember-metal/lib/property_get.js b/packages/ember-metal/lib/property_get.js index a0fb6c9225e..322e7f8462c 100644 --- a/packages/ember-metal/lib/property_get.js +++ b/packages/ember-metal/lib/property_get.js @@ -3,10 +3,10 @@ */ import { DEBUG } from '@glimmer/env'; import { assert, deprecate } from '@ember/debug'; -import { HAS_NATIVE_PROXY, symbol } from 'ember-utils'; +import { HAS_NATIVE_PROXY, symbol, toString } from 'ember-utils'; import { EMBER_METAL_TRACKED_PROPERTIES } from '@ember/canary-features'; import { isPath } from './path_cache'; -import { isDescriptor, descriptorFor } from 'ember-meta'; +import { descriptorFor, isDescriptor, meta } from 'ember-meta'; import { getCurrentTracker } from './tracked'; import { tagForProperty } from './tags'; @@ -105,33 +105,46 @@ export function get(obj, keyName) { } descriptor = descriptorFor(obj, keyName); + if (descriptor !== undefined) { + return descriptor.get(obj, keyName); + } - if (descriptor === undefined) { - if (DEBUG && HAS_NATIVE_PROXY) { - value = getPossibleMandatoryProxyValue(obj, keyName); - } else { - value = obj[keyName]; - } + if (DEBUG && HAS_NATIVE_PROXY) { + value = getPossibleMandatoryProxyValue(obj, keyName); + } else { + value = obj[keyName]; + } - if (isDescriptor(value)) { - deprecate( - `[DEPRECATED] computed property '${keyName}' was not set on object '${obj && - obj.toString && - obj.toString()}' via 'defineProperty'`, - false, - { - id: 'ember-meta.descriptor-on-object', - until: '3.5.0', - url: - 'https://emberjs.com/deprecations/v3.x#toc_use-defineProperty-to-define-computed-properties', - } - ); - descriptor = value; + // TODO turn if block into a sveltable deprecated block + if (isDescriptor(value)) { + deprecate( + `[DEPRECATED] computed property '${keyName}' was not set on object '${toString( + obj + )}' via 'defineProperty'`, + false, + { + id: 'ember-meta.descriptor-on-object', + until: '3.5.0', + url: + 'https://emberjs.com/deprecations/v3.x#toc_use-defineProperty-to-define-computed-properties', + } + ); + + Object.defineProperty(obj, keyName, { + configurable: true, + enumerable: value.enumerable === false, + get() { + return value.get(this, keyName); + }, + }); + + meta(obj).writeDescriptors(keyName, value); + + if (typeof value.setup === 'function') { + value.setup(obj, keyName); } - } - if (descriptor !== undefined) { - return descriptor.get(obj, keyName); + return value.get(obj, keyName); } } else { value = obj[keyName]; diff --git a/packages/ember-metal/lib/property_set.js b/packages/ember-metal/lib/property_set.js index 6aa7e3dca85..e53d128cda1 100644 --- a/packages/ember-metal/lib/property_set.js +++ b/packages/ember-metal/lib/property_set.js @@ -1,12 +1,12 @@ import { HAS_NATIVE_PROXY, toString } from 'ember-utils'; import EmberError from '@ember/error'; import { DEBUG } from '@glimmer/env'; -import { assert } from '@ember/debug'; +import { assert, deprecate } from '@ember/debug'; import { getPossibleMandatoryProxyValue, _getPath as getPath } from './property_get'; import { notifyPropertyChange } from './property_events'; import { isPath } from './path_cache'; -import { isDescriptor, peekMeta, descriptorFor } from 'ember-meta'; +import { meta, peekMeta, descriptorFor, isDescriptor } from 'ember-meta'; /** @module @ember/object @@ -78,10 +78,40 @@ export function set(obj, keyName, value, tolerant) { currentValue = obj[keyName]; } + // TODO turn if block into a sveltable deprecated block if (isDescriptor(currentValue)) { - /* computed property */ + deprecate( + `[DEPRECATED] computed property '${keyName}' was not set on object '${toString( + obj + )}' via 'defineProperty'`, + false, + { + id: 'ember-meta.descriptor-on-object', + until: '3.5.0', + url: + 'https://emberjs.com/deprecations/v3.x#toc_use-defineProperty-to-define-computed-properties', + } + ); + + Object.defineProperty(obj, keyName, { + configurable: true, + enumerable: currentValue.enumerable === false, + get() { + return currentValue.get(this, keyName); + }, + }); + + meta(obj).writeDescriptors(keyName, currentValue); + + if (typeof currentValue.setup === 'function') { + currentValue.setup(obj, keyName); + } + currentValue.set(obj, keyName, value); - } else if ( + return value; + } + + if ( currentValue === undefined && 'object' === typeof obj && !(keyName in obj) && diff --git a/packages/ember-metal/tests/computed_test.js b/packages/ember-metal/tests/computed_test.js index aca0ba1e713..900ae6ef84b 100644 --- a/packages/ember-metal/tests/computed_test.js +++ b/packages/ember-metal/tests/computed_test.js @@ -78,7 +78,36 @@ moduleFor( obj.a = 10; notifyPropertyChange(obj, 'b'); - assert.equal(get(obj, 'b'), 2); + assert.equal(obj.b, 2); + } + + ['@test set works for a computed property not setup using Ember.defineProperty'](assert) { + let obj = { + a: 50, + b: computed('a', { + get() { + return this.a * 2; + }, + set(_, value) { + set(this, 'a', value / 2); + return value; + }, + }), + }; + + assert.equal(obj.a, 50); + + expectDeprecation(function() { + set(obj, 'b', 80); + }); + + assert.equal(obj.b, 80); + assert.equal(obj.a, 40); + + set(obj, 'a', 100); + + assert.equal(obj.a, 100); + assert.equal(obj.b, 200); } ['@test defining computed property should invoke property on get'](assert) {