From 26ea077d07cbafb4705f3488fe69b436c3d09d6d Mon Sep 17 00:00:00 2001 From: bekzod Date: Wed, 2 May 2018 10:49:50 +0500 Subject: [PATCH] [BUGFIX release] Fix descriptorFor to support fallback Ensure all fallback cases are deprecated. Fixes #16427 and add test for `notifyPropertyChange` not working for plain obj. --- packages/ember-meta/index.ts | 1 + packages/ember-meta/lib/meta.ts | 41 ++++++++++++++++--- packages/ember-metal/index.js | 2 +- packages/ember-metal/lib/property_events.js | 3 +- packages/ember-metal/lib/property_get.js | 39 ++++-------------- packages/ember-metal/lib/property_set.js | 7 +--- packages/ember-metal/tests/computed_test.js | 23 +++++++++++ .../ember-runtime/lib/system/core_object.js | 3 +- packages/ember-runtime/lib/utils.js | 3 +- 9 files changed, 75 insertions(+), 47 deletions(-) diff --git a/packages/ember-meta/index.ts b/packages/ember-meta/index.ts index 2083134cd48..c41791b9436 100644 --- a/packages/ember-meta/index.ts +++ b/packages/ember-meta/index.ts @@ -7,6 +7,7 @@ export { meta, MetaCounters, peekMeta, + PROXY_CONTENT, setMeta, UNDEFINED, } from './lib/meta'; diff --git a/packages/ember-meta/lib/meta.ts b/packages/ember-meta/lib/meta.ts index f7da8a963d3..9d30b1c71cc 100644 --- a/packages/ember-meta/lib/meta.ts +++ b/packages/ember-meta/lib/meta.ts @@ -1,8 +1,10 @@ -import { assert } from '@ember/debug'; +import { assert, deprecate } from '@ember/debug'; import { DEBUG } from '@glimmer/env'; import { Tag } from '@glimmer/reference'; import { ENV } from 'ember-environment'; -import { lookupDescriptor, symbol, toString } from 'ember-utils'; +import { HAS_NATIVE_PROXY, lookupDescriptor, symbol, toString } from 'ember-utils'; + +export const PROXY_CONTENT = symbol('PROXY_CONTENT'); export interface MetaCounters { peekCalls: number; @@ -787,10 +789,33 @@ export function descriptorFor(obj: object, keyName: string, _meta?: Meta) { ); let meta = _meta === undefined ? peekMeta(obj) : _meta; - + let descriptor; if (meta !== undefined) { - return meta.peekDescriptors(keyName); + descriptor = meta.peekDescriptors(keyName); + } + if (descriptor === undefined) { + let possibleDesc; + if (DEBUG && HAS_NATIVE_PROXY && obj[PROXY_CONTENT] !== undefined) { + possibleDesc = obj[PROXY_CONTENT][keyName]; + } else { + possibleDesc = obj[keyName]; + } + if (isDescriptor(possibleDesc)) { + deprecate( + `[DEPRECATED] computed property '${keyName}' was not set on object '${obj && + 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', + } + ); + descriptor = possibleDesc; + } } + return descriptor; } /** @@ -802,8 +827,12 @@ 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; + return ( + possibleDesc !== undefined && + possibleDesc !== null && + typeof possibleDesc === 'object' && + possibleDesc.isDescriptor === true + ); } export { counters }; diff --git a/packages/ember-metal/index.js b/packages/ember-metal/index.js index 5237a1f3820..a18e4c8640e 100644 --- a/packages/ember-metal/index.js +++ b/packages/ember-metal/index.js @@ -2,7 +2,7 @@ export { default as computed, ComputedProperty, _globalsComputed } from './lib/c export { getCacheFor, getCachedValueFor, peekCacheFor } from './lib/computed_cache'; export { default as alias } from './lib/alias'; export { deprecateProperty } from './lib/deprecate_property'; -export { PROXY_CONTENT, _getPath, get, getWithDefault } from './lib/property_get'; +export { _getPath, get, getWithDefault } from './lib/property_get'; export { set, trySet } from './lib/property_set'; export { objectAt, 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..d5f40e7ecf8 100644 --- a/packages/ember-metal/lib/property_get.js +++ b/packages/ember-metal/lib/property_get.js @@ -2,11 +2,11 @@ @module @ember/object */ import { DEBUG } from '@glimmer/env'; -import { assert, deprecate } from '@ember/debug'; -import { HAS_NATIVE_PROXY, symbol } from 'ember-utils'; +import { assert } from '@ember/debug'; +import { HAS_NATIVE_PROXY } 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, PROXY_CONTENT } from 'ember-meta'; import { getCurrentTracker } from './tracked'; import { tagForProperty } from './tags'; @@ -16,8 +16,6 @@ const ALLOWABLE_TYPES = { string: true, }; -export const PROXY_CONTENT = symbol('PROXY_CONTENT'); - export let getPossibleMandatoryProxyValue; if (DEBUG && HAS_NATIVE_PROXY) { @@ -105,34 +103,15 @@ export function get(obj, keyName) { } descriptor = descriptorFor(obj, keyName); - - if (descriptor === undefined) { - 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; - } - } - if (descriptor !== undefined) { return descriptor.get(obj, keyName); } + + if (DEBUG && HAS_NATIVE_PROXY) { + value = getPossibleMandatoryProxyValue(obj, keyName); + } else { + value = 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..694c7d46af0 100644 --- a/packages/ember-metal/lib/property_set.js +++ b/packages/ember-metal/lib/property_set.js @@ -6,7 +6,7 @@ import { getPossibleMandatoryProxyValue, _getPath as getPath } from './property_ import { notifyPropertyChange } from './property_events'; import { isPath } from './path_cache'; -import { isDescriptor, peekMeta, descriptorFor } from 'ember-meta'; +import { peekMeta, descriptorFor } from 'ember-meta'; /** @module @ember/object @@ -78,10 +78,7 @@ export function set(obj, keyName, value, tolerant) { currentValue = obj[keyName]; } - if (isDescriptor(currentValue)) { - /* computed property */ - currentValue.set(obj, keyName, value); - } else if ( + 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 2ea8635ab8a..45f8218f4e6 100644 --- a/packages/ember-metal/tests/computed_test.js +++ b/packages/ember-metal/tests/computed_test.js @@ -9,6 +9,7 @@ import { set, isWatching, addObserver, + notifyPropertyChange, } from '..'; import { meta as metaFor } from 'ember-meta'; import { moduleFor, AbstractTestCase } from 'internal-test-helpers'; @@ -60,6 +61,28 @@ moduleFor( assert.equal(count, 1, 'should have invoked computed property'); } + ['@test `notifyPropertyChange` works for plain objects #GH16427'](assert) { + let obj = { + a: 50, + b: computed(function() { + return this.a / 5; + }), + }; + + expectDeprecation(function() { + assert.equal(get(obj, 'b'), 10); + }); + + obj.a = 10; + expectDeprecation(function() { + notifyPropertyChange(obj, 'b'); + }); + + expectDeprecation(function() { + assert.equal(get(obj, 'b'), 2); + }); + } + ['@test defining computed property should invoke property on get'](assert) { let obj = {}; let count = 0; diff --git a/packages/ember-runtime/lib/system/core_object.js b/packages/ember-runtime/lib/system/core_object.js index 2bd6b2da564..a66551fab81 100644 --- a/packages/ember-runtime/lib/system/core_object.js +++ b/packages/ember-runtime/lib/system/core_object.js @@ -13,9 +13,8 @@ import { isInternalSymbol, } from 'ember-utils'; import { schedule } from '@ember/runloop'; -import { descriptorFor, meta, peekMeta, deleteMeta } from 'ember-meta'; +import { descriptorFor, meta, peekMeta, deleteMeta, PROXY_CONTENT } from 'ember-meta'; import { - PROXY_CONTENT, finishChains, sendEvent, Mixin, diff --git a/packages/ember-runtime/lib/utils.js b/packages/ember-runtime/lib/utils.js index e70263c99cc..487852b93d9 100644 --- a/packages/ember-runtime/lib/utils.js +++ b/packages/ember-runtime/lib/utils.js @@ -1,5 +1,6 @@ import { DEBUG } from '@glimmer/env'; -import { PROXY_CONTENT, get } from 'ember-metal'; +import { get } from 'ember-metal'; +import { PROXY_CONTENT } from 'ember-meta'; import { HAS_NATIVE_PROXY } from 'ember-utils'; import EmberArray, { A } from './mixins/array'; import EmberObject from './system/object';