observed properties are enumerable when set

emberjs · Jun 11, 2018 · 04c48cb · 04c48cb
1 parent 6e3d454
commit 04c48cb
Show file tree

Hide file tree

Showing 3 changed files with 52 additions and 5 deletions.
diff --git a/packages/ember-metal/lib/property_set.ts b/packages/ember-metal/lib/property_set.ts
@@ -3,7 +3,7 @@ import { PROPERTY_BASED_DESCRIPTORS } from '@ember/deprecated-features';
 import EmberError from '@ember/error';
 import { DEBUG } from '@glimmer/env';
 import { descriptorFor, isDescriptor, Meta, meta, peekMeta } from 'ember-meta';
-import { HAS_NATIVE_PROXY, toString } from 'ember-utils';
+import { HAS_NATIVE_PROXY, lookupDescriptor, toString } from 'ember-utils';
 import { isPath } from './path_cache';
 import { Descriptor, MandatorySetterFunction } from './properties';
 import { notifyPropertyChange } from './property_events';
@@ -162,9 +162,13 @@ if (DEBUG) {
   };
 
   makeEnumerable = (obj: object, key: string) => {
-    let desc = Object.getOwnPropertyDescriptor(obj, key);
+    let desc = lookupDescriptor(obj, key);
 
-    if (desc && desc.set && (desc.set as MandatorySetterFunction).isMandatorySetter) {
+    if (
+      desc !== null &&
+      desc.set !== undefined &&
+      (desc.set as MandatorySetterFunction).isMandatorySetter
+    ) {
       desc.enumerable = true;
       Object.defineProperty(obj, key, desc);
     }

diff --git a/packages/ember-metal/tests/accessors/set_test.js b/packages/ember-metal/tests/accessors/set_test.js
@@ -1,4 +1,5 @@
-import { get, set, trySet, setHasViews } from '../..';
+import { CoreObject } from 'ember-runtime';
+import { get, set, trySet, setHasViews, observer } from '../..';
 import { moduleFor, AbstractTestCase } from 'internal-test-helpers';
 
 moduleFor(
@@ -124,5 +125,28 @@ moduleFor(
       trySet(obj, 'favoriteFood', 'hot dogs');
       assert.equal(obj.favoriteFood, undefined, 'does not set and does not error');
     }
+
+    ['@test observed properties are enumerable when set GH#14594'](assert) {
+      let callCount = 0;
+      let Test = CoreObject.extend({
+        myProp: null,
+        anotherProp: undefined,
+        didChangeMyProp: observer('myProp', function() {
+          callCount++;
+        }),
+      });
+
+      let test = Test.create();
+      set(test, 'id', '3');
+      set(test, 'myProp', { id: 1 });
+
+      assert.deepEqual(Object.keys(test).sort(), ['id', 'myProp']);
+
+      set(test, 'anotherProp', 'nice');
+
+      assert.deepEqual(Object.keys(test).sort(), ['anotherProp', 'id', 'myProp']);
+
+      assert.equal(callCount, 1);
+    }
   }
 );
diff --git a/packages/ember-metal/tests/set_properties_test.js b/packages/ember-metal/tests/set_properties_test.js
@@ -1,4 +1,5 @@
-import { setProperties } from '..';
+import { CoreObject } from 'ember-runtime';
+import { setProperties, observer } from '..';
 import { moduleFor, AbstractTestCase } from 'internal-test-helpers';
 
 moduleFor(
@@ -45,5 +46,23 @@ moduleFor(
         'Set an additional, previously unset property'
       );
     }
+
+    ['@test observed properties are enumerable when set GH#14594'](assert) {
+      let callCount = 0;
+      let Test = CoreObject.extend({
+        myProp: null,
+        anotherProp: undefined,
+        didChangeMyProp: observer('myProp', function() {
+          callCount++;
+        }),
+      });
+
+      let test = Test.create();
+      setProperties(test, { id: 3, myProp: { id: 1 } });
+
+      assert.deepEqual(Object.keys(test).sort(), ['id', 'myProp']);
+
+      assert.equal(callCount, 1);
+    }
   }
 );