diff --git a/packages/@ember/-internals/metal/lib/property_get.ts b/packages/@ember/-internals/metal/lib/property_get.ts index 0251d4f4670..17f76f3b6c7 100644 --- a/packages/@ember/-internals/metal/lib/property_get.ts +++ b/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; diff --git a/packages/@ember/-internals/metal/lib/tracked.ts b/packages/@ember/-internals/metal/lib/tracked.ts index e51e8e9e0e9..4d4324ead38 100644 --- a/packages/@ember/-internals/metal/lib/tracked.ts +++ b/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 | 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); } diff --git a/packages/@ember/-internals/metal/tests/tracked/validation_test.js b/packages/@ember/-internals/metal/tests/tracked/validation_test.js index 25530230d69..37273dcfcce 100644 --- a/packages/@ember/-internals/metal/tests/tracked/validation_test.js +++ b/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/); + } } ); }