refactor(bundle-size): route attribute updates through a handler registry (alt to #523)#529
Merged
layershifter merged 9 commits intoApr 29, 2026
Conversation
Same goal as microsoft#523 — move the subsystem-specific create/setProps logic out of `Instance.ts` so it only enters bundles whose consumer calls the matching `getX()`. Different shape: instead of attaching an `applyAttribute` method to each subsystem API class, each `get*` file registers a closure on a `tabster.attrHandlers: Map<key, handler>` when its API is first instantiated. ## Why a registry - Instance.ts no longer references subsystem names for the 5 heavy cases (deloser, groupper, mover, modalizer, restorer). Adding or removing a subsystem doesn't touch Instance.ts. - Subsystem API classes (`Modalizer.ts` etc.) stay focused on their actual job — they don't gain a method whose only purpose is to mutate `TabsterOnElement` on behalf of the attribute pipeline. - Wiring lives in `src/get/*` where it conceptually belongs: that's where the API instance is wired into `TabsterCore` already. ## What stays inline in Instance.ts `root` (always present), `focusable` / `uncontrolled` / `sys` (literal property assignments, not API-gated), and the remaining tiny gated cases `observed` / `outline` (1-2 lines each, no create/setProps logic to relocate). Putting these through the registry would add indirection without bundle-size gain. ## Bundle-size Same as microsoft#523 in shape: - Each subsystem's create-or-setProps closure ships only with its own `get*` file, which is itself only imported when the consumer calls `getDeloser()` / `getGroupper()` / etc. - The registry adds a small fixed cost (one `Map` instance per TabsterCore) that's amortized across however many subsystems the consumer uses. ## Behavior changes vs master - For deloser / groupper / mover / modalizer / restorer the dev-only "API used before initialization" error now fires whenever the registry has no handler for the key (i.e. `getX()` was never called). In master the equivalent error was guarded on `tabster.X === undefined` AND `tabsterOnElement.X === undefined`. The new version fires even when `tabsterOnElement.X` exists but the API isn't registered — a state that requires the API to have been disposed mid-flight, which the codebase doesn't construct. - Restorer: when `tabsterOnElement.restorer` exists and `newTabsterProps.restorer` is undefined, master called `setProps(undefined as RestorerProps)`. The new code skips the call. No test exercises the previous behavior. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
📊 Bundle size report
🤖 This report was generated against 1f90765aecd3c5914d21278031d35cbf763f36fb |
Handlers no longer receive (and mutate) the full TabsterOnElement. They take the existing instance for their own key and return the new instance to assign — or undefined when nothing should change (setProps on existing, or no-op). This removes the TabsterOnElement type from handler interfaces and moves storage assignment into Instance.ts where it belongs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Each handler is now \`TabsterAttrHandler<K extends keyof TabsterAttributeProps>\`,
with \`existing\`, \`newProps\`, \`oldProps\`, and the return value all inferred
from the key — no per-handler casts inside \`get*\` files. Modalizer's
\`oldProps?.id\` no longer needs a manual \`as ModalizerProps\` cast; deloser /
groupper / mover / restorer drop their \`as Types.X\` casts on \`existing\`
and \`newProps\` entirely.
The Map's value type is invariant in V, so a plain \`Map<key, handler>\`
can't simultaneously give type-safe registration and a uniform value
type. Solved by introducing a small \`TabsterAttrHandlerRegistry\`
interface — \`set<K>\` is generic for type-safe registration; \`get\` returns
the type-erased \`AnyTabsterAttrHandler\` shape since Instance.ts iterates
\`keyof TabsterAttributeProps\` and can't statically narrow at the call
site. The cast from per-K to Any is hidden inside the registry impl.
The simplified restorer handler drops master's \`if (newProps)\` runtime
guard. The handler signature now declares \`newProps:
NonNullable<RestorerProps>\`, matching the typing of the other handlers.
The guard previously protected against an unlikely \`{"restorer": null}\`
JSON payload — every other subsystem already lacked the same guard, so
this is consistency, not a regression.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Drops the "return undefined to signal no-change" branch. Each handler now returns the (possibly-mutated) existing instance for the setProps case, so Instance.ts can assign unconditionally. The handler return type tightens to NonNullable<TabsterOnElement[K]> — there is no path where the handler is invoked and the slot ends up empty. The dispatch in Instance.ts collapses to a single statement. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The handler signature has the same `existing` parameter name across five subsystems, but at each call site the type is concrete (Deloser, Groupper, Mover, Modalizer, Restorer). Spelling the type into the variable name makes the body easier to read at a glance and matches the convention used elsewhere in the codebase (e.g. element + role suffix in handler closures). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The registered closures capture the subsystem API instances. Without explicitly clearing the registry, those references survive \`dispose()\` and would let any post-dispose \`updateTabsterByAttribute\` call dispatch to the just-disposed APIs. Adds a \`clear()\` method to \`TabsterAttrHandlerRegistry\` and calls it after the per-API \`dispose()\` block. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Drop the wrapper object + factory. The registry interface now extends Map<key, AnyTabsterAttrHandler> and only overrides \`set\` with a generic per-key signature; \`get\` and \`clear\` come from Map. A plain \`new Map()\` is cast to the typed view at the field declaration — single cast, no double-cast inside set, no closure capture. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The interface-extends-Map declaration overflows the prettier line limit; reformat it to multi-line generic args. Caught by format:check on CI. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
TS's write-side bivariance check on \`obj[unionKey] = value\` requires the intersection of all possible value types, which is \`never\` here. Casting the value to \`never\` is the standard TS escape hatch for this exact issue and reads as a single inline assertion at the value, versus reshaping the storage type around the indexer. Net: \`tabsterOnElement[key] = handler(...) as never;\` Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
mshoho
approved these changes
Apr 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Alternative to #523
Same goal — pull the per-subsystem create/setProps logic out of
Instance.ts's switch so agetGroupper-only bundle no longer ships the deadcase "deloser":/"modalizer":/ etc. branches. Different mechanism.#523 attaches an
applyAttribute(element, storage, newProps, …)method to each subsystem API class (DeloserAPI,GroupperAPI,ModalizerAPI,MoverAPI,RestorerAPI).This PR introduces
tabster.attrHandlers: Map<key, handler>. Eachget*file registers a closure on the map when its API is first instantiated.Instance.tslooks up the handler by key — it has no idea which subsystems exist.Tradeoffs
defaultbranch via Map lookup)TabsterOnElementapplyAttribute(…, storage, …)mutates it)get*does)ModalizerAPI.applyAttribute)unknown, casts inside)get*onlyMap.getper attribute keyWhy I prefer this shape
updateTabsterByAttributeis the attribute pipeline. The fact that "modalizer" or "groupper" exist as concepts is a layering above it. With a registry the pipeline becomes generic, and the layer that knows about subsystems (the wiring, inget*) is where the subsystem-specific code lives. That matches how the rest ofTabsterCoreworks —tabster.modalizeretc. are populated fromgetModalizer(), not declared statically.What still lives in
Instance.ts's switchThe cases that have no create-or-setProps logic to relocate:
root—RootAPIis always present, no API-gatingfocusable/uncontrolled/sys— literal property assignmentsobserved/outline— 1-2 lines each, no factory callPutting these through the registry would add indirection without saving bytes.
Bundle-size
Identical shape to #523's gain: each subsystem's create-or-setProps closure ships only with its own
get*file, which is only imported when the consumer callsgetDeloser()/getGroupper()/getMover()/getModalizer()/getRestorer(). The registry itself is oneMapperTabsterCore, amortized.Behavior changes vs master
Both this PR and #523 introduce the same two:
getX()was never called — instead of being conditional on bothtabster.XandtabsterOnElement.Xbeing undefined. The state where they diverge requires the API to have been disposed mid-flight, which the codebase doesn't construct.tabsterOnElement.restorerexists andnewTabsterProps.restoreris undefined, master calledsetProps(undefined as RestorerProps). The new code skips. No test exercises the previous behavior.Verification
npm run type-check(lib + tests + stories) passesnpm run lint:checkpassesnpm run format:checkpassesOut of scope
The dispose loop at
Instance.ts:74-118still hardcodes the list of "Part" subsystems with.dispose(). It doesn't reference subsystem APIs (justtabsterOnElement[key].dispose()), so there's no bundle-size benefit to making it generic. Keeping it inline.Open this PR alongside #523 — pick whichever shape you prefer. Closing one when the other lands is fine.
🤖 Generated with Claude Code