[BUGFIX release] Ensures the arg proxy works with get

Previously, the arg proxy used a custom system for passing along its
`capturedArgs` directly to `getChainTags`, in order to interoperate with
computed properties. As it turns out, there are a number of other places
where the correct tag needs to be gotten and setup.

This PR replaces the `UNKNOWN_PROPERTY_TAG` system with a more general
`CUSTOM_TAG_FOR` system. In this system, if we detect that an object has
this method, we defer to it to get the tag, even if the property was
defined on the object. There are two users of this system:

- ProxyMixin
- The arg proxy

Given that it's private, and we have relatively few use cases, I believe
this is the cleanest solution at the moment. The alternative would be to
keep `UNKNOWN_PROPERTY_TAG` and also add `CUSTOM_TAG_FOR`, but that
seems unnecessary given low usage.

Author: Chris Garrett

packages/@ember/-internals/glimmer/lib/component-managers/custom.ts

@@ -1,4 +1,4 @@
-import { ARGS_PROXY_TAGS, consume } from '@ember/-internals/metal';
+import { consume, CUSTOM_TAG_FOR } from '@ember/-internals/metal';
 import { Factory } from '@ember/-internals/owner';
 import { HAS_NATIVE_PROXY } from '@ember/-internals/utils';
 import { OwnedTemplateMeta } from '@ember/-internals/views';
