[BUGFIX release] Fix #16427

Setup a non setup descriptor on access.
emberjs · May 18, 2018 · 166d395 · 166d395
1 parent c486d7a
commit 166d395
Show file tree

Hide file tree

Showing 5 changed files with 110 additions and 34 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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) {