makes access a value inside an unknownProperty a deprecation instead …

…of an assertion

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

@@ -8,7 +8,7 @@ import { DEBUG } from '@glimmer/env';
 import { descriptorForProperty } from './descriptor_map';
 import { isPath } from './path_cache';
 import { tagForProperty } from './tags';
-import { consume, isTracking } from './tracked';
+import { consume, deprecateMutationsInAutotrackingTransaction, isTracking } from './tracked';
 
 export const PROXY_CONTENT = symbol('PROXY_CONTENT');
 
@@ -143,7 +143,17 @@ export function get(obj: object, keyName: string): any {
       !(keyName in obj) &&
       typeof (obj as MaybeHasUnknownProperty).unknownProperty === 'function'
     ) {
-      return (obj as MaybeHasUnknownProperty).unknownProperty!(keyName);
+      if (DEBUG) {
+        let ret;
+
+        deprecateMutationsInAutotrackingTransaction!(() => {
+          ret = (obj as MaybeHasUnknownProperty).unknownProperty!(keyName);
+        });
+
+        return ret;
+      } else {
+        return (obj as MaybeHasUnknownProperty).unknownProperty!(keyName);
+      }
     }
   }
   return value;

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

@@ -13,7 +13,7 @@ interface AutotrackingTransactionSourceData {
   error: Error;
 }
 
-let WARN_IN_AUTOTRACKING_TRANSACTION = false;
+let DEPRECATE_IN_AUTOTRACKING_TRANSACTION = false;
 let AUTOTRACKING_TRANSACTION: WeakMap<Tag, AutotrackingTransactionSourceData> | null = null;
 
 export let runInAutotrackingTransaction: undefined | ((fn: () => void) => void);
@@ -28,40 +28,51 @@ export let assertTagNotConsumed:
 let markTagAsConsumed: undefined | ((_tag: Tag, sourceError: Error) => void);
 
 if (DEBUG) {
+  /**
+   * Creates a global autotracking transaction. This will prevent any backflow
+   * in any `track` calls within the transaction, even if they are not
+   * externally consumed.
+   *
+   * `runInAutotrackingTransaction` can be called within itself, and it will add
+   * onto the existing transaction if one exists.
+   *
+   * TODO: Only throw an error if the `track` is consumed.
+   */
   runInAutotrackingTransaction = (fn: () => void) => {
-    if (AUTOTRACKING_TRANSACTION !== null) {
-      // already in a transaction, continue and let the original transaction finish
-      fn();
-      return;
-    }
+    let previousDeprecateState = DEPRECATE_IN_AUTOTRACKING_TRANSACTION;
+    let previousTransactionState = AUTOTRACKING_TRANSACTION;
 
-    AUTOTRACKING_TRANSACTION = new WeakMap();
+    DEPRECATE_IN_AUTOTRACKING_TRANSACTION = false;
+
+    if (previousTransactionState === null) {
+      // if there was no transaction start it. Otherwise, the transaction already exists.
+      AUTOTRACKING_TRANSACTION = new WeakMap();
+    }
 
     try {
       fn();
     } finally {
-      AUTOTRACKING_TRANSACTION = null;
+      DEPRECATE_IN_AUTOTRACKING_TRANSACTION = previousDeprecateState;
+      AUTOTRACKING_TRANSACTION = previousTransactionState;
     }
   };
 
+  /**
+   * Switches to deprecating within an autotracking transaction, if one exists.
+   * If `runInAutotrackingTransaction` is called within the callback of this
+   * method, it switches back to throwing an error, allowing zebra-striping of
+   * the types of errors that are thrown.
+   *
+   * Does not start an autotracking transaction.
+   */
   deprecateMutationsInAutotrackingTransaction = (fn: () => void) => {
-    assert(
-      'deprecations can only occur inside of an autotracking transaction',
-      AUTOTRACKING_TRANSACTION !== null
-    );
-
-    if (WARN_IN_AUTOTRACKING_TRANSACTION) {
-      // already in a transaction, continue and let the original transaction finish
-      fn();
-      return;
-    }
-
-    WARN_IN_AUTOTRACKING_TRANSACTION = true;
+    let previousDeprecateState = DEPRECATE_IN_AUTOTRACKING_TRANSACTION;
+    DEPRECATE_IN_AUTOTRACKING_TRANSACTION = true;
 
     try {
       fn();
     } finally {
-      WARN_IN_AUTOTRACKING_TRANSACTION = false;
+      DEPRECATE_IN_AUTOTRACKING_TRANSACTION = previousDeprecateState;
     }
   };
 
@@ -137,7 +148,7 @@ if (DEBUG) {
 
     if (!sourceData) return;
 
-    if (WARN_IN_AUTOTRACKING_TRANSACTION && !forceHardError) {
+    if (DEPRECATE_IN_AUTOTRACKING_TRANSACTION && !forceHardError) {
       deprecate(makeAutotrackingErrorMessage(sourceData, obj, keyName), false, {
         id: 'autotracking.mutation-after-consumption',
         until: '4.0.0',
@@ -354,7 +365,7 @@ function descriptorForField([_target, key, desc]: [
     get(): any {
       let propertyTag = tagForProperty(this, key) as UpdatableTag;
 
-      if (CURRENT_TRACKER) CURRENT_TRACKER.add(propertyTag);
+      consume(propertyTag);
 
       let value;
 
@@ -378,6 +389,9 @@ function descriptorForField([_target, key, desc]: [
 
     set(newValue: any): void {
       if (DEBUG) {
+        // No matter what, attempting to update a tracked property in an
+        // autotracking context after it has been read is invalid, even if we
+        // are otherwise warning, so always assert.
         assertTagNotConsumed!(tagForProperty(this, key), this, key, true);
       }
 

packages/@ember/-internals/metal/tests/tracked/validation_test.js

@@ -356,6 +356,48 @@ if (EMBER_METAL_TRACKED_PROPERTIES) {
           }
         }, /You attempted to update `value` on `EmberObject`, but it had already been used previously in the same computation/);
       }
+
+      ['@test gives helpful deprecation when a property tracked with `get` is mutated after access within unknownProperty within an autotracking transaction']() {
+        class EmberObject {
+          foo = null;
+
+          unknownProperty() {
+            get(this, 'foo');
+            set(this, 'foo', 123);
+          }
+        }
+
+        let obj = new EmberObject();
+
+        expectDeprecation(() => {
+          if (DEBUG) {
+            track(() => {
+              get(obj, 'bar');
+            });
+          }
+        }, /You attempted to update `foo` on `EmberObject`, but it had already been used previously in the same computation/);
+      }
+
+      ['@test gives helpful assertion when a tracked property is mutated after access within unknownProperty within an autotracking transaction']() {
+        class EmberObject {
+          @tracked foo;
+
+          unknownProperty() {
+            this.foo;
+            this.foo = 123;
+          }
+        }
+
+        let obj = new EmberObject();
+
+        expectAssertion(() => {
+          if (DEBUG) {
+            track(() => {
+              get(obj, 'bar');
+            });
+          }
+        }, /You attempted to update `foo` on `EmberObject`, but it had already been used previously in the same computation/);
+      }
     }
   );
 }