Merge pull request #18721 from emberjs/backport/autotracking-bugfixes

[BUGFIX release] Backport autotracking bugfixes

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/glimmer/tests/integration/syntax/each-test.js

@@ -1,6 +1,6 @@
 import { moduleFor, RenderingTestCase, applyMixins, strip, runTask } from 'internal-test-helpers';
 
-import { get, set, notifyPropertyChange } from '@ember/-internals/metal';
+import { get, set, notifyPropertyChange, computed } from '@ember/-internals/metal';
 import { A as emberA, ArrayProxy, RSVP } from '@ember/-internals/runtime';
 import { HAS_NATIVE_SYMBOL } from '@ember/-internals/utils';
 
@@ -1089,6 +1089,25 @@ moduleFor(
   }
 );
 
+moduleFor(
+  'Syntax test: {{#each}} with array proxies, arrangedContent depends on external content',
+  class extends EachTest {
+    createList(items) {
+      let wrapped = emberA(items);
+      let proxy = ArrayProxy.extend({
+        arrangedContent: computed('wrappedItems.[]', function() {
+          // Slice the items to ensure that updates must be propogated
+          return this.wrappedItems.slice();
+        }),
+      }).create({
+        wrappedItems: wrapped,
+      });
+
+      return { list: proxy, delegate: wrapped };
+    }
+  }
+);
+
 moduleFor(
   'Syntax test: {{#each as}} undefined path',
   class extends RenderingTestCase {

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,13 @@ 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/array_events.ts

@@ -32,7 +32,8 @@ export function arrayContentDidChange<T extends { length: number }>(
   array: T,
   startIdx: number,
   removeAmt: number,
-  addAmt: number
+  addAmt: number,
+  notify = true
 ): T {
   // if no args are passed assume everything changes
   if (startIdx === undefined) {
@@ -50,11 +51,13 @@ export function arrayContentDidChange<T extends { length: number }>(
 
   let meta = peekMeta(array);
 
-  if (addAmt < 0 || removeAmt < 0 || addAmt - removeAmt !== 0) {
-    notifyPropertyChange(array, 'length', meta);
-  }
+  if (notify) {
+    if (addAmt < 0 || removeAmt < 0 || addAmt - removeAmt !== 0) {
+      notifyPropertyChange(array, 'length', meta);
+    }
 
-  notifyPropertyChange(array, '[]', meta);
+    notifyPropertyChange(array, '[]', meta);
+  }
 
   sendEvent(array, '@array:change', [array, startIdx, removeAmt, addAmt]);
 

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,28 @@ 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) {
@@ -27,7 +36,7 @@ export function tagForProperty(object: any, propertyKey: string | symbol, _meta?
   let newTag = createUpdatableTag();
 
   if (DEBUG) {
-    setupMandatorySetter!(object, propertyKey);
+    setupMandatorySetter!(newTag, object, propertyKey);
 
     (newTag as any)._propertyKey = propertyKey;
   }

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,14 @@ export default Mixin.create({
     return Boolean(get(this, 'content'));
   }),
 
-  [UNKNOWN_PROPERTY_TAG](key) {
-    return combine(getChainTagsForKey(this, `content.${key}`));
+  [CUSTOM_TAG_FOR](key) {
+    let tag = createTagForProperty(this, key);
+
+    if (key in this) {
+      return tag;
+    } else {
+      return combine([tag, ...getChainTagsForKey(this, `content.${key}`)]);
+    }
   },
 
   unknownProperty(key) {