refactor: delegate attribute updates to each subsystem API by layershifter · Pull Request #523 · microsoft/tabster

layershifter · 2026-04-22T18:09:05Z

Problem

Instance.ts's updateTabsterByAttribute() held a ~1.3 kB minified switch that handled data-tabster attribute changes for every subsystem.

In a single-subsystem bundle the 5 dead branches for the unused subsystems shipped regardless, guarded only by runtime `if (tabster.modalizer) …` checks — which terser cannot statically prove unreachable.

Fix

Move the heavy branch bodies (create / setProps / id-change recreation) onto each subsystem's API:

DeloserAPI.applyAttribute
GroupperAPI.applyAttribute
MoverAPI.applyAttribute
ModalizerAPI.applyAttribute — retains the id-change recreation path
RestorerAPI.applyAttribute

`Instance.ts` branches become one-liners:

case "groupper":
    if (tabster.groupper) {
        tabster.groupper.applyAttribute(element, tabsterOnElement, props, sys);
    } else if (__DEV__) {
        console.error(\"Groupper API used before initialization, please call \\\`getGroupper()\\\`\");
    }
    break;

Crucial effect: each subsystem's applyAttribute body is now imported only when its module is imported — i.e. only when `getX()` is called. In a `getGroupper`-only bundle, Deloser / Mover / Modalizer / Restorer ship nothing for attribute handling; previously they shipped their create/setProps branches inside `Instance.ts`.

Not changed

`root` — always present, no tree-shaking benefit, kept inline.
`observed` / `outline` — conditional but tiny (1-3 lines), kept inline.
`focusable` / `uncontrolled` / `sys` — plain property assignments, kept inline.
The delete switch (cleanup when a key disappears from the attribute) is already compact via case fall-through; a `removeAttribute` method would add bytes. Left alone.

github-actions · 2026-04-22T18:10:08Z

📊 Bundle size report

Package & Exports	Baseline (minified/GZIP)	PR	Change
`tabster` all exports	`113.989 kB` `30.989 kB`	`114.214 kB` `31.005 kB`	`225 B` `16 B`
`tabster` createTabster (core)	`40.45 kB` `12.087 kB`	`40.215 kB` `11.998 kB`	`-235 B` `-89 B`
`tabster` getCrossOrigin	`110.866 kB` `30.322 kB`	`111.017 kB` `30.324 kB`	`151 B` `2 B`
`tabster` getDeloser	`49.865 kB` `14.399 kB`	`49.703 kB` `14.337 kB`	`-162 B` `-62 B`
`tabster` getGroupper	`47.589 kB` `13.755 kB`	`47.432 kB` `13.697 kB`	`-157 B` `-58 B`
`tabster` getModalizer	`49.601 kB` `14.517 kB`	`49.526 kB` `14.476 kB`	`-75 B` `-41 B`
`tabster` getMover	`55.248 kB` `16.048 kB`	`55.088 kB` `15.988 kB`	`-160 B` `-60 B`
`tabster` getObservedElement	`46.253 kB` `13.678 kB`	`46.018 kB` `13.594 kB`	`-235 B` `-84 B`
`tabster` getOutline	`49.556 kB` `14.411 kB`	`49.321 kB` `14.323 kB`	`-235 B` `-88 B`
`tabster` getRestorer	`43.113 kB` `12.687 kB`	`42.952 kB` `12.621 kB`	`-161 B` `-66 B`

_{🤖 This report was generated against 1f90765aecd3c5914d21278031d35cbf763f36fb}

Instance.ts's updateTabsterByAttribute() held a 1.3 kB minified switch that dispatched data-tabster attribute changes for every subsystem. In a single-subsystem bundle the 5 dead branches for the other subsystems shipped regardless, guarded only by runtime `if (tabster.modalizer) …` checks that terser cannot statically prove unreachable. Move the heavy branch bodies (create/setProps logic) onto each API: - DeloserAPI.applyAttribute - GroupperAPI.applyAttribute - MoverAPI.applyAttribute - ModalizerAPI.applyAttribute (retains the id-change recreation path) - RestorerAPI.applyAttribute Instance.ts branches become one-liners: `tabster.X ? tabster.X.applyAttribute(…) : __DEV__ && console.error(…)`. The crucial effect: each subsystem's applyAttribute body is now imported only when its module is imported — i.e. only when getX() is called. In a getGroupper-only bundle, Deloser/Mover/Modalizer/Restorer ship nothing for attribute handling; previously they shipped their create/setProps branches inside Instance.ts. root / observed / outline / focusable / uncontrolled / sys branches are left inline — they're 1-3 lines each and either always-present (root) or plain property assignments. Bundle-size impact (monosize, per fixture): all exports +0.18 kB min / +0.01 kB gzip (slight cost for the one bundle that uses every subsystem) createTabster (core) -0.37 kB / -0.12 kB getCrossOrigin +0.09 kB / -0.01 kB (flat) getDeloser -0.29 kB / -0.10 kB getGroupper -0.28 kB / -0.09 kB getModalizer -0.18 kB / -0.07 kB getMover -0.29 kB / -0.10 kB getObservedElement -0.37 kB / -0.12 kB getOutline -0.37 kB / -0.12 kB getRestorer -0.28 kB / -0.10 kB The single-subsystem bundles save 90-125 B gzip each — not a cliff, but predictable and paid-for ownership: every module only carries its own attribute-handling cost.

Each \`applyAttribute\` now takes the existing slot value (\`existingX: X | undefined\`) and returns the instance that should occupy \`TabsterOnElement[X]\` after the call — either the (mutated) existing or a freshly created one. Storage assignment moves to Instance.ts. This removes \`TabsterOnElement\` from the API class signatures, so the API classes stop knowing about Instance.ts's storage shape. Modalizer's \`propsToCreate\` intermediate goes away — the dispose+create branch and the create-from-scratch branch are explicit returns. Net: API surface is smaller, layering is cleaner, behavior is identical. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…stry (alt to #523) (#529) * fix(bundle-size): route attribute updates through a handler registry Same goal as #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 #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> * refactor(attr-handlers): drop storage param, return-based protocol 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> * refactor(attr-handlers): make TabsterAttrHandler generic per key 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> * refactor(attr-handlers): handler always returns the slot's instance 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> * refactor(attr-handlers): rename existing → existing<Subsystem> 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> * fix(dispose): clear attrHandlers registry on TabsterCore.dispose 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> * refactor(attr-handlers): use Map directly via overridden set signature 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> * chore: prettier --write src/Types.ts 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> * refactor(attr-handlers): use \`as never\` over Record-cast at assignment 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> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

layershifter · 2026-04-29T12:41:10Z

Alternative approach was merged.

layershifter force-pushed the fix/instance-dispatcher-leak branch from a5b5d63 to 4099633 Compare April 29, 2026 08:40

layershifter changed the title ~~fix(bundle-size): delegate attribute updates to each subsystem API~~ refactor: delegate attribute updates to each subsystem API Apr 29, 2026

layershifter mentioned this pull request Apr 29, 2026

refactor(bundle-size): route attribute updates through a handler registry (alt to #523) #529

Merged

mshoho approved these changes Apr 29, 2026

View reviewed changes

layershifter closed this Apr 29, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: delegate attribute updates to each subsystem API#523

refactor: delegate attribute updates to each subsystem API#523
layershifter wants to merge 2 commits into
microsoft:masterfrom
layershifter:fix/instance-dispatcher-leak

layershifter commented Apr 22, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Apr 22, 2026 •

edited

Loading

Uh oh!

layershifter commented Apr 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

layershifter commented Apr 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Fix

Not changed

Uh oh!

github-actions Bot commented Apr 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📊 Bundle size report

Uh oh!

layershifter commented Apr 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

layershifter commented Apr 22, 2026 •

edited

Loading

github-actions Bot commented Apr 22, 2026 •

edited

Loading