@@ -192,34 +192,41 @@ export default class CustomComponentManager<ComponentInstance>
   ): CustomComponentState<ComponentInstance> {
     const { delegate } = definition;
     const capturedArgs = args.capture();
+    const namedArgs = capturedArgs.named;
 
     let value;
     let namedArgsProxy = {};
 
     if (EMBER_CUSTOM_COMPONENT_ARG_PROXY) {
+      let getTag = (key: string) => {
+        return namedArgs.get(key).tag;
+      };
+
       if (HAS_NATIVE_PROXY) {
         let handler: ProxyHandler<{}> = {
           get(_target, prop) {
-            if (capturedArgs.named.has(prop as string)) {
-              let ref = capturedArgs.named.get(prop as string);
+            if (namedArgs.has(prop as string)) {
+              let ref = namedArgs.get(prop as string);
               consume(ref.tag);
 
               return ref.value();
+            } else if (prop === CUSTOM_TAG_FOR) {
+              return getTag;
             }
           },
 
           has(_target, prop) {
-            return capturedArgs.named.has(prop as string);
+            return namedArgs.has(prop as string);
           },
 
           ownKeys(_target) {
-            return capturedArgs.named.names;
+            return namedArgs.names;
           },
 
           getOwnPropertyDescriptor(_target, prop) {
             assert(
               'args proxies do not have real property descriptors, so you should never need to call getOwnPropertyDescriptor yourself. This code exists for enumerability, such as in for-in loops and Object.keys()',
-              capturedArgs.named.has(prop as string)
+              namedArgs.has(prop as string)
             );
 
             return {
@@ -243,12 +250,18 @@ export default class CustomComponentManager<ComponentInstance>
 
         namedArgsProxy = new Proxy(namedArgsProxy, handler);
       } else {
-        capturedArgs.named.names.forEach(name => {
+        Object.defineProperty(namedArgsProxy, CUSTOM_TAG_FOR, {
+          configurable: false,
+          enumerable: false,
+          value: getTag,
+        });
+
+        namedArgs.names.forEach(name => {
           Object.defineProperty(namedArgsProxy, name, {
             enumerable: true,
             configurable: true,
             get() {
-              let ref = capturedArgs.named.get(name);
+              let ref = namedArgs.get(name);
               consume(ref.tag);
 
               return ref.value();
@@ -257,8 +270,6 @@ export default class CustomComponentManager<ComponentInstance>
         });
       }
 
-      ARGS_PROXY_TAGS.set(namedArgsProxy, capturedArgs.named);
-
       value = {
         named: namedArgsProxy,
         positional: capturedArgs.positional.value(),

packages/@ember/-internals/glimmer/tests/integration/components/tracked-test.js

@@ -1,6 +1,6 @@
 import { Object as EmberObject, A, ArrayProxy, PromiseProxyMixin } from '@ember/-internals/runtime';
 import { EMBER_CUSTOM_COMPONENT_ARG_PROXY } from '@ember/canary-features';
-import { computed, tracked, nativeDescDecorator as descriptor } from '@ember/-internals/metal';
+import { computed, get, tracked, nativeDescDecorator as descriptor } from '@ember/-internals/metal';
 import { Promise } from 'rsvp';
 import { moduleFor, RenderingTestCase, strip, runTask } from 'internal-test-helpers';
 import GlimmerishComponent from '../../utils/glimmerish-component';
@@ -568,6 +568,50 @@ if (EMBER_CUSTOM_COMPONENT_ARG_PROXY) {
         this.assertText('hello!');
       }
 
+      '@test args can be accessed with get()'() {
+        class TestComponent extends GlimmerishComponent {
+          get text() {
+            return get(this, 'args.text');
+          }
+        }
+
+        this.registerComponent('test', {
+          ComponentClass: TestComponent,
+          template: '<p>{{this.text}}</p>',
+        });
+
+        this.render('<Test @text={{this.text}}/>', {
+          text: 'hello!',
+        });
+
+        this.assertText('hello!');
+
+        runTask(() => this.context.set('text', 'hello world!'));
+        this.assertText('hello world!');
+
+        runTask(() => this.context.set('text', 'hello!'));
+        this.assertText('hello!');
+      }
+
+      '@test args can be accessed with get() if no value is passed'() {
+        class TestComponent extends GlimmerishComponent {
+          get text() {
+            return get(this, 'args.text') || 'hello!';
+          }
+        }
+
+        this.registerComponent('test', {
+          ComponentClass: TestComponent,
+          template: '<p>{{this.text}}</p>',
+        });
+
+        this.render('<Test/>', {
+          text: 'hello!',
+        });
+
+        this.assertText('hello!');
+      }
+
       '@test named args are enumerable'() {
         class TestComponent extends GlimmerishComponent {
           get objectKeys() {

packages/@ember/-internals/glimmer/tests/integration/helpers/get-test.js

@@ -3,6 +3,7 @@ import { RenderingTestCase, moduleFor, runTask } from 'internal-test-helpers';
 import { set, get } from '@ember/-internals/metal';
 
 import { Component } from '../../utils/helpers';
+import GlimmerishComponent from '../../utils/glimmerish-component';
 
 moduleFor(
   'Helpers test: {{get}}',
@@ -616,5 +617,31 @@ moduleFor(
 
       assert.strictEqual(this.$('#get-input').val(), 'mcintosh');
     }
+
+    '@test should be able to get an object value with a path from this.args in a glimmer component'() {
+      class PersonComponent extends GlimmerishComponent {
+        options = ['first', 'last', 'age'];
+      }
+
+      this.registerComponent('person-wrapper', {
+        ComponentClass: PersonComponent,
+        template: '{{#each this.options as |option|}}{{get this.args option}}{{/each}}',
+      });
+
+      this.render('<PersonWrapper @first={{first}} @last={{last}} @age={{age}}/>', {
+        first: 'miguel',
+        last: 'andrade',
+      });
+
+      this.assertText('miguelandrade');
+
+      runTask(() => this.rerender());
+
+      this.assertText('miguelandrade');
+
+      runTask(() => set(this.context, 'age', 30));
+
+      this.assertText('miguelandrade30');
+    }
   }
 );

packages/@ember/-internals/metal/index.ts

@@ -44,7 +44,7 @@ export {
   isClassicDecorator,
   setClassicDecorator,
 } from './lib/descriptor_map';
-export { getChainTagsForKey, ARGS_PROXY_TAGS } from './lib/chain-tags';
+export { getChainTagsForKey } from './lib/chain-tags';
 export { default as libraries, Libraries } from './lib/libraries';
 export { default as getProperties } from './lib/get_properties';
 export { default as setProperties } from './lib/set_properties';
@@ -53,7 +53,7 @@ export { default as expandProperties } from './lib/expand_properties';
 export { addObserver, activateObserver, removeObserver, flushAsyncObservers } from './lib/observer';
 export { Mixin, aliasMethod, mixin, observer, applyMixin } from './lib/mixin';
 export { default as inject, DEBUG_INJECTION_FUNCTIONS } from './lib/injected_property';
-export { tagForProperty, tagFor, markObjectAsDirty, UNKNOWN_PROPERTY_TAG } from './lib/tags';
+export { tagForProperty, createTagForProperty, tagFor, markObjectAsDirty, CUSTOM_TAG_FOR } from './lib/tags';
 export {
   consume,
   Tracker,

packages/@ember/-internals/metal/lib/chain-tags.ts

@@ -6,8 +6,6 @@ import { getLastRevisionFor, peekCacheFor } from './computed_cache';
 import { descriptorForProperty } from './descriptor_map';
 import { tagForProperty } from './tags';
 
-export const ARGS_PROXY_TAGS = new WeakMap();
-
 export function finishLazyChains(obj: any, key: string, value: any) {
   let meta = peekMeta(obj);
   let lazyTags = meta !== null ? meta.readableLazyChainsFor(key) : undefined;
@@ -141,40 +139,6 @@ export function getChainTagsForKey(obj: any, path: string) {
       break;
     }
 
-    // If the segment is linking to an args proxy, we need to manually access
-    // the tags for the args, since they are direct references and don't have a
-    // tagForProperty. We then continue chaining like normal after it, since
-    // you could chain off an arg if it were an object, for instance.
-    if (segment === 'args' && ARGS_PROXY_TAGS.has(current.args)) {
-      assert(
-        `When watching the 'args' on a GlimmerComponent, you must watch a value on the args. You cannot watch the object itself, as it never changes.`,
-        segmentEnd !== pathLength
-      );
-
-      lastSegmentEnd = segmentEnd + 1;
-      segmentEnd = path.indexOf('.', lastSegmentEnd);
-
-      if (segmentEnd === -1) {
-        segmentEnd = pathLength;
-      }
-
-      segment = path.slice(lastSegmentEnd, segmentEnd)!;
-
-      let namedArgs = ARGS_PROXY_TAGS.get(current.args);
-      let ref = namedArgs.get(segment);
-
-      chainTags.push(ref.tag);
-
-      // We still need to break if we're at the end of the path.
-      if (segmentEnd === pathLength) {
-        break;
-      }
-
-      // Otherwise, set the current value and then continue to the next segment
-      current = ref.value();
-      continue;
-    }
-
     // TODO: Assert that current[segment] isn't an undecorated, non-MANDATORY_SETTER/dependentKeyCompat getter
 
     let propertyTag = tagForProperty(current, segment);

packages/@ember/-internals/metal/lib/tags.ts

@@ -5,19 +5,24 @@ import { DEBUG } from '@glimmer/env';
 import { CONSTANT_TAG, createUpdatableTag, dirty, Tag } from '@glimmer/reference';
 import { assertTagNotConsumed } from './tracked';
 
-export const UNKNOWN_PROPERTY_TAG = symbol('UNKNOWN_PROPERTY_TAG');
+export const CUSTOM_TAG_FOR = symbol('CUSTOM_TAG_FOR');
 
 export function tagForProperty(object: any, propertyKey: string | symbol, _meta?: Meta): Tag {
   let objectType = typeof object;
   if (objectType !== 'function' && (objectType !== 'object' || object === null)) {
     return CONSTANT_TAG;
   }
-  let meta = _meta === undefined ? metaFor(object) : _meta;
 
-  if (!(propertyKey in object) && typeof object[UNKNOWN_PROPERTY_TAG] === 'function') {
-    return object[UNKNOWN_PROPERTY_TAG](propertyKey);
+  if (typeof object[CUSTOM_TAG_FOR] === 'function') {
+    return object[CUSTOM_TAG_FOR](propertyKey);
   }
 
+  return createTagForProperty(object, propertyKey);
+}
+
+export function createTagForProperty(object: object, propertyKey: string | symbol, _meta?: Meta): Tag {
+  let meta = _meta === undefined ? metaFor(object) : _meta;
+
   let tags = meta.writableTags();
   let tag = tags[propertyKey];
   if (tag) {

packages/@ember/-internals/runtime/lib/mixins/-proxy.js

@@ -9,8 +9,9 @@ import {
   defineProperty,
   Mixin,
   tagFor,
+  createTagForProperty,
   computed,
-  UNKNOWN_PROPERTY_TAG,
+  CUSTOM_TAG_FOR,
   getChainTagsForKey,
 } from '@ember/-internals/metal';
 import { setProxy } from '@ember/-internals/utils';
@@ -61,8 +62,12 @@ export default Mixin.create({
     return Boolean(get(this, 'content'));
   }),
 
-  [UNKNOWN_PROPERTY_TAG](key) {
-    return combine(getChainTagsForKey(this, `content.${key}`));
+  [CUSTOM_TAG_FOR](key) {
+    if (key in this) {
+      return createTagForProperty(this, key);
+    } else {
+      return combine(getChainTagsForKey(this, `content.${key}`));
+    }
   },
 
   unknownProperty(key) {