[BUGFIX] Updates to fix the CUSTOM_TAG_FOR enumerability bug

Updates to the latest Glimmer VM, using the WeakMap based custom tag system instead of the symbol based one.
emberjs · Dec 5, 2020 · 25d99fa · 25d99fa
1 parent bcb0a49
commit 25d99fa
Show file tree

Hide file tree

Showing 3 changed files with 58 additions and 47 deletions.
diff --git a/packages/@ember/-internals/metal/lib/tags.ts b/packages/@ember/-internals/metal/lib/tags.ts
@@ -2,11 +2,17 @@ import { isObject, setupMandatorySetter, symbol, toString } from '@ember/-intern
 import { assert } from '@ember/debug';
 import { isDestroyed } from '@glimmer/destroyable';
 import { DEBUG } from '@glimmer/env';
-import { CUSTOM_TAG_FOR } from '@glimmer/manager';
+import { getCustomTagFor } from '@glimmer/manager';
 import { CONSTANT_TAG, dirtyTagFor, Tag, tagFor, TagMeta } from '@glimmer/validator';
 
 /////////
 
+type CustomTagFnWithMandatorySetter = (
+  obj: object,
+  key: string | symbol,
+  addMandatorySetter: boolean
+) => Tag;
+
 // This is exported for `@tracked`, but should otherwise be avoided. Use `tagForObject`.
 export const SELF_TAG: string | symbol = symbol('SELF_TAG');
 
@@ -16,8 +22,10 @@ export function tagForProperty(
   addMandatorySetter = false,
   meta?: TagMeta
 ): Tag {
-  if (typeof obj[CUSTOM_TAG_FOR] === 'function') {
-    return obj[CUSTOM_TAG_FOR](propertyKey, addMandatorySetter);
+  let customTagFor = getCustomTagFor(obj);
+
+  if (customTagFor !== undefined) {
+    return (customTagFor as CustomTagFnWithMandatorySetter)(obj, propertyKey, addMandatorySetter);
   }
 
   let tag = tagFor(obj, propertyKey, meta);

diff --git a/packages/@ember/-internals/runtime/lib/mixins/-proxy.js b/packages/@ember/-internals/runtime/lib/mixins/-proxy.js
@@ -15,7 +15,7 @@ import {
 import { setProxy, setupMandatorySetter, isObject } from '@ember/-internals/utils';
 import { assert } from '@ember/debug';
 import { DEBUG } from '@glimmer/env';
-import { CUSTOM_TAG_FOR } from '@glimmer/manager';
+import { setCustomTagFor } from '@glimmer/manager';
 import { combine, updateTag, tagFor, tagMetaFor } from '@glimmer/validator';
 
 export function contentFor(proxy) {
@@ -24,6 +24,34 @@ export function contentFor(proxy) {
   return content;
 }
 
+function customTagForProxy(proxy, key, addMandatorySetter) {
+  let meta = tagMetaFor(proxy);
+  let tag = tagFor(proxy, key, meta);
+
+  if (DEBUG) {
+    // TODO: Replace this with something more first class for tracking tags in DEBUG
+    tag._propertyKey = key;
+  }
+
+  if (key in proxy) {
+    if (DEBUG && addMandatorySetter) {
+      setupMandatorySetter(tag, proxy, key);
+    }
+
+    return tag;
+  } else {
+    let tags = [tag, tagFor(proxy, 'content', meta)];
+
+    let content = contentFor(proxy);
+
+    if (isObject(content)) {
+      tags.push(tagForProperty(content, key, addMandatorySetter));
+    }
+
+    return combine(tags);
+  }
+}
+
 /**
   `Ember.ProxyMixin` forwards all properties not defined by the proxy itself
   to a proxied `content` object.  See ObjectProxy for more details.
@@ -47,6 +75,7 @@ export default Mixin.create({
     this._super(...arguments);
     setProxy(this);
     tagForObject(this);
+    setCustomTagFor(this, customTagForProxy);
   },
 
   willDestroy() {
@@ -58,34 +87,6 @@ export default Mixin.create({
     return Boolean(get(this, 'content'));
   }),
 
-  [CUSTOM_TAG_FOR](key, addMandatorySetter) {
-    let meta = tagMetaFor(this);
-    let tag = tagFor(this, key, meta);
-
-    if (DEBUG) {
-      // TODO: Replace this with something more first class for tracking tags in DEBUG
-      tag._propertyKey = key;
-    }
-
-    if (key in this) {
-      if (DEBUG && addMandatorySetter) {
-        setupMandatorySetter(tag, this, key);
-      }
-
-      return tag;
-    } else {
-      let tags = [tag, tagFor(this, 'content', meta)];
-
-      let content = contentFor(this);
-
-      if (isObject(content)) {
-        tags.push(tagForProperty(content, key, addMandatorySetter));
-      }
-
-      return combine(tags);
-    }
-  },
-
   unknownProperty(key) {
     let content = contentFor(this);
     if (content) {

diff --git a/packages/@ember/-internals/runtime/lib/system/array_proxy.js b/packages/@ember/-internals/runtime/lib/system/array_proxy.js
@@ -17,14 +17,28 @@ import { isObject } from '@ember/-internals/utils';
 import EmberObject from './object';
 import { isArray, MutableArray } from '../mixins/array';
 import { assert } from '@ember/debug';
-import { CUSTOM_TAG_FOR } from '@glimmer/manager';
+import { setCustomTagFor } from '@glimmer/manager';
 import { combine, consumeTag, validateTag, valueForTag, tagFor } from '@glimmer/validator';
 
 const ARRAY_OBSERVER_MAPPING = {
   willChange: '_arrangedContentArrayWillChange',
   didChange: '_arrangedContentArrayDidChange',
 };
 
+function customTagForArrayProxy(proxy, key) {
+  if (key === '[]') {
+    proxy._revalidate();
+
+    return proxy._arrTag;
+  } else if (key === 'length') {
+    proxy._revalidate();
+
+    return proxy._lengthTag;
+  }
+
+  return tagFor(proxy, key);
+}
+
 /**
   An ArrayProxy wraps any other object that implements `Array` and/or
   `MutableArray,` forwarding all requests. This makes it very useful for
@@ -111,26 +125,14 @@ export default class ArrayProxy extends EmberObject {
     this._arrangedContentRevision = null;
     this._lengthTag = null;
     this._arrTag = null;
+
+    setCustomTagFor(this, customTagForArrayProxy);
   }
 
   [PROPERTY_DID_CHANGE]() {
     this._revalidate();
   }
 
-  [CUSTOM_TAG_FOR](key) {
-    if (key === '[]') {
-      this._revalidate();
-
-      return this._arrTag;
-    } else if (key === 'length') {
-      this._revalidate();
-
-      return this._lengthTag;
-    }
-
-    return tagFor(this, key);
-  }
-
   willDestroy() {
     this._removeArrangedContentArrayObserver();
   }