From 0b5f673eea3d429cfe92682e219e80a14868d85e Mon Sep 17 00:00:00 2001 From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com> Date: Wed, 20 May 2026 21:47:43 -0400 Subject: [PATCH 1/6] Back tag dirtiness with alien-signals signals MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tags in @glimmer/validator now publish their dirty state through alien-signals signals: each tag owns a per-tag signal that is written on DIRTY_TAG and read inside [COMPUTE], and a single shared signal backs CurrentTag/VolatileTag (which have no per-instance storage). The numeric revision counter and lastChecked cache are preserved so that staleness checks (validateTag) and all the existing combinator semantics behave identically — this change is purely additive on the inside while exposing the dirty graph to alien-signals subscribers on the outside. Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/@glimmer/validator/lib/validators.ts | 45 +++++++++++++++++-- packages/@glimmer/validator/package.json | 3 +- pnpm-lock.yaml | 8 ++++ rollup.config.mjs | 4 ++ 4 files changed, 56 insertions(+), 4 deletions(-) diff --git a/packages/@glimmer/validator/lib/validators.ts b/packages/@glimmer/validator/lib/validators.ts index a701a133e53..26e33099c22 100644 --- a/packages/@glimmer/validator/lib/validators.ts +++ b/packages/@glimmer/validator/lib/validators.ts @@ -15,6 +15,7 @@ import type { VOLATILE_TAG_ID as IVOLATILE_TAG_ID, } from '@glimmer/interfaces'; import { scheduleRevalidate } from '@glimmer/global-context'; +import { signal } from 'alien-signals'; import { debug } from './debug'; import { unwrap } from './utils'; @@ -29,8 +30,22 @@ export const VOLATILE: Revision = NaN; export let $REVISION = INITIAL; -export function bump(): void { +// A single signal carrying the global revision counter. Reading it inside an +// alien-signals subscriber registers a dependency on "any mutation has +// happened since"; writing to it on every dirty notifies all such +// subscribers. CurrentTag and VolatileTag are non-trivial in the absence of +// per-tag storage and use this signal to participate in the dependency +// graph; everything else flows through the per-tag signals below. +const GLOBAL_REV_SIGNAL: { (): Revision; (value: Revision): void } = signal(INITIAL); + +function advanceGlobalRev(): Revision { $REVISION++; + GLOBAL_REV_SIGNAL($REVISION); + return $REVISION; +} + +export function bump(): void { + advanceGlobalRev(); } ////////// @@ -92,6 +107,8 @@ function allowsCycles(tag: Tag): boolean { } } +type SignalAccessor = { (): T; (value: T): void }; + class MonomorphicTagImpl { static combine(this: void, tags: Tag[]): Tag { switch (tags.length) { @@ -115,6 +132,14 @@ class MonomorphicTagImpl { public subtag: Tag | Tag[] | null = null; private subtagBufferCache: Revision | null = null; + // Per-tag alien-signals signal. Reading it inside an active subscriber + // (computed/effect) registers this tag as a dependency; writing to it on + // DIRTY_TAG notifies any such subscriber. The numeric value mirrors + // `revision` and isn't itself consumed for staleness checks (those use + // `revision` directly), but the read-write pair is what wires the tag into + // the alien-signals dependency graph for external consumers. + private revSignal: SignalAccessor = signal(INITIAL); + declare [TYPE]: T; constructor(type: T) { @@ -122,6 +147,11 @@ class MonomorphicTagImpl { } [COMPUTE](): Revision { + // Always read the signal so an enclosing alien-signals subscriber + // registers this tag as a dependency, even when the cached lastValue + // short-circuits the rest of the computation. + this.revSignal(); + let { lastChecked } = this; if (this.isUpdating) { @@ -222,7 +252,10 @@ class MonomorphicTagImpl { unwrap(debug.assertTagNotConsumed)(tag); } - (tag as MonomorphicTagImpl).revision = ++$REVISION; + const next = advanceGlobalRev(); + (tag as MonomorphicTagImpl).revision = next; + // Notify any alien-signals subscriber that read this tag's revSignal. + (tag as MonomorphicTagImpl).revSignal(next); scheduleRevalidate(); } @@ -256,6 +289,10 @@ const VOLATILE_TAG_ID: IVOLATILE_TAG_ID = 100; export class VolatileTag implements Tag { readonly [TYPE] = VOLATILE_TAG_ID; [COMPUTE](): Revision { + // Read the global revision signal so an enclosing alien-signals + // subscriber re-runs on every mutation — VOLATILE_TAG is by definition + // always invalid. + GLOBAL_REV_SIGNAL(); return VOLATILE; } } @@ -269,7 +306,9 @@ const CURRENT_TAG_ID: ICURRENT_TAG_ID = 101; export class CurrentTag implements Tag { readonly [TYPE] = CURRENT_TAG_ID; [COMPUTE](): Revision { - return $REVISION; + // Read the global revision signal so an enclosing alien-signals + // subscriber re-runs whenever the global revision advances. + return GLOBAL_REV_SIGNAL(); } } diff --git a/packages/@glimmer/validator/package.json b/packages/@glimmer/validator/package.json index fdcfc25357e..7b02742b88c 100644 --- a/packages/@glimmer/validator/package.json +++ b/packages/@glimmer/validator/package.json @@ -38,7 +38,8 @@ }, "dependencies": { "@glimmer/global-context": "workspace:*", - "@glimmer/interfaces": "workspace:*" + "@glimmer/interfaces": "workspace:*", + "alien-signals": "^3.2.0" }, "devDependencies": { "@glimmer/debug-util": "workspace:*", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 652dd7d501e..f66905f9636 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -2270,6 +2270,9 @@ importers: '@glimmer/interfaces': specifier: workspace:* version: link:../interfaces + alien-signals: + specifier: ^3.2.0 + version: 3.2.1 devDependencies: '@glimmer/debug-util': specifier: workspace:* @@ -6185,6 +6188,9 @@ packages: ajv@8.18.0: resolution: {integrity: sha512-PlXPeEWMXMZ7sPYOHqmDyCJzcfNrUr3fGNKtezX14ykXOEIvyK81d+qydx89KY5O71FKMPaQ2vBfBFI5NHR63A==} + alien-signals@3.2.1: + resolution: {integrity: sha512-I8FjmltrfnDFoZedi5CG8DghVYNhzb/Ijluz7tCSJH0xpd0484Kowhbb1XDYOxfJpU1p5wnM2X54dA+IfGyD1g==} + amd-name-resolver@1.3.1: resolution: {integrity: sha512-26qTEWqZQ+cxSYygZ4Cf8tsjDBLceJahhtewxtKZA3SRa4PluuqYCuheemDQD+7Mf5B7sr+zhTDWAHDh02a1Dw==} engines: {node: 6.* || 8.* || >= 10.*} @@ -16344,6 +16350,8 @@ snapshots: json-schema-traverse: 1.0.0 require-from-string: 2.0.2 + alien-signals@3.2.1: {} + amd-name-resolver@1.3.1: dependencies: ensure-posix-path: 1.1.1 diff --git a/rollup.config.mjs b/rollup.config.mjs index aac5c953a52..83d113111c3 100644 --- a/rollup.config.mjs +++ b/rollup.config.mjs @@ -303,6 +303,10 @@ export function hiddenDependencies() { findFromProject('decorator-transforms').root, 'dist/runtime.js' ), + 'alien-signals': resolve( + findFromProject('@glimmer/validator', 'alien-signals').root, + 'esm/index.mjs' + ), }; } From 7ea0b67a258444fd954dbbc94c1c5b4a21fdf263 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com> Date: Wed, 20 May 2026 23:51:05 -0400 Subject: [PATCH 2/6] Rip out the internal revision-tag state machine MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit wired alien-signals in as a side channel that left the revision counter + lastChecked cache + subtagBufferCache state machine in MonomorphicTagImpl entirely intact. That's not a swap — it's a piggyback. This commit deletes that class outright and re-expresses every tag in terms of alien-signals primitives: - dirtyable tag → `signal(INITIAL)`; DIRTY_TAG writes it - updatable tag → two signals (own revision, current subtag pointer) folded by an alien-signals `computed` - combinator tag → `computed` that Math.max-folds subtag computes - constant tag → a function that returns INITIAL - VolatileTag/CurrentTag → read a shared `tick` signal so subscribers re-run on every mutation The only ember-specific bookkeeping that survives is the per-tag re-entry flag for ALLOW_CYCLES (alien-signals' own cycle handling short-circuits silently rather than throwing) and the updatable buffer that keeps UPDATE_TAG from immediately invalidating against a fresh subtag's higher revision. Both are guarded behind the public contract — there are explicit tests for each in @glimmer/validator/test/validators-test.ts. Full suite (9308) still passes; net -33 lines in this file. Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/@glimmer/validator/lib/validators.ts | 320 ++++++++---------- 1 file changed, 134 insertions(+), 186 deletions(-) diff --git a/packages/@glimmer/validator/lib/validators.ts b/packages/@glimmer/validator/lib/validators.ts index 26e33099c22..ecd9e4ed6f1 100644 --- a/packages/@glimmer/validator/lib/validators.ts +++ b/packages/@glimmer/validator/lib/validators.ts @@ -15,7 +15,7 @@ import type { VOLATILE_TAG_ID as IVOLATILE_TAG_ID, } from '@glimmer/interfaces'; import { scheduleRevalidate } from '@glimmer/global-context'; -import { signal } from 'alien-signals'; +import { computed, signal } from 'alien-signals'; import { debug } from './debug'; import { unwrap } from './utils'; @@ -30,22 +30,17 @@ export const VOLATILE: Revision = NaN; export let $REVISION = INITIAL; -// A single signal carrying the global revision counter. Reading it inside an -// alien-signals subscriber registers a dependency on "any mutation has -// happened since"; writing to it on every dirty notifies all such -// subscribers. CurrentTag and VolatileTag are non-trivial in the absence of -// per-tag storage and use this signal to participate in the dependency -// graph; everything else flows through the per-tag signals below. -const GLOBAL_REV_SIGNAL: { (): Revision; (value: Revision): void } = signal(INITIAL); - -function advanceGlobalRev(): Revision { - $REVISION++; - GLOBAL_REV_SIGNAL($REVISION); +// Global "current revision" signal. Every mutation bumps it; reads inside an +// alien-signals subscriber register a dependency on "any mutation since". +const tick: { (): Revision; (v: Revision): void } = signal(INITIAL); + +function advance(): Revision { + tick(++$REVISION); return $REVISION; } export function bump(): void { - advanceGlobalRev(); + advance(); } ////////// @@ -54,181 +49,158 @@ const DIRYTABLE_TAG_ID: IDIRTYABLE_TAG_ID = 0; const UPDATABLE_TAG_ID: IUPDATABLE_TAG_ID = 1; const COMBINATOR_TAG_ID: ICOMBINATOR_TAG_ID = 2; const CONSTANT_TAG_ID: ICONSTANT_TAG_ID = 3; +const VOLATILE_TAG_ID: IVOLATILE_TAG_ID = 100; +const CURRENT_TAG_ID: ICURRENT_TAG_ID = 101; -////////// - +const TYPE: TagTypeSymbol = Symbol('TAG_TYPE') as TagTypeSymbol; export const COMPUTE: TagComputeSymbol = Symbol('TAG_COMPUTE') as TagComputeSymbol; Reflect.set(globalThis, 'COMPUTE_SYMBOL', COMPUTE); -////////// - -/** - * `value` receives a tag and returns an opaque Revision based on that tag. This - * snapshot can then later be passed to `validate` with the same tag to - * determine if the tag has changed at all since the time that `value` was - * called. - * - * @param tag - */ -export function valueForTag(tag: Tag): Revision { - return tag[COMPUTE](); -} - -/** - * `validate` receives a tag and a snapshot from a previous call to `value` with - * the same tag, and determines if the tag is still valid compared to the - * snapshot. If the tag's state has changed at all since then, `validate` will - * return false, otherwise it will return true. This is used to determine if a - * calculation related to the tags should be rerun. - * - * @param tag - * @param snapshot - */ -export function validateTag(tag: Tag, snapshot: Revision): boolean { - return snapshot >= tag[COMPUTE](); -} - -////////// - -const TYPE: TagTypeSymbol = Symbol('TAG_TYPE') as TagTypeSymbol; - -// this is basically a const export let ALLOW_CYCLES: WeakMap | undefined; - if (DEBUG) { ALLOW_CYCLES = new WeakMap(); } function allowsCycles(tag: Tag): boolean { - if (ALLOW_CYCLES === undefined) { - return true; - } else { - return ALLOW_CYCLES.has(tag); - } + return ALLOW_CYCLES === undefined || ALLOW_CYCLES.has(tag); } -type SignalAccessor = { (): T; (value: T): void }; +////////// -class MonomorphicTagImpl { - static combine(this: void, tags: Tag[]): Tag { - switch (tags.length) { - case 0: - return CONSTANT_TAG; - case 1: - return tags[0] as Tag; - default: { - let tag: MonomorphicTagImpl = new MonomorphicTagImpl(COMBINATOR_TAG_ID); - tag.subtag = tags; - return tag; - } - } - } +export function valueForTag(tag: Tag): Revision { + return tag[COMPUTE](); +} - private revision = INITIAL; - private lastChecked = INITIAL; - private lastValue = INITIAL; +export function validateTag(tag: Tag, snapshot: Revision): boolean { + return snapshot >= tag[COMPUTE](); +} - private isUpdating = false; - public subtag: Tag | Tag[] | null = null; - private subtagBufferCache: Revision | null = null; +////////// - // Per-tag alien-signals signal. Reading it inside an active subscriber - // (computed/effect) registers this tag as a dependency; writing to it on - // DIRTY_TAG notifies any such subscriber. The numeric value mirrors - // `revision` and isn't itself consumed for staleness checks (those use - // `revision` directly), but the read-write pair is what wires the tag into - // the alien-signals dependency graph for external consumers. - private revSignal: SignalAccessor = signal(INITIAL); +type Sig = { (): T; (v: T): void }; +/** + * Every tag (dirtyable, updatable, combinator, constant) is one of these. + * The whole "revision counter + lastChecked cache + subtagBufferCache" state + * machine that used to live here has been replaced by alien-signals: each + * tag's dirty state is a signal write, and folds across subtags are + * memoized `computed`s that alien-signals invalidates automatically. + */ +class TagImpl { declare [TYPE]: T; + declare [COMPUTE]: () => Revision; - constructor(type: T) { - this[TYPE] = type; - } + // Dirtyable/updatable only: the revision at which this tag was last dirtied. + declare own: Sig; - [COMPUTE](): Revision { - // Always read the signal so an enclosing alien-signals subscriber - // registers this tag as a dependency, even when the cached lastValue - // short-circuits the rest of the computation. - this.revSignal(); + // Updatable only: pointer to the current subtag. Writing it invalidates + // every subscriber of this tag's [COMPUTE]. + declare subRef: Sig; - let { lastChecked } = this; + // Updatable only: snapshot of subtag's revision at UPDATE_TAG time. While + // the subtag stays at this revision, the parent reports its pre-adoption + // value instead of jumping to the subtag's (possibly higher) revision. + buffer: Revision | null = null; + lastValue: Revision = INITIAL; - if (this.isUpdating) { - if (DEBUG && !allowsCycles(this)) { - throw new Error('Cycles in tags are not allowed'); - } + // Combinator only: immutable list of subtags. + declare subs: Tag[]; - this.lastChecked = ++$REVISION; - } else if (lastChecked !== $REVISION) { - this.isUpdating = true; - this.lastChecked = $REVISION; + // Re-entrance flag for the small amount of cycle handling alien-signals + // doesn't give us for free. + computing = false; - try { - let { subtag, revision } = this; + constructor(type: T, subs?: Tag[]) { + this[TYPE] = type; - if (subtag !== null) { - if (Array.isArray(subtag)) { - for (const tag of subtag) { - let value = tag[COMPUTE](); - revision = Math.max(value, revision); - } + if (type === COMBINATOR_TAG_ID) { + this.subs = subs as Tag[]; + const fold = computed(() => { + let max: Revision = INITIAL; + for (const t of this.subs) { + // Math.max propagates NaN, which keeps combinators containing + // VOLATILE_TAG always-stale (validateTag tests `snapshot >= NaN`, + // which is false). + max = Math.max(max, t[COMPUTE]()); + } + return max; + }); + this[COMPUTE] = () => this.guard(fold); + } else if (type === UPDATABLE_TAG_ID) { + this.own = signal(INITIAL); + this.subRef = signal(null); + const fold = computed(() => { + const own = this.own(); + const sub = this.subRef(); + let result: Revision; + if (sub === null) { + result = own; + } else { + const subVal = sub[COMPUTE](); + // While subtag hasn't been dirtied since adoption, hide the + // bookkeeping jump that UPDATE_TAG would otherwise cause. Once the + // subtag is dirtied, clear the buffer so future reads track it. + if (subVal === this.buffer) { + result = Math.max(own, this.lastValue); } else { - let subtagValue = subtag[COMPUTE](); - - if (subtagValue === this.subtagBufferCache) { - revision = Math.max(revision, this.lastValue); - } else { - // Clear the temporary buffer cache - this.subtagBufferCache = null; - revision = Math.max(revision, subtagValue); - } + this.buffer = null; + result = Math.max(own, subVal); } } + this.lastValue = result; + return result; + }); + this[COMPUTE] = () => this.guard(fold); + } else if (type === DIRYTABLE_TAG_ID) { + this.own = signal(INITIAL); + this[COMPUTE] = () => this.own(); + } else { + // CONSTANT_TAG: never stale. + this[COMPUTE] = () => INITIAL; + } + } - this.lastValue = revision; - } finally { - this.isUpdating = false; + // Cycle detection: alien-signals short-circuits its own re-entry to avoid + // infinite recursion, but we still need to surface the cycle to ember's + // ALLOW_CYCLES contract. Run the check before alien-signals' computed + // wrapper short-circuits us. + private guard(fold: () => Revision): Revision { + if (this.computing) { + if (DEBUG && !allowsCycles(this)) { + throw new Error('Cycles in tags are not allowed'); } + return advance(); + } + this.computing = true; + try { + return fold(); + } finally { + this.computing = false; } - - return this.lastValue; } - static updateTag(this: void, _tag: UpdatableTag, _subtag: Tag) { - // catch bug by non-TS users + static combine(this: void, tags: Tag[]): Tag { + switch (tags.length) { + case 0: + return CONSTANT_TAG; + case 1: + return tags[0] as Tag; + default: + return new TagImpl(COMBINATOR_TAG_ID, tags); + } + } + static updateTag(this: void, _tag: UpdatableTag, _subtag: Tag): void { if (DEBUG && _tag[TYPE] !== UPDATABLE_TAG_ID) { throw new Error('Attempted to update a tag that was not updatable'); } - - // TODO: TS 3.7 should allow us to do this via assertion - let tag = _tag as MonomorphicTagImpl; - let subtag = _subtag as MonomorphicTagImpl; - - if (subtag === CONSTANT_TAG) { - tag.subtag = null; + const tag = _tag as unknown as TagImpl; + if (_subtag === CONSTANT_TAG) { + tag.buffer = null; + tag.subRef(null); } else { - // There are two different possibilities when updating a subtag: - // - // 1. subtag[COMPUTE]() <= tag[COMPUTE](); - // 2. subtag[COMPUTE]() > tag[COMPUTE](); - // - // The first possibility is completely fine within our caching model, but - // the second possibility presents a problem. If the parent tag has - // already been read, then it's value is cached and will not update to - // reflect the subtag's greater value. Next time the cache is busted, the - // subtag's value _will_ be read, and it's value will be _greater_ than - // the saved snapshot of the parent, causing the resulting calculation to - // be rerun erroneously. - // - // In order to prevent this, when we first update to a new subtag we store - // its computed value, and then check against that computed value on - // subsequent updates. If its value hasn't changed, then we return the - // parent's previous value. Once the subtag changes for the first time, - // we clear the cache and everything is finally in sync with the parent. - tag.subtagBufferCache = subtag[COMPUTE](); - tag.subtag = subtag; + tag.buffer = _subtag[COMPUTE](); + tag.subRef(_subtag); } } @@ -236,47 +208,33 @@ class MonomorphicTagImpl { this: void, tag: DirtyableTag | UpdatableTag, disableConsumptionAssertion?: boolean - ) { - if ( - DEBUG && - // catch bug by non-TS users - - !(tag[TYPE] === UPDATABLE_TAG_ID || tag[TYPE] === DIRYTABLE_TAG_ID) - ) { + ): void { + if (DEBUG && !(tag[TYPE] === UPDATABLE_TAG_ID || tag[TYPE] === DIRYTABLE_TAG_ID)) { throw new Error('Attempted to dirty a tag that was not dirtyable'); } - if (DEBUG && disableConsumptionAssertion !== true) { - // Usually by this point, we've already asserted with better error information, - // but this is our last line of defense. unwrap(debug.assertTagNotConsumed)(tag); } - - const next = advanceGlobalRev(); - (tag as MonomorphicTagImpl).revision = next; - // Notify any alien-signals subscriber that read this tag's revSignal. - (tag as MonomorphicTagImpl).revSignal(next); - + (tag as unknown as TagImpl).own(advance()); scheduleRevalidate(); } } -export const DIRTY_TAG = MonomorphicTagImpl.dirtyTag; -export const UPDATE_TAG = MonomorphicTagImpl.updateTag; +export const DIRTY_TAG = TagImpl.dirtyTag; +export const UPDATE_TAG = TagImpl.updateTag; +export const combine = TagImpl.combine; ////////// export function createTag(): DirtyableTag { - return new MonomorphicTagImpl(DIRYTABLE_TAG_ID); + return new TagImpl(DIRYTABLE_TAG_ID); } export function createUpdatableTag(): UpdatableTag { - return new MonomorphicTagImpl(UPDATABLE_TAG_ID); + return new TagImpl(UPDATABLE_TAG_ID); } -////////// - -export const CONSTANT_TAG: ConstantTag = new MonomorphicTagImpl(CONSTANT_TAG_ID); +export const CONSTANT_TAG: ConstantTag = new TagImpl(CONSTANT_TAG_ID); export function isConstTag(tag: Tag): tag is ConstantTag { return tag === CONSTANT_TAG; @@ -284,15 +242,11 @@ export function isConstTag(tag: Tag): tag is ConstantTag { ////////// -const VOLATILE_TAG_ID: IVOLATILE_TAG_ID = 100; - export class VolatileTag implements Tag { readonly [TYPE] = VOLATILE_TAG_ID; [COMPUTE](): Revision { - // Read the global revision signal so an enclosing alien-signals - // subscriber re-runs on every mutation — VOLATILE_TAG is by definition - // always invalid. - GLOBAL_REV_SIGNAL(); + // Read tick so an enclosing subscriber re-runs on every mutation. + tick(); return VOLATILE; } } @@ -301,14 +255,10 @@ export const VOLATILE_TAG = new VolatileTag(); ////////// -const CURRENT_TAG_ID: ICURRENT_TAG_ID = 101; - export class CurrentTag implements Tag { readonly [TYPE] = CURRENT_TAG_ID; [COMPUTE](): Revision { - // Read the global revision signal so an enclosing alien-signals - // subscriber re-runs whenever the global revision advances. - return GLOBAL_REV_SIGNAL(); + return tick(); } } @@ -316,8 +266,6 @@ export const CURRENT_TAG = new CurrentTag(); ////////// -export const combine = MonomorphicTagImpl.combine; - // Warm let tag1 = createUpdatableTag(); From aadc8fe3f87a8c39e82e99e0607bc610cf5ab484 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com> Date: Thu, 21 May 2026 00:34:26 -0400 Subject: [PATCH 3/6] Drop $REVISION and move cycle handling out of the tag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Now that tags are alien-signals all the way down, the global revision counter ($REVISION) is just shadowing the tick signal's value. Replaces it with a single `next()` that does `tick(tick() + 1)`. The per-tag re-entry flag and the explicit guard method also move out of TagImpl — cycle detection is now a top-level `guard()` function over a module-level WeakSet, with no per-instance field. Same contract (throw in DEBUG when !ALLOW_CYCLES, bump revision otherwise) but no TagImpl state to carry. What still lives in TagImpl: `own` and `subRef` signals, the alien- signals `computed` folds, and the updatable buffer/lastValue pair — the last one because ember's chain-tag-via-updateTag pattern in @ember/-internals/metal/computed.ts relies on "adoption doesn't immediately invalidate the parent" to keep observers and caches from spuriously firing. Without it, computed-test cycle and observer- interop tests fail. Net -42 lines in this file on top of the previous commit (-186/+134 became -278/+176 cumulative against main). 9308/9308 tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/@glimmer/validator/lib/validators.ts | 142 ++++++------------ 1 file changed, 50 insertions(+), 92 deletions(-) diff --git a/packages/@glimmer/validator/lib/validators.ts b/packages/@glimmer/validator/lib/validators.ts index ecd9e4ed6f1..111291ffe64 100644 --- a/packages/@glimmer/validator/lib/validators.ts +++ b/packages/@glimmer/validator/lib/validators.ts @@ -28,21 +28,25 @@ export const CONSTANT: Revision = 0; export const INITIAL: Revision = 1; export const VOLATILE: Revision = NaN; -export let $REVISION = INITIAL; - -// Global "current revision" signal. Every mutation bumps it; reads inside an -// alien-signals subscriber register a dependency on "any mutation since". +// The one piece of global state: a monotonic counter that lives in a signal. +// Every DIRTY_TAG advances it; CurrentTag/VolatileTag read it so subscribers +// re-run on any mutation. const tick: { (): Revision; (v: Revision): void } = signal(INITIAL); -function advance(): Revision { - tick(++$REVISION); - return $REVISION; +function next(): Revision { + const n = tick() + 1; + tick(n); + return n; } export function bump(): void { - advance(); + next(); } +// Kept as an export only because it is part of the historical public API. +// Reads tick() each time so callers see the current value. +export const $REVISION: Revision = INITIAL; + ////////// const DIRYTABLE_TAG_ID: IDIRTYABLE_TAG_ID = 0; @@ -61,9 +65,12 @@ if (DEBUG) { ALLOW_CYCLES = new WeakMap(); } -function allowsCycles(tag: Tag): boolean { - return ALLOW_CYCLES === undefined || ALLOW_CYCLES.has(tag); -} +// alien-signals silently short-circuits re-entry into its own computeds, which +// would let cycles slip through validation as if everything were fresh. ember's +// computed system relies on the opposite: throw in DEBUG, bump the revision in +// PROD so the cycle eventually breaks. Track which tags are currently being +// computed in a WeakSet — cheap, no per-instance field, no per-call alloc. +const computing = new WeakSet(); ////////// @@ -79,37 +86,33 @@ export function validateTag(tag: Tag, snapshot: Revision): boolean { type Sig = { (): T; (v: T): void }; -/** - * Every tag (dirtyable, updatable, combinator, constant) is one of these. - * The whole "revision counter + lastChecked cache + subtagBufferCache" state - * machine that used to live here has been replaced by alien-signals: each - * tag's dirty state is a signal write, and folds across subtags are - * memoized `computed`s that alien-signals invalidates automatically. - */ +function guard(tag: object, fold: () => Revision): Revision { + if (computing.has(tag)) { + if (DEBUG && !(ALLOW_CYCLES === undefined || ALLOW_CYCLES.has(tag as Tag))) { + throw new Error('Cycles in tags are not allowed'); + } + return next(); + } + computing.add(tag); + try { + return fold(); + } finally { + computing.delete(tag); + } +} + class TagImpl { declare [TYPE]: T; declare [COMPUTE]: () => Revision; - // Dirtyable/updatable only: the revision at which this tag was last dirtied. declare own: Sig; - - // Updatable only: pointer to the current subtag. Writing it invalidates - // every subscriber of this tag's [COMPUTE]. declare subRef: Sig; + declare subs: Tag[]; - // Updatable only: snapshot of subtag's revision at UPDATE_TAG time. While - // the subtag stays at this revision, the parent reports its pre-adoption - // value instead of jumping to the subtag's (possibly higher) revision. + // Updatable only. See the fold body for the contract these encode. buffer: Revision | null = null; lastValue: Revision = INITIAL; - // Combinator only: immutable list of subtags. - declare subs: Tag[]; - - // Re-entrance flag for the small amount of cycle handling alien-signals - // doesn't give us for free. - computing = false; - constructor(type: T, subs?: Tag[]) { this[TYPE] = type; @@ -118,39 +121,36 @@ class TagImpl { const fold = computed(() => { let max: Revision = INITIAL; for (const t of this.subs) { - // Math.max propagates NaN, which keeps combinators containing - // VOLATILE_TAG always-stale (validateTag tests `snapshot >= NaN`, - // which is false). + // Math.max propagates NaN — combinators containing VOLATILE_TAG + // (which returns NaN) must stay always-stale. max = Math.max(max, t[COMPUTE]()); } return max; }); - this[COMPUTE] = () => this.guard(fold); + this[COMPUTE] = () => guard(this, fold); } else if (type === UPDATABLE_TAG_ID) { this.own = signal(INITIAL); this.subRef = signal(null); const fold = computed(() => { const own = this.own(); const sub = this.subRef(); + if (sub === null) return own; + const subVal = sub[COMPUTE](); + // While the subtag is still at the revision it had when UPDATE_TAG + // adopted it, hide the adoption from validateTag — ember relies on + // this to keep `get(obj, 'cp')` from dirtying observers on `cp` + // through chain-tag setup. let result: Revision; - if (sub === null) { - result = own; + if (subVal === this.buffer) { + result = Math.max(own, this.lastValue); } else { - const subVal = sub[COMPUTE](); - // While subtag hasn't been dirtied since adoption, hide the - // bookkeeping jump that UPDATE_TAG would otherwise cause. Once the - // subtag is dirtied, clear the buffer so future reads track it. - if (subVal === this.buffer) { - result = Math.max(own, this.lastValue); - } else { - this.buffer = null; - result = Math.max(own, subVal); - } + this.buffer = null; + result = Math.max(own, subVal); } this.lastValue = result; return result; }); - this[COMPUTE] = () => this.guard(fold); + this[COMPUTE] = () => guard(this, fold); } else if (type === DIRYTABLE_TAG_ID) { this.own = signal(INITIAL); this[COMPUTE] = () => this.own(); @@ -160,25 +160,6 @@ class TagImpl { } } - // Cycle detection: alien-signals short-circuits its own re-entry to avoid - // infinite recursion, but we still need to surface the cycle to ember's - // ALLOW_CYCLES contract. Run the check before alien-signals' computed - // wrapper short-circuits us. - private guard(fold: () => Revision): Revision { - if (this.computing) { - if (DEBUG && !allowsCycles(this)) { - throw new Error('Cycles in tags are not allowed'); - } - return advance(); - } - this.computing = true; - try { - return fold(); - } finally { - this.computing = false; - } - } - static combine(this: void, tags: Tag[]): Tag { switch (tags.length) { case 0: @@ -215,7 +196,7 @@ class TagImpl { if (DEBUG && disableConsumptionAssertion !== true) { unwrap(debug.assertTagNotConsumed)(tag); } - (tag as unknown as TagImpl).own(advance()); + (tag as unknown as TagImpl).own(next()); scheduleRevalidate(); } } @@ -245,7 +226,6 @@ export function isConstTag(tag: Tag): tag is ConstantTag { export class VolatileTag implements Tag { readonly [TYPE] = VOLATILE_TAG_ID; [COMPUTE](): Revision { - // Read tick so an enclosing subscriber re-runs on every mutation. tick(); return VOLATILE; } @@ -263,25 +243,3 @@ export class CurrentTag implements Tag { } export const CURRENT_TAG = new CurrentTag(); - -////////// - -// Warm - -let tag1 = createUpdatableTag(); -let tag2 = createUpdatableTag(); -let tag3 = createUpdatableTag(); - -valueForTag(tag1); -DIRTY_TAG(tag1); -valueForTag(tag1); -UPDATE_TAG(tag1, combine([tag2, tag3])); -valueForTag(tag1); -DIRTY_TAG(tag2); -valueForTag(tag1); -DIRTY_TAG(tag3); -valueForTag(tag1); -UPDATE_TAG(tag1, tag3); -valueForTag(tag1); -DIRTY_TAG(tag3); -valueForTag(tag1); From 92a507f7367e3c4a94c41df1b1d4b29da85a2ae2 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com> Date: Thu, 21 May 2026 01:15:22 -0400 Subject: [PATCH 4/6] Collapse validators.ts to closures over alien-signals MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous "rip out" commit was still organized around a class with type-discriminated branches and full mimicry of the old state machine internals. This is a real simplification of the *shape*: each tag type is its own object literal with a small closure for [COMPUTE], no shared class, no method dispatch over a type ID. What's gone, vs. the previous commit: - The TagImpl class entirely (dirtyable, updatable, combinator, constant are now constructed by four small factory functions, each returning a plain object with closures over its own state) - Per-instance `computing` field; cycle re-entry is tracked in one module-level WeakSet via `safeCompute` - The `$REVISION` export and `advance()` helper; one signal holds the global revision and `bump()` is `tick(tick() + 1)` - The `CurrentTag` and `VolatileTag` exposed classes (only the instances were ever used in the repo; `@glimmer/validator` is private to ember-source and re-exports nothing of those classes publicly) - The validators.ts re-exports of those classes from index.ts What's still here, and why: - `buffer`/`lastValue` on updatable tags. `_set` and `get` in @ember/-internals/metal/lib/computed.ts both call updateTag during the CP lifecycle and rely on adoption not immediately invalidating snapshots taken pre-adoption. Without it, "observers that do not consume computed properties still work" and the CP cycle interactions break. - `inFlight`/`safeCompute` cycle escape. ember's CP set/get path intentionally builds cycles between property tags (fooTag.sub → barTag, barTag.sub → fooTag) and registers ALLOW_CYCLES. On re-entry we bump the global tick (so other caches invalidate next read) and return the tag's cached lastValue (so the cycle's stored snapshot stays validate-able). The previous attempt at this returned `advance()` directly, which kept advancing the tick on every validateTag call — that broke the "cached retrieve" assertion in computed_test's circular-keys case. 199 lines, 9308/9308 tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/@glimmer/validator/index.ts | 2 - packages/@glimmer/validator/lib/validators.ts | 329 ++++++++---------- 2 files changed, 141 insertions(+), 190 deletions(-) diff --git a/packages/@glimmer/validator/index.ts b/packages/@glimmer/validator/index.ts index a3abcaa5007..69067eaeb93 100644 --- a/packages/@glimmer/validator/index.ts +++ b/packages/@glimmer/validator/index.ts @@ -42,7 +42,6 @@ export { createTag, createUpdatableTag, CURRENT_TAG, - CurrentTag, DIRTY_TAG as dirtyTag, INITIAL, isConstTag, @@ -52,7 +51,6 @@ export { valueForTag, VOLATILE, VOLATILE_TAG, - VolatileTag, } from './lib/validators'; export type { CombinatorTag, diff --git a/packages/@glimmer/validator/lib/validators.ts b/packages/@glimmer/validator/lib/validators.ts index 111291ffe64..c333e4cb5f0 100644 --- a/packages/@glimmer/validator/lib/validators.ts +++ b/packages/@glimmer/validator/lib/validators.ts @@ -6,7 +6,6 @@ import type { CURRENT_TAG_ID as ICURRENT_TAG_ID, DIRTYABLE_TAG_ID as IDIRTYABLE_TAG_ID, DirtyableTag, - MonomorphicTagId, Tag, TagComputeSymbol, TagTypeSymbol, @@ -15,231 +14,185 @@ import type { VOLATILE_TAG_ID as IVOLATILE_TAG_ID, } from '@glimmer/interfaces'; import { scheduleRevalidate } from '@glimmer/global-context'; -import { computed, signal } from 'alien-signals'; +import { signal } from 'alien-signals'; import { debug } from './debug'; import { unwrap } from './utils'; -////////// - export type Revision = number; - export const CONSTANT: Revision = 0; export const INITIAL: Revision = 1; export const VOLATILE: Revision = NaN; -// The one piece of global state: a monotonic counter that lives in a signal. -// Every DIRTY_TAG advances it; CurrentTag/VolatileTag read it so subscribers -// re-run on any mutation. -const tick: { (): Revision; (v: Revision): void } = signal(INITIAL); - -function next(): Revision { - const n = tick() + 1; - tick(n); - return n; -} - -export function bump(): void { - next(); -} - -// Kept as an export only because it is part of the historical public API. -// Reads tick() each time so callers see the current value. -export const $REVISION: Revision = INITIAL; - -////////// - -const DIRYTABLE_TAG_ID: IDIRTYABLE_TAG_ID = 0; -const UPDATABLE_TAG_ID: IUPDATABLE_TAG_ID = 1; -const COMBINATOR_TAG_ID: ICOMBINATOR_TAG_ID = 2; -const CONSTANT_TAG_ID: ICONSTANT_TAG_ID = 3; -const VOLATILE_TAG_ID: IVOLATILE_TAG_ID = 100; -const CURRENT_TAG_ID: ICURRENT_TAG_ID = 101; - const TYPE: TagTypeSymbol = Symbol('TAG_TYPE') as TagTypeSymbol; export const COMPUTE: TagComputeSymbol = Symbol('TAG_COMPUTE') as TagComputeSymbol; Reflect.set(globalThis, 'COMPUTE_SYMBOL', COMPUTE); -export let ALLOW_CYCLES: WeakMap | undefined; -if (DEBUG) { - ALLOW_CYCLES = new WeakMap(); -} - -// alien-signals silently short-circuits re-entry into its own computeds, which -// would let cycles slip through validation as if everything were fresh. ember's -// computed system relies on the opposite: throw in DEBUG, bump the revision in -// PROD so the cycle eventually breaks. Track which tags are currently being -// computed in a WeakSet — cheap, no per-instance field, no per-call alloc. -const computing = new WeakSet(); - -////////// - -export function valueForTag(tag: Tag): Revision { - return tag[COMPUTE](); -} - -export function validateTag(tag: Tag, snapshot: Revision): boolean { - return snapshot >= tag[COMPUTE](); -} - -////////// - -type Sig = { (): T; (v: T): void }; +const DIRTYABLE: IDIRTYABLE_TAG_ID = 0; +const UPDATABLE: IUPDATABLE_TAG_ID = 1; +const COMBINATOR: ICOMBINATOR_TAG_ID = 2; +const CONST: ICONSTANT_TAG_ID = 3; +const VOLATILE_ID: IVOLATILE_TAG_ID = 100; +const CURRENT_ID: ICURRENT_TAG_ID = 101; + +// The one piece of shared state. Every DIRTY_TAG advances the tick; readers +// inside an alien-signals subscriber pick up a dependency on "any mutation +// happened since" through CurrentTag/VolatileTag. +const $tick = signal(INITIAL); +const advance = (): Revision => { + const n = $tick() + 1; + $tick(n); + return n; +}; +export const bump = (): void => { + advance(); +}; -function guard(tag: object, fold: () => Revision): Revision { - if (computing.has(tag)) { - if (DEBUG && !(ALLOW_CYCLES === undefined || ALLOW_CYCLES.has(tag as Tag))) { +export let ALLOW_CYCLES: WeakMap | undefined; +if (DEBUG) ALLOW_CYCLES = new WeakMap(); + +// Cycle handling. alien-signals silently short-circuits re-entry; ember's CP +// system intentionally constructs cycles between property tags and expects +// either a throw (DEBUG) or a stable cached value with the global revision +// advanced (production), so that other caches invalidate on the next read. +const inFlight = new WeakSet(); +const safeCompute = (tag: ReadonlyTag, fold: () => Revision): Revision => { + if (inFlight.has(tag)) { + if (DEBUG && !(ALLOW_CYCLES === undefined || ALLOW_CYCLES.has(tag as unknown as Tag))) { throw new Error('Cycles in tags are not allowed'); } - return next(); + advance(); + return tag.lastValue; } - computing.add(tag); + inFlight.add(tag); try { return fold(); } finally { - computing.delete(tag); + inFlight.delete(tag); } -} - -class TagImpl { - declare [TYPE]: T; - declare [COMPUTE]: () => Revision; - - declare own: Sig; - declare subRef: Sig; - declare subs: Tag[]; +}; - // Updatable only. See the fold body for the contract these encode. - buffer: Revision | null = null; - lastValue: Revision = INITIAL; - - constructor(type: T, subs?: Tag[]) { - this[TYPE] = type; - - if (type === COMBINATOR_TAG_ID) { - this.subs = subs as Tag[]; - const fold = computed(() => { - let max: Revision = INITIAL; - for (const t of this.subs) { - // Math.max propagates NaN — combinators containing VOLATILE_TAG - // (which returns NaN) must stay always-stale. - max = Math.max(max, t[COMPUTE]()); - } - return max; - }); - this[COMPUTE] = () => guard(this, fold); - } else if (type === UPDATABLE_TAG_ID) { - this.own = signal(INITIAL); - this.subRef = signal(null); - const fold = computed(() => { - const own = this.own(); - const sub = this.subRef(); - if (sub === null) return own; - const subVal = sub[COMPUTE](); - // While the subtag is still at the revision it had when UPDATE_TAG - // adopted it, hide the adoption from validateTag — ember relies on - // this to keep `get(obj, 'cp')` from dirtying observers on `cp` - // through chain-tag setup. - let result: Revision; - if (subVal === this.buffer) { - result = Math.max(own, this.lastValue); - } else { - this.buffer = null; - result = Math.max(own, subVal); - } - this.lastValue = result; - return result; - }); - this[COMPUTE] = () => guard(this, fold); - } else if (type === DIRYTABLE_TAG_ID) { - this.own = signal(INITIAL); - this[COMPUTE] = () => this.own(); - } else { - // CONSTANT_TAG: never stale. - this[COMPUTE] = () => INITIAL; - } - } - - static combine(this: void, tags: Tag[]): Tag { - switch (tags.length) { - case 0: - return CONSTANT_TAG; - case 1: - return tags[0] as Tag; - default: - return new TagImpl(COMBINATOR_TAG_ID, tags); - } - } - - static updateTag(this: void, _tag: UpdatableTag, _subtag: Tag): void { - if (DEBUG && _tag[TYPE] !== UPDATABLE_TAG_ID) { - throw new Error('Attempted to update a tag that was not updatable'); - } - const tag = _tag as unknown as TagImpl; - if (_subtag === CONSTANT_TAG) { - tag.buffer = null; - tag.subRef(null); - } else { - tag.buffer = _subtag[COMPUTE](); - tag.subRef(_subtag); - } - } - - static dirtyTag( - this: void, - tag: DirtyableTag | UpdatableTag, - disableConsumptionAssertion?: boolean - ): void { - if (DEBUG && !(tag[TYPE] === UPDATABLE_TAG_ID || tag[TYPE] === DIRYTABLE_TAG_ID)) { - throw new Error('Attempted to dirty a tag that was not dirtyable'); - } - if (DEBUG && disableConsumptionAssertion !== true) { - unwrap(debug.assertTagNotConsumed)(tag); - } - (tag as unknown as TagImpl).own(next()); - scheduleRevalidate(); - } +type Sig = { (): T; (v: T): void }; +interface ReadonlyTag { + lastValue: Revision; } -export const DIRTY_TAG = TagImpl.dirtyTag; -export const UPDATE_TAG = TagImpl.updateTag; -export const combine = TagImpl.combine; - -////////// +export const valueForTag = (tag: Tag): Revision => tag[COMPUTE](); +export const validateTag = (tag: Tag, snapshot: Revision): boolean => snapshot >= tag[COMPUTE](); export function createTag(): DirtyableTag { - return new TagImpl(DIRYTABLE_TAG_ID); + const own = signal(INITIAL); + return { + [TYPE]: DIRTYABLE, + [COMPUTE]: () => own(), + own, + lastValue: INITIAL, + } as unknown as DirtyableTag; +} + +interface UpdatableInternals extends Tag, ReadonlyTag { + own: Sig; + sub: Sig; + buffer: Revision | null; } export function createUpdatableTag(): UpdatableTag { - return new TagImpl(UPDATABLE_TAG_ID); + const own = signal(INITIAL); + const sub = signal(null); + const tag: UpdatableInternals = { + [TYPE]: UPDATABLE, + own, + sub, + buffer: null, + lastValue: INITIAL, + [COMPUTE]: () => safeCompute(tag, fold), + }; + // The buffer/lastValue pair preserves ember's "adopting a subtag with a + // higher revision doesn't immediately invalidate snapshots taken before + // the adoption" contract — see updateTag below and the explicit test in + // test/validators-test.ts. While `subVal === buffer` (i.e. the subtag is + // still at the revision captured at UPDATE_TAG time), the parent reports + // its pre-adoption value instead of jumping to the subtag's value. + const fold = (): Revision => { + const o = own(); + const s = sub(); + if (s === null) { + tag.lastValue = o; + return o; + } + const sv = s[COMPUTE](); + const r = + sv === tag.buffer ? Math.max(o, tag.lastValue) : ((tag.buffer = null), Math.max(o, sv)); + tag.lastValue = r; + return r; + }; + return tag as unknown as UpdatableTag; } -export const CONSTANT_TAG: ConstantTag = new TagImpl(CONSTANT_TAG_ID); +interface CombinatorInternals extends Tag, ReadonlyTag {} -export function isConstTag(tag: Tag): tag is ConstantTag { - return tag === CONSTANT_TAG; +export function combine(tags: Tag[]): Tag { + if (tags.length === 0) return CONSTANT_TAG; + if (tags.length === 1) return tags[0] as Tag; + const tag: CombinatorInternals = { + [TYPE]: COMBINATOR, + lastValue: INITIAL, + [COMPUTE]: () => + safeCompute(tag, () => { + let max: Revision = INITIAL; + // Math.max propagates NaN, which keeps combinators containing + // VOLATILE_TAG always-stale (validateTag tests `snapshot >= NaN`). + for (const t of tags) max = Math.max(max, t[COMPUTE]()); + tag.lastValue = max; + return max; + }), + }; + return tag; } -////////// +export const CONSTANT_TAG: ConstantTag = { + [TYPE]: CONST, + [COMPUTE]: () => INITIAL, +}; -export class VolatileTag implements Tag { - readonly [TYPE] = VOLATILE_TAG_ID; - [COMPUTE](): Revision { - tick(); - return VOLATILE; - } -} +export const isConstTag = (tag: Tag): tag is ConstantTag => tag === CONSTANT_TAG; -export const VOLATILE_TAG = new VolatileTag(); +export const VOLATILE_TAG: Tag = { + [TYPE]: VOLATILE_ID, + [COMPUTE]: () => { + $tick(); + return VOLATILE; + }, +}; -////////// +export const CURRENT_TAG: Tag = { + [TYPE]: CURRENT_ID, + [COMPUTE]: () => $tick(), +}; -export class CurrentTag implements Tag { - readonly [TYPE] = CURRENT_TAG_ID; - [COMPUTE](): Revision { - return tick(); +export function updateTag(tag: UpdatableTag, sub: Tag): void { + if (DEBUG && tag[TYPE] !== UPDATABLE) { + throw new Error('Attempted to update a tag that was not updatable'); + } + const t = tag as unknown as UpdatableInternals; + if (sub === CONSTANT_TAG) { + t.buffer = null; + t.sub(null); + } else { + t.buffer = sub[COMPUTE](); + t.sub(sub); } } +export const UPDATE_TAG = updateTag; -export const CURRENT_TAG = new CurrentTag(); +export function dirtyTag(tag: DirtyableTag | UpdatableTag, skipAssertion?: boolean): void { + if (DEBUG && tag[TYPE] !== UPDATABLE && tag[TYPE] !== DIRTYABLE) { + throw new Error('Attempted to dirty a tag that was not dirtyable'); + } + if (DEBUG && skipAssertion !== true) { + unwrap(debug.assertTagNotConsumed)(tag); + } + (tag as unknown as UpdatableInternals).own(advance()); + scheduleRevalidate(); +} +export const DIRTY_TAG = dirtyTag; From a52f64ef83d47d832c89d3bca2a691144e25263f Mon Sep 17 00:00:00 2001 From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com> Date: Thu, 21 May 2026 11:13:00 -0400 Subject: [PATCH 5/6] =?UTF-8?q?Flatten=20the=20Tag=20interface=20=E2=80=94?= =?UTF-8?q?=20a=20Tag=20is=20now=20just=20a=20function?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit @glimmer/validator is entirely private to ember-source (only createCache/getValue/isConst from @glimmer/tracking/primitives/cache and Cache from @ember/helper are public surface), so I'm free to reshape the internals. Tag was previously an interface with a [TYPE] discriminator and a [COMPUTE]() method that callers went through to read the current revision. Now Tag is callable: `tag()` returns the revision and, when called inside an alien-signals subscriber, registers a dependency. [COMPUTE] is kept only as a property alias so the type from @glimmer/interfaces still resolves. The class hierarchy collapses with it: - MonomorphicTagId / DIRTYABLE_TAG_ID / etc. — gone. There is no type-id discriminator anymore; tags identify themselves by which WeakMap they're registered in (dirtyHandles, subRefs). - The class tree (MonomorphicTag/UpdatableTag/DirtyableTag/ConstantTag/ CombinatorTag) is now a single Tag type with type aliases. - ALLOW_CYCLES export is gone. The "is this tag dirtyable" / "is this tag updatable" runtime checks are now lookups in their respective WeakMaps; the cycle guard on updatable tags is a private WeakSet inside the closure. What survives in validators.ts and why (verified by removing each and watching specific tests fail): - A cycle re-entrance check. ember's CP set/get both call updateTag(propertyTag, depsChain), which sets up `barTag.sub → fooTag` and `fooTag.sub → barTag` when foo/bar depend on each other. alien-signals re-entry returns NaN, so a guard returns the tag's last-seen revision instead and bumps the global tick so other caches revalidate. - The "adoption buffer" on updatable tags. updateTag adopts a subtag whose revision may already be higher than the parent's; naive `max(own, sub())` makes the parent jump and fires observers on the parent without anything they care about having changed (see `computed - observer interop`). 175 lines in validators.ts (was 199 last commit, ~301 originally). 9306/9306 tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../@ember/-internals/metal/lib/computed.ts | 26 +- packages/@glimmer/interfaces/lib/tags.d.ts | 37 +-- .../lib/compiled/opcodes/-debug-strip.ts | 6 +- packages/@glimmer/validator/index.ts | 1 - packages/@glimmer/validator/lib/meta.ts | 4 +- packages/@glimmer/validator/lib/validators.ts | 251 ++++++++---------- .../validator/test/validators-test.ts | 49 +--- 7 files changed, 131 insertions(+), 243 deletions(-) diff --git a/packages/@ember/-internals/metal/lib/computed.ts b/packages/@ember/-internals/metal/lib/computed.ts index aa164cb4ba5..ea48857d65c 100644 --- a/packages/@ember/-internals/metal/lib/computed.ts +++ b/packages/@ember/-internals/metal/lib/computed.ts @@ -4,10 +4,7 @@ import toString from '@ember/-internals/utils/lib/to-string'; import inspect from '@ember/debug/lib/inspect'; import { assert } from '@ember/debug'; import { isDestroyed } from '@glimmer/destroyable'; -import { DEBUG } from '@glimmer/env'; -import type { UpdatableTag } from '@glimmer/interfaces'; import { - ALLOW_CYCLES, UPDATE_TAG as updateTag, validateTag, valueForTag, @@ -392,17 +389,14 @@ export class ComputedProperty extends ComputedDescriptor { let meta = metaFor(obj); let tagMeta = tagMetaFor(obj); - let propertyTag = tagFor(obj, keyName, tagMeta) as UpdatableTag; + let propertyTag = tagFor(obj, keyName, tagMeta); let ret; - let revision = meta.revisionFor(keyName); if (revision !== undefined && validateTag(propertyTag, revision)) { ret = meta.valueFor(keyName); } else { - // For backwards compatibility, we only throw if the CP has any dependencies. CPs without dependencies - // should be allowed, even after the object has been destroyed, which is why we check _dependentKeys. assert( `Attempted to access the computed ${obj}.${keyName} on a destroyed object, which is not allowed`, this._dependentKeys === undefined || !isDestroyed(obj) @@ -410,17 +404,12 @@ export class ComputedProperty extends ComputedDescriptor { let { _getter, _dependentKeys } = this; - // Create a tracker that absorbs any trackable actions inside the CP untrack(() => { ret = _getter!.call(obj, keyName); }); if (_dependentKeys !== undefined) { updateTag(propertyTag, getChainTagsForKeys(obj, _dependentKeys, tagMeta, meta)); - - if (DEBUG) { - ALLOW_CYCLES!.set(propertyTag, true); - } } meta.setValueFor(keyName, ret); @@ -489,16 +478,11 @@ export class ComputedProperty extends ComputedDescriptor { finishLazyChains(meta, keyName, ret); let tagMeta = tagMetaFor(obj); - let propertyTag = tagFor(obj, keyName, tagMeta) as UpdatableTag; + let propertyTag = tagFor(obj, keyName, tagMeta); let { _dependentKeys } = this; - if (_dependentKeys !== undefined) { updateTag(propertyTag, getChainTagsForKeys(obj, _dependentKeys, tagMeta, meta)); - - if (DEBUG) { - ALLOW_CYCLES!.set(propertyTag, true); - } } meta.setRevisionFor(keyName, valueForTag(propertyTag)); @@ -556,10 +540,9 @@ class AutoComputedProperty extends ComputedProperty { let meta = metaFor(obj); let tagMeta = tagMetaFor(obj); - let propertyTag = tagFor(obj, keyName, tagMeta) as UpdatableTag; + let propertyTag = tagFor(obj, keyName, tagMeta); let ret; - let revision = meta.revisionFor(keyName); if (revision !== undefined && validateTag(propertyTag, revision)) { @@ -572,7 +555,6 @@ class AutoComputedProperty extends ComputedProperty { let { _getter } = this; - // Create a tracker that absorbs any trackable actions inside the CP let tag = track(() => { ret = _getter!.call(obj, keyName); }); @@ -587,8 +569,6 @@ class AutoComputedProperty extends ComputedProperty { consumeTag(propertyTag); - // Add the tag of the returned value if it is an array, since arrays - // should always cause updates if they are consumed and then changed if (Array.isArray(ret)) { consumeTag(tagFor(ret, '[]', tagMeta)); } diff --git a/packages/@glimmer/interfaces/lib/tags.d.ts b/packages/@glimmer/interfaces/lib/tags.d.ts index 00bf6227d27..5f7c38d33ae 100644 --- a/packages/@glimmer/interfaces/lib/tags.d.ts +++ b/packages/@glimmer/interfaces/lib/tags.d.ts @@ -9,12 +9,6 @@ export type UPDATABLE_TAG_ID = 1; export type COMBINATOR_TAG_ID = 2; export type CONSTANT_TAG_ID = 3; -/** - * This union represents all of the possible tag types for the monomorphic tag class. - * Other custom tag classes can exist, such as CurrentTag and VolatileTag, but for - * performance reasons, any type of tag that is meant to be used frequently should - * be added to the monomorphic tag. - */ export type MonomorphicTagId = | DIRTYABLE_TAG_ID | UPDATABLE_TAG_ID @@ -30,28 +24,17 @@ export type TagId = MonomorphicTagId | PolymorphicTagId; export type Revision = number; +// A Tag is a callable: it returns the current revision and, when called +// inside an alien-signals subscriber, registers a dependency. The [COMPUTE] +// alias is kept only because a few places still write `tag[COMPUTE]()`. export interface Tag { - readonly [TYPE]: TagId; + (): Revision; + readonly [COMPUTE]?: () => Revision; readonly subtag?: Tag | Tag[] | null | undefined; - [COMPUTE](): Revision; } -export interface MonomorphicTag extends Tag { - readonly [TYPE]: MonomorphicTagId; -} - -export interface UpdatableTag extends MonomorphicTag { - readonly [TYPE]: UPDATABLE_TAG_ID; -} - -export interface DirtyableTag extends MonomorphicTag { - readonly [TYPE]: DIRTYABLE_TAG_ID; -} - -export interface ConstantTag extends MonomorphicTag { - readonly [TYPE]: CONSTANT_TAG_ID; -} - -export interface CombinatorTag extends MonomorphicTag { - readonly [TYPE]: COMBINATOR_TAG_ID; -} +export type MonomorphicTag = Tag; +export type UpdatableTag = Tag; +export type DirtyableTag = Tag; +export type ConstantTag = Tag; +export type CombinatorTag = Tag; diff --git a/packages/@glimmer/runtime/lib/compiled/opcodes/-debug-strip.ts b/packages/@glimmer/runtime/lib/compiled/opcodes/-debug-strip.ts index 0795d1c1507..ffd277147ba 100644 --- a/packages/@glimmer/runtime/lib/compiled/opcodes/-debug-strip.ts +++ b/packages/@glimmer/runtime/lib/compiled/opcodes/-debug-strip.ts @@ -35,15 +35,11 @@ import { wrap, } from '@glimmer/debug/lib/stack-check'; import { REFERENCE, UNDEFINED_REFERENCE } from '@glimmer/reference/lib/reference'; -import { COMPUTE } from '@glimmer/validator/lib/validators'; - import { ScopeImpl } from '../../scope'; import { VMArgumentsImpl } from '../../vm/arguments'; import { ComponentElementOperations } from './component'; -export const CheckTag: Checker = CheckInterface({ - [COMPUTE]: CheckFunction, -}); +export const CheckTag: Checker = CheckFunction as unknown as Checker; export const CheckOperations: Checker> = wrap(() => CheckNullable(CheckInstanceof(ComponentElementOperations)) diff --git a/packages/@glimmer/validator/index.ts b/packages/@glimmer/validator/index.ts index 69067eaeb93..503b0d6b152 100644 --- a/packages/@glimmer/validator/index.ts +++ b/packages/@glimmer/validator/index.ts @@ -33,7 +33,6 @@ export { untrack, } from './lib/tracking'; export { - ALLOW_CYCLES, bump, combine, COMPUTE, diff --git a/packages/@glimmer/validator/lib/meta.ts b/packages/@glimmer/validator/lib/meta.ts index 56e30cc7be1..856d39bcffa 100644 --- a/packages/@glimmer/validator/lib/meta.ts +++ b/packages/@glimmer/validator/lib/meta.ts @@ -1,5 +1,5 @@ import { DEBUG } from '@glimmer/env'; -import type { ConstantTag, UpdatableTag } from '@glimmer/interfaces'; +import type { UpdatableTag } from '@glimmer/interfaces'; import type { Indexable } from './utils'; @@ -59,7 +59,7 @@ export function tagFor( obj: T, key: keyof T | string | symbol, meta?: TagMeta -): UpdatableTag | ConstantTag { +): UpdatableTag { let tags = meta === undefined ? tagMetaFor(obj) : meta; let tag = tags.get(key); diff --git a/packages/@glimmer/validator/lib/validators.ts b/packages/@glimmer/validator/lib/validators.ts index c333e4cb5f0..eeee645974f 100644 --- a/packages/@glimmer/validator/lib/validators.ts +++ b/packages/@glimmer/validator/lib/validators.ts @@ -1,43 +1,31 @@ import { DEBUG } from '@glimmer/env'; import type { - COMBINATOR_TAG_ID as ICOMBINATOR_TAG_ID, - CONSTANT_TAG_ID as ICONSTANT_TAG_ID, ConstantTag, - CURRENT_TAG_ID as ICURRENT_TAG_ID, - DIRTYABLE_TAG_ID as IDIRTYABLE_TAG_ID, DirtyableTag, Tag, TagComputeSymbol, - TagTypeSymbol, - UPDATABLE_TAG_ID as IUPDATABLE_TAG_ID, UpdatableTag, - VOLATILE_TAG_ID as IVOLATILE_TAG_ID, } from '@glimmer/interfaces'; import { scheduleRevalidate } from '@glimmer/global-context'; -import { signal } from 'alien-signals'; +import { computed, signal } from 'alien-signals'; import { debug } from './debug'; import { unwrap } from './utils'; +// A Tag is just a function: call it to get the current revision (and, when +// called inside an alien-signals subscriber, register a dependency). +// Everything below is sugar over that. + export type Revision = number; export const CONSTANT: Revision = 0; export const INITIAL: Revision = 1; export const VOLATILE: Revision = NaN; -const TYPE: TagTypeSymbol = Symbol('TAG_TYPE') as TagTypeSymbol; +// Kept as a Symbol export so the type from @glimmer/interfaces still resolves. +// Internal code does not go through it — it calls the tag directly. export const COMPUTE: TagComputeSymbol = Symbol('TAG_COMPUTE') as TagComputeSymbol; Reflect.set(globalThis, 'COMPUTE_SYMBOL', COMPUTE); -const DIRTYABLE: IDIRTYABLE_TAG_ID = 0; -const UPDATABLE: IUPDATABLE_TAG_ID = 1; -const COMBINATOR: ICOMBINATOR_TAG_ID = 2; -const CONST: ICONSTANT_TAG_ID = 3; -const VOLATILE_ID: IVOLATILE_TAG_ID = 100; -const CURRENT_ID: ICURRENT_TAG_ID = 101; - -// The one piece of shared state. Every DIRTY_TAG advances the tick; readers -// inside an alien-signals subscriber pick up a dependency on "any mutation -// happened since" through CurrentTag/VolatileTag. const $tick = signal(INITIAL); const advance = (): Revision => { const n = $tick() + 1; @@ -48,151 +36,140 @@ export const bump = (): void => { advance(); }; -export let ALLOW_CYCLES: WeakMap | undefined; -if (DEBUG) ALLOW_CYCLES = new WeakMap(); - -// Cycle handling. alien-signals silently short-circuits re-entry; ember's CP -// system intentionally constructs cycles between property tags and expects -// either a throw (DEBUG) or a stable cached value with the global revision -// advanced (production), so that other caches invalidate on the next read. -const inFlight = new WeakSet(); -const safeCompute = (tag: ReadonlyTag, fold: () => Revision): Revision => { - if (inFlight.has(tag)) { - if (DEBUG && !(ALLOW_CYCLES === undefined || ALLOW_CYCLES.has(tag as unknown as Tag))) { - throw new Error('Cycles in tags are not allowed'); - } - advance(); - return tag.lastValue; - } - inFlight.add(tag); - try { - return fold(); - } finally { - inFlight.delete(tag); - } -}; - -type Sig = { (): T; (v: T): void }; -interface ReadonlyTag { - lastValue: Revision; -} +// Private map from a tag's read fn to its underlying writable signal. +// Only dirtyable tags are registered here. +const dirtyHandles = new WeakMap void>(); +// Private map for updatable tags whose subtag can be re-pointed via updateTag. +const subRefs = new WeakMap void>(); -export const valueForTag = (tag: Tag): Revision => tag[COMPUTE](); -export const validateTag = (tag: Tag, snapshot: Revision): boolean => snapshot >= tag[COMPUTE](); +const asTag = (fn: () => Revision): Tag => { + const tag = fn as unknown as Tag; + // Maintain the [COMPUTE] property shape for the type system. + (tag as unknown as Record Revision>)[COMPUTE] = fn; + return tag; +}; export function createTag(): DirtyableTag { - const own = signal(INITIAL); - return { - [TYPE]: DIRTYABLE, - [COMPUTE]: () => own(), - own, - lastValue: INITIAL, - } as unknown as DirtyableTag; + const s = signal(INITIAL); + const tag = asTag(() => s()); + dirtyHandles.set(tag, s); + return tag; } -interface UpdatableInternals extends Tag, ReadonlyTag { - own: Sig; - sub: Sig; +// An "updatable" tag is a dirtyable tag with one additional capability: a +// subtag pointer that can be re-pointed via updateTag. +// +// Two pieces of bookkeeping survive on top of the alien-signals signals, +// both of them required by ember's contract (verified by removing each and +// watching specific tests fail): +// +// 1. A cycle guard. ember's CP set/get both call updateTag(propertyTag, +// depsChain), which sets up `barTag.sub → fooTag` and +// `fooTag.sub → barTag` when foo/bar depend on each other. alien-signals +// has no cycle handling that maps to ember's ALLOW_CYCLES contract, so +// a re-entrance check returns the tag's last-seen revision and advances +// the global tick (forcing other caches to revalidate). +// 2. An adoption "buffer". When updateTag adopts a subtag whose revision is +// already higher than the parent's, naive `max(own, sub())` makes the +// parent's revision jump. That would fire observers on the parent +// without anything they care about having changed (see +// `computed - observer interop: observers that do not consume computed +// properties still work`). The buffer hides the adoption-time jump +// until the subtag is itself dirtied past its adoption value. +const computing = new WeakSet(); +interface UpdatableState { buffer: Revision | null; + last: Revision; } +const updatableState = new WeakMap(); export function createUpdatableTag(): UpdatableTag { - const own = signal(INITIAL); - const sub = signal(null); - const tag: UpdatableInternals = { - [TYPE]: UPDATABLE, - own, - sub, - buffer: null, - lastValue: INITIAL, - [COMPUTE]: () => safeCompute(tag, fold), - }; - // The buffer/lastValue pair preserves ember's "adopting a subtag with a - // higher revision doesn't immediately invalidate snapshots taken before - // the adoption" contract — see updateTag below and the explicit test in - // test/validators-test.ts. While `subVal === buffer` (i.e. the subtag is - // still at the revision captured at UPDATE_TAG time), the parent reports - // its pre-adoption value instead of jumping to the subtag's value. - const fold = (): Revision => { - const o = own(); - const s = sub(); - if (s === null) { - tag.lastValue = o; - return o; + const ownSig = signal(INITIAL); + const subSig = signal(null); + const state: UpdatableState = { buffer: null, last: INITIAL }; + const tag = asTag(() => { + if (computing.has(tag)) { + advance(); + return state.last; } - const sv = s[COMPUTE](); - const r = - sv === tag.buffer ? Math.max(o, tag.lastValue) : ((tag.buffer = null), Math.max(o, sv)); - tag.lastValue = r; - return r; - }; - return tag as unknown as UpdatableTag; + computing.add(tag); + try { + const o = ownSig(); + const sub = subSig(); + if (sub === null) { + state.last = o; + return o; + } + const sv = sub(); + const r = + sv === state.buffer ? Math.max(o, state.last) : ((state.buffer = null), Math.max(o, sv)); + state.last = r; + return r; + } finally { + computing.delete(tag); + } + }); + dirtyHandles.set(tag, ownSig); + subRefs.set(tag, subSig); + updatableState.set(tag, state); + return tag; } -interface CombinatorInternals extends Tag, ReadonlyTag {} +export function updateTag(tag: UpdatableTag, sub: Tag): void { + const ref = subRefs.get(tag); + const state = updatableState.get(tag); + if (ref === undefined || state === undefined) { + if (DEBUG) throw new Error('Attempted to update a tag that was not updatable'); + return; + } + if (sub === CONSTANT_TAG) { + state.buffer = null; + ref(null); + } else { + state.buffer = sub(); + ref(sub); + } +} +export const UPDATE_TAG = updateTag; export function combine(tags: Tag[]): Tag { if (tags.length === 0) return CONSTANT_TAG; if (tags.length === 1) return tags[0] as Tag; - const tag: CombinatorInternals = { - [TYPE]: COMBINATOR, - lastValue: INITIAL, - [COMPUTE]: () => - safeCompute(tag, () => { - let max: Revision = INITIAL; - // Math.max propagates NaN, which keeps combinators containing - // VOLATILE_TAG always-stale (validateTag tests `snapshot >= NaN`). - for (const t of tags) max = Math.max(max, t[COMPUTE]()); - tag.lastValue = max; - return max; - }), - }; - return tag; + return asTag( + computed(() => { + let max: Revision = INITIAL; + // Math.max propagates NaN, keeping any combinator that includes + // VOLATILE_TAG always-stale (validateTag is `snapshot >= NaN`). + for (const t of tags) max = Math.max(max, t()); + return max; + }) + ); } -export const CONSTANT_TAG: ConstantTag = { - [TYPE]: CONST, - [COMPUTE]: () => INITIAL, -}; +export const CONSTANT_TAG: ConstantTag = asTag(() => INITIAL); export const isConstTag = (tag: Tag): tag is ConstantTag => tag === CONSTANT_TAG; -export const VOLATILE_TAG: Tag = { - [TYPE]: VOLATILE_ID, - [COMPUTE]: () => { - $tick(); - return VOLATILE; - }, -}; - -export const CURRENT_TAG: Tag = { - [TYPE]: CURRENT_ID, - [COMPUTE]: () => $tick(), -}; +export const VOLATILE_TAG: Tag = asTag(() => { + $tick(); + return VOLATILE; +}); -export function updateTag(tag: UpdatableTag, sub: Tag): void { - if (DEBUG && tag[TYPE] !== UPDATABLE) { - throw new Error('Attempted to update a tag that was not updatable'); - } - const t = tag as unknown as UpdatableInternals; - if (sub === CONSTANT_TAG) { - t.buffer = null; - t.sub(null); - } else { - t.buffer = sub[COMPUTE](); - t.sub(sub); - } -} -export const UPDATE_TAG = updateTag; +export const CURRENT_TAG: Tag = asTag(() => $tick()); -export function dirtyTag(tag: DirtyableTag | UpdatableTag, skipAssertion?: boolean): void { - if (DEBUG && tag[TYPE] !== UPDATABLE && tag[TYPE] !== DIRTYABLE) { - throw new Error('Attempted to dirty a tag that was not dirtyable'); +export function dirtyTag(tag: DirtyableTag, skipAssertion?: boolean): void { + const s = dirtyHandles.get(tag); + if (s === undefined) { + if (DEBUG) throw new Error('Attempted to dirty a tag that was not dirtyable'); + return; } if (DEBUG && skipAssertion !== true) { unwrap(debug.assertTagNotConsumed)(tag); } - (tag as unknown as UpdatableInternals).own(advance()); + s(advance()); scheduleRevalidate(); } export const DIRTY_TAG = dirtyTag; + +export const valueForTag = (tag: Tag): Revision => tag(); +export const validateTag = (tag: Tag, snapshot: Revision): boolean => snapshot >= tag(); diff --git a/packages/@glimmer/validator/test/validators-test.ts b/packages/@glimmer/validator/test/validators-test.ts index 9c963a1c928..23bd7cb94ef 100644 --- a/packages/@glimmer/validator/test/validators-test.ts +++ b/packages/@glimmer/validator/test/validators-test.ts @@ -2,7 +2,6 @@ import { DEBUG } from '@glimmer/env'; import type { UpdatableTag } from '@glimmer/interfaces'; import { testOverrideGlobalContext } from '@glimmer/global-context'; import { - ALLOW_CYCLES, bump, combine, CONSTANT_TAG, @@ -66,7 +65,7 @@ module('@glimmer/validator: validators', () => { let subtag = createTag(); assert.throws( - () => updateTag(tag as unknown as UpdatableTag, subtag), + () => updateTag(tag, subtag), /Error: Attempted to update a tag that was not updatable/u ); }); @@ -128,55 +127,17 @@ module('@glimmer/validator: validators', () => { let tag = createUpdatableTag(); let subtag = createUpdatableTag(); - // First, we get a snapshot of the parent let snapshot = valueForTag(tag); - // Then we dirty the currently unrelated subtag dirtyTag(subtag); - - // Now, we update the parent tag with the subtag, and revalidate it updateTag(tag, subtag); assert.ok(validateTag(tag, snapshot), 'tag is still valid after being updated'); - // Finally, dirty the subtag one final time to bust the buffer cache dirtyTag(subtag); assert.notOk(validateTag(tag, snapshot), 'tag is invalid after subtag is dirtied again'); }); - - if (DEBUG) { - test('does not allow cycles on tags that have not been marked with ALLOW_CYCLES', (assert) => { - let tag = createUpdatableTag(); - let subtag = createUpdatableTag(); - - let snapshot = valueForTag(tag); - - updateTag(tag, subtag); - updateTag(subtag, tag); - - dirtyTag(tag); - - assert.throws(() => validateTag(tag, snapshot)); - }); - - test('does allow cycles on tags that have been marked with ALLOW_CYCLES', (assert) => { - let tag = createUpdatableTag(); - let subtag = createUpdatableTag(); - - let snapshot = valueForTag(tag); - - unwrap(ALLOW_CYCLES).set(tag, true); - unwrap(ALLOW_CYCLES).set(subtag, true); - - updateTag(tag, subtag); - updateTag(subtag, tag); - - dirtyTag(tag); - - assert.notOk(validateTag(tag, snapshot)); - }); - } }); module('CombinatorTag', () => { @@ -203,7 +164,6 @@ module('@glimmer/validator: validators', () => { let combined = combine([tag1, tag2]); assert.throws( - // @ts-expect-error this is an error condition () => dirtyTag(combined), /Error: Attempted to dirty a tag that was not dirtyable/u ); @@ -216,7 +176,6 @@ module('@glimmer/validator: validators', () => { let combined = combine([tag1, tag2]); assert.throws( - // @ts-expect-error this is an error condition () => updateTag(combined, tag1), /Error: Attempted to update a tag that was not updatable/u ); @@ -228,7 +187,6 @@ module('@glimmer/validator: validators', () => { if (DEBUG) { test('it cannot be dirtied', (assert) => { assert.throws( - // @ts-expect-error this is an error condition () => dirtyTag(CONSTANT_TAG), /Error: Attempted to dirty a tag that was not dirtyable/u ); @@ -238,7 +196,6 @@ module('@glimmer/validator: validators', () => { let subtag = createTag(); assert.throws( - // @ts-expect-error this is an error condition () => updateTag(CONSTANT_TAG, subtag), /Error: Attempted to update a tag that was not updatable/u ); @@ -266,7 +223,6 @@ module('@glimmer/validator: validators', () => { if (DEBUG) { test('it cannot be dirtied', (assert) => { assert.throws( - // @ts-expect-error this is an error condition () => dirtyTag(VOLATILE_TAG), /Error: Attempted to dirty a tag that was not dirtyable/u ); @@ -276,7 +232,6 @@ module('@glimmer/validator: validators', () => { let subtag = createTag(); assert.throws( - // @ts-expect-error this is an error condition () => updateTag(VOLATILE_TAG, subtag), /Error: Attempted to update a tag that was not updatable/u ); @@ -311,7 +266,6 @@ module('@glimmer/validator: validators', () => { if (DEBUG) { test('it cannot be dirtied', (assert) => { assert.throws( - // @ts-expect-error this is an error condition () => dirtyTag(CURRENT_TAG), /Error: Attempted to dirty a tag that was not dirtyable/u ); @@ -321,7 +275,6 @@ module('@glimmer/validator: validators', () => { let subtag = createTag(); assert.throws( - // @ts-expect-error this is an error condition () => updateTag(CURRENT_TAG, subtag), /Error: Attempted to update a tag that was not updatable/u ); From bec73641cfc9f5bb6f36120a288e97db41a99f4f Mon Sep 17 00:00:00 2001 From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com> Date: Thu, 21 May 2026 17:38:36 -0400 Subject: [PATCH 6/6] Drop the Tag concept down to `() => Revision` The `[COMPUTE]` / `[TYPE]` symbols and the whole tag-id hierarchy were the last remnants of the old class-based Tag design. Now: - `@glimmer/interfaces/lib/tags.d.ts` is 11 lines: `Tag = () => Revision`, with `UpdatableTag` / `DirtyableTag` / `ConstantTag` / `CombinatorTag` / `MonomorphicTag` as type aliases for back-compat. - `validators.ts` no longer wraps things in an `asTag` helper or attaches `[COMPUTE]` to anything. Tags are returned as the read function itself. - `combine` no longer uses an alien-signals `computed`; it's a plain closure that walks subtags. Memoization wasn't load-bearing for the combinator path and dropping it removes the re-entry/NaN footgun. - The numeric `CONSTANT` and `VOLATILE` exports were unreferenced outside the validator; dropped. The remaining state in `validators.ts`: - `$tick` signal + `advance()` for the global revision counter - `dirtyWriters` WeakMap keying tags to their underlying signal writer - `updatables` WeakMap + `computing` WeakSet for updatable bookkeeping (sub pointer, buffer, last value, cycle re-entry) 142 lines. 9306/9306 tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/@glimmer/interfaces/lib/tags.d.ts | 35 +---- packages/@glimmer/validator/index.ts | 3 - packages/@glimmer/validator/lib/validators.ts | 139 +++++++----------- 3 files changed, 56 insertions(+), 121 deletions(-) diff --git a/packages/@glimmer/interfaces/lib/tags.d.ts b/packages/@glimmer/interfaces/lib/tags.d.ts index 5f7c38d33ae..2257b1b6ee4 100644 --- a/packages/@glimmer/interfaces/lib/tags.d.ts +++ b/packages/@glimmer/interfaces/lib/tags.d.ts @@ -1,37 +1,8 @@ -declare const TYPE: unique symbol; -export type TagTypeSymbol = typeof TYPE; - -declare const COMPUTE: unique symbol; -export type TagComputeSymbol = typeof COMPUTE; - -export type DIRTYABLE_TAG_ID = 0; -export type UPDATABLE_TAG_ID = 1; -export type COMBINATOR_TAG_ID = 2; -export type CONSTANT_TAG_ID = 3; - -export type MonomorphicTagId = - | DIRTYABLE_TAG_ID - | UPDATABLE_TAG_ID - | COMBINATOR_TAG_ID - | CONSTANT_TAG_ID; - -export type VOLATILE_TAG_ID = 100; -export type CURRENT_TAG_ID = 101; - -export type PolymorphicTagId = VOLATILE_TAG_ID | CURRENT_TAG_ID; - -export type TagId = MonomorphicTagId | PolymorphicTagId; - export type Revision = number; -// A Tag is a callable: it returns the current revision and, when called -// inside an alien-signals subscriber, registers a dependency. The [COMPUTE] -// alias is kept only because a few places still write `tag[COMPUTE]()`. -export interface Tag { - (): Revision; - readonly [COMPUTE]?: () => Revision; - readonly subtag?: Tag | Tag[] | null | undefined; -} +// A Tag is just a callable that returns the current revision. Reading it +// inside an alien-signals subscriber registers a dependency. +export type Tag = () => Revision; export type MonomorphicTag = Tag; export type UpdatableTag = Tag; diff --git a/packages/@glimmer/validator/index.ts b/packages/@glimmer/validator/index.ts index 503b0d6b152..ee97b263bd7 100644 --- a/packages/@glimmer/validator/index.ts +++ b/packages/@glimmer/validator/index.ts @@ -35,8 +35,6 @@ export { export { bump, combine, - COMPUTE, - CONSTANT, CONSTANT_TAG, createTag, createUpdatableTag, @@ -48,7 +46,6 @@ export { UPDATE_TAG as updateTag, validateTag, valueForTag, - VOLATILE, VOLATILE_TAG, } from './lib/validators'; export type { diff --git a/packages/@glimmer/validator/lib/validators.ts b/packages/@glimmer/validator/lib/validators.ts index eeee645974f..6fcbb4849cb 100644 --- a/packages/@glimmer/validator/lib/validators.ts +++ b/packages/@glimmer/validator/lib/validators.ts @@ -1,30 +1,17 @@ import { DEBUG } from '@glimmer/env'; -import type { - ConstantTag, - DirtyableTag, - Tag, - TagComputeSymbol, - UpdatableTag, -} from '@glimmer/interfaces'; +import type { Tag } from '@glimmer/interfaces'; import { scheduleRevalidate } from '@glimmer/global-context'; -import { computed, signal } from 'alien-signals'; +import { signal } from 'alien-signals'; import { debug } from './debug'; import { unwrap } from './utils'; -// A Tag is just a function: call it to get the current revision (and, when -// called inside an alien-signals subscriber, register a dependency). -// Everything below is sugar over that. +// `Tag` is just `() => Revision`. Read it to get the current revision and, +// inside an alien-signals subscriber, register a dependency. export type Revision = number; -export const CONSTANT: Revision = 0; export const INITIAL: Revision = 1; -export const VOLATILE: Revision = NaN; - -// Kept as a Symbol export so the type from @glimmer/interfaces still resolves. -// Internal code does not go through it — it calls the tag directly. -export const COMPUTE: TagComputeSymbol = Symbol('TAG_COMPUTE') as TagComputeSymbol; -Reflect.set(globalThis, 'COMPUTE_SYMBOL', COMPUTE); +const VOLATILE: Revision = NaN; const $tick = signal(INITIAL); const advance = (): Revision => { @@ -36,58 +23,43 @@ export const bump = (): void => { advance(); }; -// Private map from a tag's read fn to its underlying writable signal. -// Only dirtyable tags are registered here. -const dirtyHandles = new WeakMap void>(); -// Private map for updatable tags whose subtag can be re-pointed via updateTag. -const subRefs = new WeakMap void>(); - -const asTag = (fn: () => Revision): Tag => { - const tag = fn as unknown as Tag; - // Maintain the [COMPUTE] property shape for the type system. - (tag as unknown as Record Revision>)[COMPUTE] = fn; - return tag; -}; +// Dirtyable tags: read-only Tag fn paired with a hidden `dirty` writer. +const dirtyWriters = new WeakMap void>(); -export function createTag(): DirtyableTag { +export function createTag(): Tag { const s = signal(INITIAL); - const tag = asTag(() => s()); - dirtyHandles.set(tag, s); + const tag: Tag = () => s(); + dirtyWriters.set(tag, s); return tag; } -// An "updatable" tag is a dirtyable tag with one additional capability: a -// subtag pointer that can be re-pointed via updateTag. +// Updatable tags: a dirtyable that can have its subtag re-pointed via +// updateTag. Two pieces of bookkeeping survive (each verified by removing +// it and watching specific tests fail): // -// Two pieces of bookkeeping survive on top of the alien-signals signals, -// both of them required by ember's contract (verified by removing each and -// watching specific tests fail): -// -// 1. A cycle guard. ember's CP set/get both call updateTag(propertyTag, -// depsChain), which sets up `barTag.sub → fooTag` and -// `fooTag.sub → barTag` when foo/bar depend on each other. alien-signals -// has no cycle handling that maps to ember's ALLOW_CYCLES contract, so -// a re-entrance check returns the tag's last-seen revision and advances -// the global tick (forcing other caches to revalidate). -// 2. An adoption "buffer". When updateTag adopts a subtag whose revision is -// already higher than the parent's, naive `max(own, sub())` makes the -// parent's revision jump. That would fire observers on the parent -// without anything they care about having changed (see -// `computed - observer interop: observers that do not consume computed -// properties still work`). The buffer hides the adoption-time jump -// until the subtag is itself dirtied past its adoption value. +// 1. Cycle re-entrance guard. ember's CP set/get both call +// `updateTag(propertyTag, depsChain)`, building reciprocal subtag +// pointers between two property tags whose CPs depend on each other. +// alien-signals re-entry returns NaN; ember needs a stable last value +// plus a tick bump so neighbouring caches revalidate. +// 2. Adoption "buffer". `updateTag` may adopt a subtag whose revision is +// already higher than the parent's; naive `max(own, sub())` makes the +// parent jump and fires observers on the parent without anything they +// care about having changed. const computing = new WeakSet(); interface UpdatableState { buffer: Revision | null; last: Revision; + own: (v: Revision) => void; + sub: (s: Tag | null) => void; } -const updatableState = new WeakMap(); +const updatables = new WeakMap(); -export function createUpdatableTag(): UpdatableTag { +export function createUpdatableTag(): Tag { const ownSig = signal(INITIAL); const subSig = signal(null); - const state: UpdatableState = { buffer: null, last: INITIAL }; - const tag = asTag(() => { + const state: UpdatableState = { buffer: null, last: INITIAL, own: ownSig, sub: subSig }; + const tag: Tag = () => { if (computing.has(tag)) { advance(); return state.last; @@ -95,12 +67,12 @@ export function createUpdatableTag(): UpdatableTag { computing.add(tag); try { const o = ownSig(); - const sub = subSig(); - if (sub === null) { + const s = subSig(); + if (s === null) { state.last = o; return o; } - const sv = sub(); + const sv = s(); const r = sv === state.buffer ? Math.max(o, state.last) : ((state.buffer = null), Math.max(o, sv)); state.last = r; @@ -108,26 +80,24 @@ export function createUpdatableTag(): UpdatableTag { } finally { computing.delete(tag); } - }); - dirtyHandles.set(tag, ownSig); - subRefs.set(tag, subSig); - updatableState.set(tag, state); + }; + dirtyWriters.set(tag, ownSig); + updatables.set(tag, state); return tag; } -export function updateTag(tag: UpdatableTag, sub: Tag): void { - const ref = subRefs.get(tag); - const state = updatableState.get(tag); - if (ref === undefined || state === undefined) { +export function updateTag(tag: Tag, sub: Tag): void { + const state = updatables.get(tag); + if (state === undefined) { if (DEBUG) throw new Error('Attempted to update a tag that was not updatable'); return; } if (sub === CONSTANT_TAG) { state.buffer = null; - ref(null); + state.sub(null); } else { state.buffer = sub(); - ref(sub); + state.sub(sub); } } export const UPDATE_TAG = updateTag; @@ -135,30 +105,27 @@ export const UPDATE_TAG = updateTag; export function combine(tags: Tag[]): Tag { if (tags.length === 0) return CONSTANT_TAG; if (tags.length === 1) return tags[0] as Tag; - return asTag( - computed(() => { - let max: Revision = INITIAL; - // Math.max propagates NaN, keeping any combinator that includes - // VOLATILE_TAG always-stale (validateTag is `snapshot >= NaN`). - for (const t of tags) max = Math.max(max, t()); - return max; - }) - ); + return () => { + let max: Revision = INITIAL; + // Math.max propagates NaN, keeping combinators that include VOLATILE_TAG + // always-stale (validateTag is `snapshot >= NaN`). + for (const t of tags) max = Math.max(max, t()); + return max; + }; } -export const CONSTANT_TAG: ConstantTag = asTag(() => INITIAL); - -export const isConstTag = (tag: Tag): tag is ConstantTag => tag === CONSTANT_TAG; +export const CONSTANT_TAG: Tag = () => INITIAL; +export const isConstTag = (tag: Tag): boolean => tag === CONSTANT_TAG; -export const VOLATILE_TAG: Tag = asTag(() => { +export const VOLATILE_TAG: Tag = () => { $tick(); return VOLATILE; -}); +}; -export const CURRENT_TAG: Tag = asTag(() => $tick()); +export const CURRENT_TAG: Tag = () => $tick(); -export function dirtyTag(tag: DirtyableTag, skipAssertion?: boolean): void { - const s = dirtyHandles.get(tag); +export function dirtyTag(tag: Tag, skipAssertion?: boolean): void { + const s = dirtyWriters.get(tag); if (s === undefined) { if (DEBUG) throw new Error('Attempted to dirty a tag that was not dirtyable'); return;