Skip to content

Commit a6a74d4

Browse files
NullVoxPopuliclaude
andcommitted
Defer backtracking assertion to end-of-frame for same-frame mutations
Instead of immediately allowing or throwing for same-frame get/set, defer the assertion to endTrackingTransaction. If the tag is re-consumed (get/set/get pattern) before the frame ends, the pending assertion is cleared. Otherwise it still fires. This preserves the safety net for get/set without re-get (which can cause infinite revalidation) while allowing the lazy initialization pattern (get/set/get). Also fixes lint: remove unused backtrackingMessageFor import. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
1 parent 24f5256 commit a6a74d4

5 files changed

Lines changed: 100 additions & 6 deletions

File tree

packages/@ember/-internals/glimmer/tests/integration/custom-modifier-manager-test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import { Component } from '@ember/-internals/glimmer';
1111
import { setModifierManager, modifierCapabilities } from '@glimmer/manager';
1212
import EmberObject, { set } from '@ember/object';
1313
import { tracked } from '@ember/-internals/metal';
14-
import { backtrackingMessageFor } from '../utils/debug-stack';
1514

1615
class ModifierManagerTest extends RenderingTestCase {
1716
'@test throws a useful error when missing capabilities'(assert) {
@@ -272,6 +271,7 @@ class ModifierManagerTest extends RenderingTestCase {
272271
let val = person.name;
273272
if (val === undefined) {
274273
person.name = 'sam';
274+
val = person.name;
275275
}
276276
}
277277
}

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -380,6 +380,24 @@ moduleFor(
380380
});
381381
}
382382

