[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

@@ -7,6 +7,7 @@ export {
   meta,
   MetaCounters,
   peekMeta,
+  PROXY_CONTENT,
   setMeta,
   UNDEFINED,
 } from './lib/meta';

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 };

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,

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);
   }
 

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];
   }

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) &&

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;

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,

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';