Skip to content

Allow get/set/get (lazy initialization) within same tracking frame #21226

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
NullVoxPopuli-ai-agent wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Allow get/set/get (lazy initialization) within same tracking frame #21226

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
24f5256
Allow get/set/get (lazy initialization) within same tracking frame
NullVoxPopuli Mar 19, 2026
a6a74d4
Defer backtracking assertion to end-of-frame for same-frame mutations
NullVoxPopuli Mar 19, 2026
fd7c758
Fix CI: prettier formatting, TypeScript type error, unused import
NullVoxPopuli Mar 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 11 additions & 11 deletions packages/@ember/-internals/glimmer/tests/integration/custom-modifier-manager-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
import { setModifierManager, modifierCapabilities } from '@glimmer/manager';
import EmberObject, { set } from '@ember/object';
import { tracked } from '@ember/-internals/metal';
import { backtrackingMessageFor } from '../utils/debug-stack';

Check failure on line 14 in packages/@ember/-internals/glimmer/tests/integration/custom-modifier-manager-test.js

View workflow job for this annotation

GitHub Actions / tests / Linting 

'backtrackingMessageFor' is defined but never used

class ModifierManagerTest extends RenderingTestCase {
'@test throws a useful error when missing capabilities'(assert) {
Expand Down Expand Up @@ -242,9 +242,9 @@
assert.equal(updateCount, 2);
}

'@test provides a helpful assertion when mutating a value that was consumed already'() {
'@test allows get/set (lazy initialization) within modifier didInsertElement'(assert) {
class Person {
@tracked name = 'bob';
@tracked name;
}

let ModifierClass = setModifierManager(
Expand All @@ -262,24 +262,24 @@
}
);

let person = new Person();

this.registerModifier(
'foo-bar',
class MyModifier extends ModifierClass {
didInsertElement([person]) {
person.name;
person.name = 'sam';
// get/set/get pattern (lazy initialization)
let val = person.name;
if (val === undefined) {
person.name = 'sam';
}
}
}
);

let expectedMessage = backtrackingMessageFor('name', Person.name, {
renderTree: [],
includeTopLevel: false,
});
this.render('<h1 {{foo-bar this.person}}>hello world</h1>', { person });

expectAssertion(() => {
this.render('<h1 {{foo-bar this.person}}>hello world</h1>', { person: new Person() });
}, expectedMessage);
assert.strictEqual(person.name, 'sam', 'lazy initialization in modifier works');
}

'@test capabilities helper function must be used to generate capabilities'(assert) {
Expand Down
58 changes: 23 additions & 35 deletions packages/@ember/-internals/glimmer/tests/integration/helpers/helper-manager-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import { set } from '@ember/object';
import { setOwner } from '@ember/-internals/owner';
import Service, { service } from '@ember/service';
import { registerDestructor } from '@glimmer/destroyable';
import { backtrackingMessageFor } from '../../utils/debug-stack';

class TestHelperManager {
capabilities = helperCapabilities('3.23', {
Expand Down Expand Up @@ -216,7 +215,7 @@ moduleFor(
this.assertText('hello');
}

['@test debug name is used for backtracking message']() {
['@test allows get/set within the same tracking frame (lazy initialization)']() {
this.registerCustomHelper(
'hello',
class extends TestHelper {
Expand All @@ -225,17 +224,13 @@ moduleFor(
value() {
this.foo;
this.foo = 456;
return this.foo;
}
}
);

let expectedMessage = backtrackingMessageFor('foo', '.*', {
renderTree: ['\\(result of a `TEST_HELPER` helper\\)'],
});

expectAssertion(() => {
this.render('{{hello}}');
}, expectedMessage);
this.render('{{hello}}');
this.assertText('456');
Copy link
Copy Markdown
Contributor

@NullVoxPopuli NullVoxPopuli Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should probably assert renderSettled or stablerender, whatever that helper is we use all over the place in other tests

}

['@test asserts against using both `hasValue` and `hasScheduledEffect`'](assert) {
Expand Down Expand Up @@ -347,20 +342,20 @@ moduleFor(
assert.verifySteps([]);
}

'@test custom helpers gives helpful assertion when reading then mutating a tracked value within constructor'() {
'@test custom helpers allows get/set/get (lazy initialization) within constructor'() {
this.registerCustomHelper(
'hello',
class extends TestHelper {
@tracked foo = 123;
@tracked foo;

constructor() {
super(...arguments);

// first read the tracked property
this.foo;

// then attempt to update the tracked property
this.foo = 456;
// get/set/get pattern (lazy initialization)
let val = this.foo;
if (val === undefined) {
this.foo = 456;
}
}

value() {
Expand All @@ -369,36 +364,29 @@ moduleFor(
}
);

let expectedMessage = backtrackingMessageFor('foo');

expectAssertion(() => {
this.render('{{hello}}');
}, expectedMessage);
this.render('{{hello}}');
this.assertText('456');
}

'@test custom helpers gives helpful assertion when reading then mutating a tracked value within value'() {
'@test custom helpers allows get/set/get (lazy initialization) within value'() {
this.registerCustomHelper(
'hello',
class extends TestHelper {
@tracked foo = 123;
@tracked foo;

value() {
// first read the tracked property
this.foo;

// then attempt to update the tracked property
this.foo = 456;
// get/set/get pattern (lazy initialization)
let val = this.foo;
if (val === undefined) {
this.foo = 456;
}
return this.foo;
}
}
);

let expectedMessage = backtrackingMessageFor('foo', '.*', {
renderTree: ['\\(result of a `TEST_HELPER` helper\\)'],
});

expectAssertion(() => {
this.render('{{hello}}');
}, expectedMessage);
this.render('{{hello}}');
this.assertText('456');
}
}
);
13 changes: 6 additions & 7 deletions packages/@ember/-internals/metal/tests/tracked/validation_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,7 @@ moduleFor(
);
}

['@test gives helpful assertion when a tracked property is mutated after access in with an autotracking transaction']() {
['@test allows get/set/get (lazy initialization) within the same tracking frame'](assert) {
class MyObject {
@tracked value;
toString() {
Expand All @@ -373,12 +373,11 @@ moduleFor(

let obj = new MyObject();

expectAssertion(() => {
track(() => {
obj.value;
obj.value = 123;
});
}, /You attempted to update `value` on `MyObject`, but it had already been used previously in the same computation/);
track(() => {
obj.value;
obj.value = 123;
assert.strictEqual(obj.value, 123, 'get after set returns the updated value');
});
}

['@test get() does not entangle in the autotracking stack until after retrieving the value'](
Expand Down
53 changes: 23 additions & 30 deletions packages/@glimmer-workspace/integration-tests/test/managers/helper-manager-test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -352,19 +352,19 @@ class HelperManagerTest extends RenderTest {
this.assertHTML('hello');
}

@test({ skip: !DEBUG }) 'debug name is used for backtracking message'(assert: Assert) {
@test({ skip: !DEBUG }) 'allows get/set within the same tracking frame (lazy initialization)'() {
class Hello extends TestHelper {
@tracked foo = 123;

value() {
consume(this.foo);
this.foo = 456;
return this.foo;
}
}

assert.throws(() => {
this.renderComponent(defineComponent({ hello: Hello }, '{{hello}}'));
}, /You attempted to update `foo` on/u);
this.renderComponent(defineComponent({ hello: Hello }, '{{hello}}'));
this.assertHTML('456');
}

@test({ skip: !DEBUG }) 'asserts against using both `hasValue` and `hasScheduledEffect`'(
Expand Down Expand Up @@ -451,53 +451,46 @@ class HelperManagerTest extends RenderTest {
}

@test({ skip: !DEBUG })
'custom helpers gives helpful assertion when reading then mutating a tracked value within constructor'(
assert: Assert
) {
'custom helpers allows get/set/get (lazy initialization) within constructor'() {
class Hello extends TestHelper {
@tracked foo = 123;
@tracked foo: number | undefined;

constructor(owner: Owner, args: Arguments) {
super(owner, args);

// first read the tracked property

consume(this.foo);

// then attempt to update the tracked property
this.foo = 456;
// get/set/get pattern (lazy initialization)
let val = this.foo;
if (val === undefined) {
this.foo = 456;
}
}

value() {
return this.foo;
}
}

assert.throws(() => {
this.renderComponent(defineComponent({ hello: Hello }, '{{hello}}'));
}, /You attempted to update `foo` on /u);
this.renderComponent(defineComponent({ hello: Hello }, '{{hello}}'));
this.assertHTML('456');
}

@test({ skip: !DEBUG })
'custom helpers gives helpful assertion when reading then mutating a tracked value within value'(
assert: Assert
) {
'custom helpers allows get/set/get (lazy initialization) within value'() {
class Hello extends TestHelper {
@tracked foo = 123;
@tracked foo: number | undefined;

value() {
// first read the tracked property

consume(this.foo);

// then attempt to update the tracked property
this.foo = 456;
// get/set/get pattern (lazy initialization)
let val = this.foo;
if (val === undefined) {
this.foo = 456;
}
return this.foo;
}
}

assert.throws(() => {
this.renderComponent(defineComponent({ hello: Hello }, '{{hello}}'));
}, /You attempted to update `foo` on /u);
this.renderComponent(defineComponent({ hello: Hello }, '{{hello}}'));
this.assertHTML('456');
}
}

Expand Down
22 changes: 9 additions & 13 deletions packages/@glimmer-workspace/integration-tests/test/managers/modifier-manager-test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,21 +217,18 @@ abstract class ModifierManagerTest extends RenderTest {
}

@test({ skip: !DEBUG })
'provides a helpful deprecation when mutating a tracked value that was consumed already within constructor'(
assert: Assert
) {
'allows get/set (lazy initialization) within modifier constructor'() {
class Foo extends CustomModifier {
@tracked foo = 123;
@tracked foo: number | undefined;

constructor(owner: Owner, args: Arguments) {
super(owner, args);

// first read the tracked property

consume(this.foo);

// then attempt to update the tracked property
this.foo = 456;
// get/set/get pattern (lazy initialization)
let val = this.foo;
if (val === undefined) {
this.foo = 456;
}
}

override didInsertElement() {}
Expand All @@ -243,9 +240,8 @@ abstract class ModifierManagerTest extends RenderTest {

let Main = defineComponent({ foo }, '<h1 {{foo}}>hello world</h1>');

assert.throws(() => {
this.renderComponent(Main);
}, /You attempted to update `foo` on `.*`, but it had already been used previously in the same computation/u);
this.renderComponent(Main);
this.assertHTML('<h1>hello world</h1>');
}

@test
Expand Down
9 changes: 9 additions & 0 deletions packages/@glimmer/validator/lib/debug.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,15 @@ if (DEBUG) {

if (!transaction) return;

// Allow get/set/get (lazy initialization) within the same tracking frame.
// If the tag was consumed in the current transaction, un-consume it so that
// 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);
Copy link
Copy Markdown
Contributor

@NullVoxPopuli NullVoxPopuli Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 🤔

Copy link
Copy Markdown
Contributor

@NullVoxPopuli NullVoxPopuli Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assertTagNotConsumed seems like a debug only thing? how do we protect production from infinite render loops?

Copy link
Copy Markdown
Contributor Author

@NullVoxPopuli-ai-agent NullVoxPopuli-ai-agent Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

@NullVoxPopuli-ai-agent NullVoxPopuli-ai-agent Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor

@NullVoxPopuli NullVoxPopuli Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we want to support lazy initialization without infinite looping

Copy link
Copy Markdown
Contributor

@NullVoxPopuli NullVoxPopuli Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor Author

@NullVoxPopuli-ai-agent NullVoxPopuli-ai-agent Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

  1. On same-frame get → set: the tag is added to PENDING_SAME_FRAME_ASSERTIONS and un-consumed
  2. On subsequent get (re-consume): the pending assertion is cleared — this is the valid lazy init pattern
  3. At endTrackingTransaction: any remaining pending assertions fire — this catches get → set without 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.

return;
}

// This hack makes the assertion message nicer, we can cut off the first
// few lines of the stack trace and let users know where the actual error
// occurred.
Expand Down
Loading
Loading