Allow get/set/get (lazy initialization) within same tracking frame#21226
Conversation
When a tracked property is read, then written, then read again within the same tracking frame (e.g., lazy initialization pattern), the backtracking assertion no longer fires. Cross-frame mutations (e.g., component A reads, component B writes) still error as before. Fixes emberjs#21225 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
| // a subsequent read can re-consume it with the updated value. | ||
| let currentTransaction = TRANSACTION_STACK[TRANSACTION_STACK.length - 1]; | ||
| if (transaction === currentTransaction) { | ||
| CONSUMED_TAGS.delete(tag); |
There was a problem hiding this comment.
I wonder if there is a way to check that this tag is the same tag that is being set 🤔
Perhaps when we should do this before marking a tag as dirty (and not mark dirty?) I wonder if there are render-output consequences to doing that 🤔
There was a problem hiding this comment.
assertTagNotConsumed seems like a debug only thing? how do we protect production from infinite render loops?
There was a problem hiding this comment.
The tag param in assertTagNotConsumed IS the tag being dirtied — so we're already checking the same tag identity (it was consumed, and now it's the one being set).
The idea of skipping the dirty instead of just suppressing the assertion is interesting. If we skip DIRTY_TAG for same-frame set, the computation still uses the new value (it reads it on the subsequent get), and we avoid scheduling a redundant revalidation — since we're already mid-computation and the result will reflect the new value.
However, that would change production behavior (not just debug). Right now this PR only touches the debug assertion path — DIRTY_TAG still fires, revision still bumps, scheduleRevalidate() still runs. If you'd prefer the "skip dirty" approach, I can move the same-transaction check into dirtyTagFor and conditionally skip the DIRTY_TAG call. The tradeoff is that if a parent frame also consumed the same tag, it wouldn't see the invalidation until the next external mutation.
There was a problem hiding this comment.
Yes, assertTagNotConsumed is entirely debug-only — the whole thing lives inside if (DEBUG). In production, there has never been a backtracking check. Get/set/get already works silently in production builds today; the debug assertion was the only thing catching it in development.
For infinite render loops in production: the lazy init pattern converges because the set is conditional (if (!c) this.count = 2). On re-render, the value is already set, the condition is false, no dirty happens, no further revalidation. An unconditional get → set(new value) → get would infinite-loop in production, but that's already the case today (the debug assertion catches it in dev, but production has no guard).
The rendering engine does have its own max-revalidation safeguard as a last resort, but this PR doesn't change the production behavior at all — only the dev-mode assertion.
There was a problem hiding this comment.
we want to support lazy initialization without infinite looping
There was a problem hiding this comment.
this behavior change would probably need an RFC, fwiw (emberjs/rfcs) -- the original decision to throw so aggressively also should have probably gone through RFC
There was a problem hiding this comment.
Updated the implementation to address both concerns. Instead of immediately un-consuming on same-frame set, it now defers the assertion to end-of-frame:
- On same-frame
get → set: the tag is added toPENDING_SAME_FRAME_ASSERTIONSand un-consumed - On subsequent
get(re-consume): the pending assertion is cleared — this is the valid lazy init pattern - At
endTrackingTransaction: any remaining pending assertions fire — this catchesget → setwithout re-get, which risks infinite revalidation
So now:
get → set → get(lazy init) → allowed ✓get → set(no re-get) → still errors ✓- cross-frame backtracking → still errors ✓
All 8795 tests pass, lint is clean. This is still debug-only — production behavior unchanged.
Re: RFC — acknowledged. Happy to help draft one if you'd like to go that route.
| // eslint-disable-next-line @typescript-eslint/no-non-null-assertion -- @fixme | ||
| debug.runInTrackingTransaction!(() => { | ||
| track(() => { | ||
| consumeTag(tag); |
There was a problem hiding this comment.
when this group of 3 operations occurs in a tracking frame, it should be collapsed down to one consume -- but only if we have get-then-set.
it's possible that doing get-then-set counts as a no-op (as far as tracking goes), but set-then-get is also fine.
this would mean that a set used in this pattern never counts as a dirty. So if rendering already consumed the value fully, no update would be seen 🤔 is there a way to consider this?
(this is what the original error was meant to protect -- in the pursuit of always correct render-output)
I wonder how solidjs handles this 🤔
There was a problem hiding this comment.
Good points. Let me break this down:
Current behavior in this PR: The tag IS still dirtied (revision bumped, scheduleRevalidate() called). We only suppress the debug assertion. So a same-frame get/set/get will still schedule a revalidation. For the lazy init pattern this converges (the set is conditional — on re-render the condition is false, no dirty, no loop). An unconditional get/set/get would infinite-loop, but that's the same as production today.
Collapsing get-then-set to a no-op: If we also skip the DIRTY_TAG call for same-frame mutations, the computation still uses the new value (reads it on the second get), but no revalidation is scheduled. This is arguably more correct — we're mid-computation, the result will already reflect the new value, so dirtying is redundant. But it changes production behavior, not just debug, and has a subtlety: if a parent frame already consumed this tag during the same render, it wouldn't see the invalidation.
A middle ground (if you want to preserve the assertion for get/set without re-get): Instead of un-consuming at set-time, we could defer the check to end-of-frame. Record "pending" dirty tags, and at endTrackingTransaction, assert that any pending tags were re-consumed. This catches get → set (no second get, likely a bug) while allowing get → set → get (lazy init). More complex but preserves the dev-mode safety net.
SolidJS: Solid's fine-grained reactivity doesn't have a "backtracking" concept. Signals are read/written freely inside createMemo / createEffect. The key difference is Solid doesn't batch DOM updates the way Glimmer does — each signal write synchronously updates dependents, so there's no stale-computation concern. Ember/Glimmer's tracking transaction model is fundamentally different — the entire render tree is a single batch.
Which direction do you prefer? I can implement the deferred check (middle ground) or the skip-dirty approach if you'd like.
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) <noreply@anthropic.com>
| this.render('{{hello}}'); | ||
| }, expectedMessage); | ||
| this.render('{{hello}}'); | ||
| this.assertText('456'); |
There was a problem hiding this comment.
we should probably assert renderSettled or stablerender, whatever that helper is we use all over the place in other tests
- Format debug.ts with prettier - Fix TS2345: use `!= null` check instead of `&&` for PropertyKey - Remove unused backtrackingMessageFor import - Ensure modifier tests use get/set/get pattern (re-get after set) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
How it works
In
assertTagNotConsumed, when a dirty (set) is detected for a tag consumed in the current transaction (same frame), the tag is removed fromCONSUMED_TAGSinstead of throwing. This allows:get → set → get(lazy init): first get consumes, set un-consumes, second get re-consumes ✓Test plan
@glimmer/validatorunit tests for the new get/set/get behavior@ember/-internals/metaltracked validation tests@emberand@glimmer-workspace)🤖 Generated with Claude Code