Merge pull request #18707 from emberjs/bugfix/fix-object-proxy-tags

[BUGFIX release] Fixes tag chaining on Proxy mixins
emberjs · Jan 30, 2020 · 54072b7 · 54072b7
2 parents f357405 + 4934bc1
commit 54072b7
Show file tree

Hide file tree

Showing 4 changed files with 60 additions and 16 deletions.
diff --git a/packages/@ember/-internals/metal/lib/tags.ts b/packages/@ember/-internals/metal/lib/tags.ts
@@ -7,7 +7,6 @@ import {
   toString,
 } from '@ember/-internals/utils';
 import { assert, deprecate } from '@ember/debug';
-import { _WeakSet as WeakSet } from '@ember/polyfills';
 import { backburner } from '@ember/runloop';
 import { DEBUG } from '@glimmer/env';
 import {
@@ -55,12 +54,6 @@ export const CUSTOM_TAG_FOR = symbol('CUSTOM_TAG_FOR');
 // This is exported for `@tracked`, but should otherwise be avoided. Use `tagForObject`.
 export const SELF_TAG: string = symbol('SELF_TAG');
 
-let SEEN_TAGS: WeakSet<Tag> | undefined;
-
-if (DEBUG) {
-  SEEN_TAGS = new WeakSet();
-}
-
 export function tagForProperty(obj: unknown, propertyKey: string | symbol): Tag {
   if (!isObject(obj)) {
     return CONSTANT_TAG;
@@ -72,11 +65,10 @@ export function tagForProperty(obj: unknown, propertyKey: string | symbol): Tag
 
   let tag = tagFor(obj, propertyKey);
 
-  if (DEBUG && !SEEN_TAGS!.has(tag)) {
-    SEEN_TAGS!.add(tag);
-
-    setupMandatorySetter!(obj, propertyKey);
+  if (DEBUG) {
+    setupMandatorySetter!(tag, obj, propertyKey);
 
+    // TODO: Replace this with something more first class for tracking tags in DEBUG
     (tag as any)._propertyKey = propertyKey;
   }
 

diff --git a/packages/@ember/-internals/runtime/lib/mixins/-proxy.js b/packages/@ember/-internals/runtime/lib/mixins/-proxy.js
@@ -13,8 +13,9 @@ import {
   CUSTOM_TAG_FOR,
   getChainTagsForKey,
 } from '@ember/-internals/metal';
-import { setProxy } from '@ember/-internals/utils';
+import { setProxy, setupMandatorySetter } from '@ember/-internals/utils';
 import { assert } from '@ember/debug';
+import { DEBUG } from '@glimmer/env';
 import { combine, update, tagFor } from '@glimmer/validator';
 
 export function contentFor(proxy) {
@@ -58,10 +59,21 @@ export default Mixin.create({
   }),
 
   [CUSTOM_TAG_FOR](key) {
+    let tag = tagFor(this, key);
+
+    if (DEBUG) {
+      // TODO: Replace this with something more first class for tracking tags in DEBUG
+      tag._propertyKey = key;
+    }
+
     if (key in this) {
-      return tagFor(this, key);
+      if (DEBUG) {
+        setupMandatorySetter(tag, this, key);
+      }
+
+      return tag;
     } else {
-      return combine(getChainTagsForKey(this, `content.${key}`));
+      return combine([tag, ...getChainTagsForKey(this, `content.${key}`)]);
     }
   },
 

diff --git a/packages/@ember/-internals/runtime/tests/system/object_proxy_test.js b/packages/@ember/-internals/runtime/tests/system/object_proxy_test.js
@@ -338,5 +338,33 @@ moduleFor(
         observe: observer('foo', function() {}),
       }).create();
     }
+
+    async '@test custom proxies should be able to notify property changes manually'(assert) {
+      let proxy = ObjectProxy.extend({
+        locals: { foo: 123 },
+
+        unknownProperty(key) {
+          return this.locals[key];
+        },
+
+        setUnknownProperty(key, value) {
+          this.locals[key] = value;
+          this.notifyPropertyChange(key);
+        },
+      }).create();
+
+      let count = 0;
+
+      proxy.addObserver('foo', function() {
+        count++;
+      });
+
+      proxy.set('foo', 456);
+      await runLoopSettled();
+
+      assert.equal(count, 1);
+      assert.equal(proxy.get('foo'), 456);
+      assert.equal(proxy.get('locals.foo'), 456);
+    }
   }
 );
diff --git a/packages/@ember/-internals/utils/lib/mandatory-setter.ts b/packages/@ember/-internals/utils/lib/mandatory-setter.ts
@@ -1,8 +1,12 @@
 import { assert } from '@ember/debug';
+import { _WeakSet as WeakSet } from '@ember/polyfills';
 import { DEBUG } from '@glimmer/env';
+import { Tag } from '@glimmer/validator';
 import lookupDescriptor from './lookup-descriptor';
 
-export let setupMandatorySetter: ((obj: object, keyName: string | symbol) => void) | undefined;
+export let setupMandatorySetter:
+  | ((tag: Tag, obj: object, keyName: string | symbol) => void)
+  | undefined;
 export let teardownMandatorySetter: ((obj: object, keyName: string | symbol) => void) | undefined;
 export let setWithMandatorySetter:
   | ((obj: object, keyName: string | symbol, value: any) => void)
@@ -11,6 +15,8 @@ export let setWithMandatorySetter:
 type PropertyDescriptorWithMeta = PropertyDescriptor & { hadOwnProperty?: boolean };
 
 if (DEBUG) {
+  let SEEN_TAGS = new WeakSet();
+
   let MANDATORY_SETTERS: WeakMap<
     object,
     // @ts-ignore
@@ -21,7 +27,13 @@ if (DEBUG) {
     return Object.prototype.propertyIsEnumerable.call(obj, key);
   };
 
-  setupMandatorySetter = function(obj: object, keyName: string | symbol) {
+  setupMandatorySetter = function(tag: Tag, obj: object, keyName: string | symbol) {
+    if (SEEN_TAGS.has(tag)) {
+      return;
+    }
+
+    SEEN_TAGS!.add(tag);
+
     let desc = (lookupDescriptor(obj, keyName) as PropertyDescriptorWithMeta) || {};
 
     if (desc.get || desc.set) {