383+
['@test gives helpful assertion when a tracked property is mutated after access without re-read']() {
384+
class MyObject {
385+
@tracked value;
386+
toString() {
387+
return 'MyObject';
388+
}
389+
}
390+
391+
let obj = new MyObject();
392+
393+
expectAssertion(() => {
394+
track(() => {
395+
obj.value;
396+
obj.value = 123;
397+
});
398+
}, /You attempted to update `value` on `MyObject`, but it had already been used previously in the same computation/);
399+
}
400+
383401
['@test get() does not entangle in the autotracking stack until after retrieving the value'](
384402
assert
385403
) {

packages/@glimmer-workspace/integration-tests/test/managers/modifier-manager-test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,7 @@ abstract class ModifierManagerTest extends RenderTest {
228228
let val = this.foo;
229229
if (val === undefined) {
230230
this.foo = 456;
231+
val = this.foo;
231232
}
232233
}
233234

packages/@glimmer/validator/lib/debug.ts

Lines changed: 42 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,14 @@ interface Transaction {
3434
if (DEBUG) {
3535
let CONSUMED_TAGS: WeakMap<Tag, Transaction> | null = null;
3636

37+
// Tracks tags that were dirtied after being consumed within the same tracking frame.
38+
// If a tag is re-consumed before the frame ends, it's removed (get/set/get is OK).
39+
// Any remaining entries at end-of-frame trigger the backtracking assertion.
40+
let PENDING_SAME_FRAME_ASSERTIONS: Map<
41+
Tag,
42+
{ obj?: unknown; keyName?: string | symbol; transaction: Transaction }
43+
> | null = null;
44+
3745
const TRANSACTION_STACK: Transaction[] = [];
3846

3947
/////////
@@ -81,10 +89,26 @@ if (DEBUG) {
8189
throw new Error('attempted to close a tracking transaction, but one was not open');
8290
}
8391

84-
TRANSACTION_STACK.pop();
92+
let closingTransaction = TRANSACTION_STACK.pop();
93+
94+
// Check for any same-frame get/set that was never followed by a re-get.
95+
// This catches patterns like `get → set` (without a second get) which can
96+
// cause infinite revalidation loops.
97+
if (PENDING_SAME_FRAME_ASSERTIONS !== null && closingTransaction !== undefined) {
98+
for (let [, pending] of PENDING_SAME_FRAME_ASSERTIONS) {
99+
if (pending.transaction === closingTransaction) {
100+
// This tag was dirtied after being consumed but never re-consumed
101+
// before the frame ended — this is likely a bug, not lazy init.
102+
let message = TRANSACTION_ENV.debugMessage(pending.obj, pending.keyName && String(pending.keyName));
103+
PENDING_SAME_FRAME_ASSERTIONS = null;
104+
assert(false, message);
105+
}
106+
}
107+
}
85108

86109
if (TRANSACTION_STACK.length === 0) {
87110
CONSUMED_TAGS = null;
111+
PENDING_SAME_FRAME_ASSERTIONS = null;
88112
}
89113
};
90114

@@ -97,6 +121,7 @@ if (DEBUG) {
97121

98122
TRANSACTION_STACK.splice(0, TRANSACTION_STACK.length);
99123
CONSUMED_TAGS = null;
124+
PENDING_SAME_FRAME_ASSERTIONS = null;
100125

101126
return stack;
102127
};
@@ -178,7 +203,15 @@ if (DEBUG) {
178203
};
179204

180205
debug.markTagAsConsumed = (_tag: Tag) => {
181-
if (!CONSUMED_TAGS || CONSUMED_TAGS.has(_tag)) return;
206+
if (!CONSUMED_TAGS) return;
207+
208+
// If this tag has a pending same-frame assertion, the re-consume (second get)
209+
// resolves it — this is the valid get/set/get (lazy initialization) pattern.
210+
if (PENDING_SAME_FRAME_ASSERTIONS !== null && PENDING_SAME_FRAME_ASSERTIONS.has(_tag)) {
211+
PENDING_SAME_FRAME_ASSERTIONS.delete(_tag);
212+
}
213+
214+
if (CONSUMED_TAGS.has(_tag)) return;
182215

183216
CONSUMED_TAGS.set(_tag, getLast(asPresentArray(TRANSACTION_STACK)));
184217

@@ -204,10 +237,15 @@ if (DEBUG) {
204237
if (!transaction) return;
205238

206239
// Allow get/set/get (lazy initialization) within the same tracking frame.
207-
// If the tag was consumed in the current transaction, un-consume it so that
208-
// a subsequent read can re-consume it with the updated value.
240+
// Instead of throwing immediately, defer the assertion to end-of-frame.
241+
// If the tag is re-consumed (second get) before the frame ends, the pending
242+
// assertion is cleared. Otherwise it fires at endTrackingTransaction.
209243
let currentTransaction = TRANSACTION_STACK[TRANSACTION_STACK.length - 1];
210244
if (transaction === currentTransaction) {
245+
if (PENDING_SAME_FRAME_ASSERTIONS === null) {
246+
PENDING_SAME_FRAME_ASSERTIONS = new Map();
247+
}
248+
PENDING_SAME_FRAME_ASSERTIONS.set(tag, { obj, keyName, transaction });
211249
CONSUMED_TAGS.delete(tag);
212250
return;
213251
}

packages/@glimmer/validator/test/tracking-test.ts

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -513,6 +513,29 @@ module('@glimmer/validator: tracking', () => {
513513
assert.strictEqual(result, 789, 'get after set returns the updated value');
514514
});
515515

516+
test('it errors when get/set is not followed by a re-get in the same frame', (assert) => {
517+
class Foo {
518+
foo = 123;
519+
bar = 456;
520+
}
521+
522+
let { getter, setter } = trackedData<Foo, keyof Foo>('foo', function (this: Foo) {
523+
return this.bar;
524+
});
525+
526+
let foo = new Foo();
527+
528+
assert.throws(() => {
529+
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion -- @fixme
530+
debug.runInTrackingTransaction!(() => {
531+
track(() => {
532+
getter(foo);
533+
setter(foo, 789);
534+
});
535+
});
536+
}, /You attempted to update `foo` on `Foo`/);
537+
});
538+
516539
test('it still errors when updating a value consumed in a previous tracking frame', (assert) => {
517540
class Foo {
518541
foo = 123;
@@ -547,7 +570,7 @@ module('@glimmer/validator: tracking', () => {
547570
assert.expect(0);
548571
let tag = createTag();
549572

550-
// consume/dirty within the same frame should not throw (supports get/set/get pattern)
573+
// consume/dirty/consume within the same frame should not throw (get/set/get pattern)
551574
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion -- @fixme
552575
debug.runInTrackingTransaction!(() => {
553576
track(() => {
@@ -558,6 +581,20 @@ module('@glimmer/validator: tracking', () => {
558581
});
559582
});
560583

584+
test('it errors when consume/dirty is not followed by a re-consume in the same frame', (assert) => {
585+
let tag = createTag();
586+
587+
assert.throws(() => {
588+
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion -- @fixme
589+
debug.runInTrackingTransaction!(() => {
590+
track(() => {
591+
consumeTag(tag);
592+
dirtyTag(tag);
593+
});
594+
});
595+
}, /Error: Assertion Failed: You attempted to update/u);
596+
});
597+
561598
test('it throws errors across track frames within the same debug transaction', (assert) => {
562599
let tag = createTag();
563600

0 commit comments

Comments
 (0)