diff --git a/docs/superpowers/plans/2026-04-22-pr1-drop-weakref-shim.md b/docs/superpowers/plans/2026-04-22-pr1-drop-weakref-shim.md new file mode 100644 index 000000000..6e65aa8b6 --- /dev/null +++ b/docs/superpowers/plans/2026-04-22-pr1-drop-weakref-shim.md @@ -0,0 +1,343 @@ +# PR 1 — Drop `WeakRefInstance` IE11 Shim + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Replace the internal `WeakRefInstance` IE11 shim with native `WeakRef` throughout `src/`, deleting the shim file. Public API unchanged; Playwright tests must stay green. + +**Architecture:** Mechanical rename. `WeakRefInstance` → `WeakRef`, `new WeakRefInstance(x)` → `new WeakRef(x)`. Drop the companion `Disposable` interface and the unused (and inverted) `KeyborgCore.isDisposed()` method, which exist only to support the shim's fallback path. `.browserslistrc` already excludes IE 11, so this path is unreachable. + +**Tech Stack:** TypeScript, tsdown (build), Playwright (tests), monosize (bundle-size measurement). + +**Design reference:** `docs/superpowers/specs/2026-04-22-keyborg-bundle-size-design.md` (§PR 1). + +**TDD note:** This is a behavior-preserving refactor. No new test is added; the existing Playwright suite is the regression tripwire. The discipline is "run tests before and after; they must stay green." + +--- + +### Task 1: Create branch and capture baseline measurement + +**Files:** none + +- [ ] **Step 1: Create feature branch from `main`** + +```bash +git checkout main +git pull +git checkout -b perf/drop-weakref-shim +``` + +- [ ] **Step 2: Run baseline build + measurement** + +```bash +npm run bundle-size +``` + +Expected: completes successfully and prints a table like: + +``` + Fixture Minified size Gzipped size + All exports XX.XX kB X.XX kB + `createKeyborg()` & `disposeKeyborg()` X.XX kB X.XX kB + `KEYBORG_FOCUSIN` constant XX B XX B +``` + +- [ ] **Step 3: Record baseline numbers** + +Copy the printed table into a local scratch note (plain text file or the PR draft). You will paste a before/after diff of this table into the PR description at the end of Task 5. + +--- + +### Task 2: Migrate `src/FocusEvent.mts` to native `WeakRef` + +**Files:** + +- Modify: `src/FocusEvent.mts` (5 locations) + +- [ ] **Step 1: Remove the `WeakRefInstance` import** + +In `src/FocusEvent.mts`, delete line 5: + +```typescript +import { WeakRefInstance } from "./WeakRefInstance.mts"; +``` + +- [ ] **Step 2: Update the `KeyborgFocusEventData` interface types** + +Still in `src/FocusEvent.mts`, lines 17–22 currently read: + +```typescript +interface KeyborgFocusEventData { + focusInHandler: (e: FocusEvent) => void; + focusOutHandler: (e: FocusEvent) => void; + lastFocusedProgrammatically?: WeakRefInstance; + shadowTargets: Set>; +} +``` + +Replace with: + +```typescript +interface KeyborgFocusEventData { + focusInHandler: (e: FocusEvent) => void; + focusOutHandler: (e: FocusEvent) => void; + lastFocusedProgrammatically?: WeakRef; + shadowTargets: Set>; +} +``` + +- [ ] **Step 3: Update the `shadowTargets` local declaration** + +Still in `src/FocusEvent.mts`, around line 104, change: + +```typescript +const shadowTargets: Set> = new Set(); +``` + +to: + +```typescript +const shadowTargets: Set> = new Set(); +``` + +- [ ] **Step 4: Update the `shadowTargets.add(...)` call** + +Still in `src/FocusEvent.mts`, around line 230, change: + +```typescript +shadowTargets.add(new WeakRefInstance(shadowRoot)); +``` + +to: + +```typescript +shadowTargets.add(new WeakRef(shadowRoot)); +``` + +- [ ] **Step 5: Update the `lastFocusedProgrammatically` assignment inside the overridden `focus` function** + +Still in `src/FocusEvent.mts`, around lines 283–286, change: + +```typescript +if (keyborgNativeFocusEvent) { + keyborgNativeFocusEvent.lastFocusedProgrammatically = new WeakRefInstance( + this, + ); +} +``` + +to: + +```typescript +if (keyborgNativeFocusEvent) { + keyborgNativeFocusEvent.lastFocusedProgrammatically = new WeakRef(this); +} +``` + +- [ ] **Step 6: Verify no remaining references in this file** + +```bash +grep -n "WeakRefInstance" src/FocusEvent.mts +``` + +Expected: no output (exit code 1). If any match remains, repeat the relevant step. + +--- + +### Task 3: Migrate `src/Keyborg.mts` to drop the `Disposable` interface + +**Files:** + +- Modify: `src/Keyborg.mts` (3 locations) + +- [ ] **Step 1: Remove the `Disposable` import** + +In `src/Keyborg.mts`, delete line 12: + +```typescript +import { Disposable } from "./WeakRefInstance.mts"; +``` + +- [ ] **Step 2: Remove `implements Disposable` from `KeyborgCore`** + +Around line 40, change: + +```typescript +class KeyborgCore implements Disposable { +``` + +to: + +```typescript +class KeyborgCore { +``` + +- [ ] **Step 3: Delete the `isDisposed()` method** + +Around lines 120–122, delete the entire method: + +```typescript +isDisposed(): boolean { + return !!this._win; +} +``` + +Also delete the blank line that follows it if one remains, so the file stays tidy. + +- [ ] **Step 4: Verify no remaining references in this file** + +```bash +grep -nE "Disposable|isDisposed" src/Keyborg.mts +``` + +Expected: no output (exit code 1). If any match remains, repeat the relevant step. + +--- + +### Task 4: Delete the shim file + +**Files:** + +- Delete: `src/WeakRefInstance.mts` + +- [ ] **Step 1: Confirm no importers remain** + +```bash +grep -rn "WeakRefInstance\|from \"./WeakRefInstance" src/ tests/ +``` + +Expected: no output. If any match appears, return to Task 2 or Task 3 and clean it up before deleting. + +- [ ] **Step 2: Delete the file** + +```bash +git rm src/WeakRefInstance.mts +``` + +Expected: `rm 'src/WeakRefInstance.mts'`. + +--- + +### Task 5: Verify, measure, commit, push, open PR + +**Files:** none (verification and publication) + +- [ ] **Step 1: Lint** + +```bash +npm run lint +``` + +Expected: exits 0 with no errors. + +- [ ] **Step 2: Format check** + +```bash +npm run format +``` + +Expected: exits 0. If it reports unformatted files among the ones you touched, run `npm run format:fix` and re-run `npm run format`. + +- [ ] **Step 3: Build** + +```bash +npm run build +``` + +Expected: produces `dist/index.js`, `dist/index.cjs`, `dist/index.d.ts`, `dist/ts3.9/index.d.ts` with no errors. + +- [ ] **Step 4: Run Playwright tests** + +```bash +npm test +``` + +Expected: all tests pass. Playwright auto-starts Storybook on port 3000 via `playwright.config.mts`' `webServer` entry. If a prior Storybook instance is already listening on 3000, `reuseExistingServer: true` will reuse it. + +- [ ] **Step 5: Re-run bundle-size measurement** + +```bash +npm run bundle-size +``` + +Expected: the printed table should show each fixture's minified size at or below the baseline recorded in Task 1, Step 3. If `all-exports` or `create-n-dispose` grew, stop and investigate before committing. + +- [ ] **Step 6: Stage all changes** + +```bash +git add src/FocusEvent.mts src/Keyborg.mts +git status +``` + +Expected: `git status` shows `src/FocusEvent.mts` and `src/Keyborg.mts` as modified, and `src/WeakRefInstance.mts` as deleted. + +- [ ] **Step 7: Commit** + +```bash +git commit -m "$(cat <<'EOF' +perf: drop WeakRefInstance IE11 shim in favor of native WeakRef + +.browserslistrc excludes IE 11, so the shim's fallback path is +unreachable. Replace with native WeakRef, remove the companion +Disposable interface, and delete the unused KeyborgCore.isDisposed() +method (its body return !!this._win was inverted — returned true +before dispose — which confirms nothing on the supported path ever +called it). + +Co-Authored-By: Claude Opus 4.7 (1M context) +EOF +)" +``` + +Expected: commit succeeds. + +- [ ] **Step 8: Push the branch** + +```bash +git push -u origin perf/drop-weakref-shim +``` + +Expected: branch published and tracking set. Note the PR creation URL that `git push` prints. + +- [ ] **Step 9: Open the PR** + +```bash +gh pr create --title "perf: drop WeakRefInstance IE11 shim" --body "$(cat <<'EOF' +## Summary +- Replaces the internal `WeakRefInstance` shim with native `WeakRef`. `.browserslistrc` already excludes IE 11, so the shim's fallback branch is dead code on every supported target. +- Drops the companion `Disposable` interface and the unused `KeyborgCore.isDisposed()` method (its body was also inverted). +- Public API unchanged. PR 1 of the bundle-size reduction stack — see `docs/superpowers/specs/2026-04-22-keyborg-bundle-size-design.md`. + +## Bundle size + +| Fixture | Before (min / gz) | After (min / gz) | Delta | +| --- | --- | --- | --- | +| All exports | _paste_ | _paste_ | _paste_ | +| `createKeyborg()` & `disposeKeyborg()` | _paste_ | _paste_ | _paste_ | +| `KEYBORG_FOCUSIN` constant | _paste_ | _paste_ | _paste_ | + +## Test plan +- [ ] `npm run lint` clean +- [ ] `npm run format` clean +- [ ] `npm run build` succeeds +- [ ] `npm test` (Playwright) green +- [ ] `npm run bundle-size` shows no regression + +🤖 Generated with [Claude Code](https://claude.com/claude-code) +EOF +)" +``` + +Expected: `gh` prints the PR URL. + +- [ ] **Step 10: Fill in the bundle-size table** + +Edit the PR description on GitHub (or via `gh pr edit`) and replace the `_paste_` placeholders with the before/after numbers from Task 1, Step 3 and Task 5, Step 5. Compute the `Delta` column as `(after − before)` in bytes or as a percentage. + +--- + +## Out of scope for this PR + +- Refactoring classes to closures (PR 2, separate plan). +- Measuring ES2022 target (PR 3 spike). +- Measuring plain `tsc` as emitter (PR 4 spike). +- Any public API change. diff --git a/docs/superpowers/plans/2026-04-22-pr2-closure-refactor.md b/docs/superpowers/plans/2026-04-22-pr2-closure-refactor.md new file mode 100644 index 000000000..4c8782a85 --- /dev/null +++ b/docs/superpowers/plans/2026-04-22-pr2-closure-refactor.md @@ -0,0 +1,582 @@ +# PR 2 — Refactor Keyborg classes to closure-based factories + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Rewrite `src/Keyborg.mts` to replace `class KeyborgCore` and `class Keyborg` with factory functions whose per-instance state lives in closure variables. Public API unchanged; bundle shrinks because minifiers can mangle closure locals but not class property names at `target: "es2019"`. + +**Architecture:** `createKeyborgCore(win, props)` returns an opaque `KeyborgCoreHandle` exposing `dispose`, `getNavigating`, `setNavigating`. `createKeyborg(win, props)` returns an object literal implementing the public `Keyborg` interface, plus internal `_notify(state)` and `_dispose()` hooks used by the core's broadcast loop and by `disposeKeyborg(instance)`. `WindowWithKeyborg.__keyborg` keeps the same outer shape `{ core, refs }` — only the runtime type of `core` changes (class instance → plain object), and `refs[id]` now holds a plain object instead of a class instance. + +**Tech Stack:** TypeScript (target ES2019), tsdown (build), Playwright (tests), monosize (bundle-size measurement). + +**Design reference:** `docs/superpowers/specs/2026-04-22-keyborg-bundle-size-design.md` (§PR 2). + +**Base branch:** `perf/drop-weakref-shim` (stacked on PR 1). + +**TDD note:** Behavior-preserving rewrite. No new test added; the existing 9 Playwright specs are the regression tripwire. Discipline: tests must stay green before and after. + +--- + +### Task 1: Create branch and capture baseline + +**Files:** none + +- [ ] **Step 1: Branch off `perf/drop-weakref-shim`** + +```bash +git checkout perf/drop-weakref-shim +git checkout -b perf/closure-refactor +``` + +Expected: `Switched to a new branch 'perf/closure-refactor'`. + +- [ ] **Step 2: Run baseline bundle-size measurement** + +Ensure you are on Node 20 (match CI). If you use nvm locally, `nvm use 20`. + +```bash +npm run bundle-size +``` + +Expected: completes successfully and prints a table. Record the three fixture numbers — these are the post-PR-1 baseline that PR 2 will be compared against. + +Expected values (post-PR-1 baseline): + +| Fixture | Minified | Gzipped | +| -------------------------------------- | -------- | ------- | +| All exports | 6.606 kB | 1.957 kB | +| `createKeyborg()` & `disposeKeyborg()` | 6.370 kB | 1.904 kB | +| `KEYBORG_FOCUSIN` constant | 64 B | 80 B | + +If your numbers differ by more than 10 bytes on any fixture, stop and investigate before proceeding — either Node version drift or an unexpected tree state. + +--- + +### Task 2: Rewrite `src/Keyborg.mts` as closure-based factories + +**Files:** + +- Replace contents: `src/Keyborg.mts` + +- [ ] **Step 1: Replace the entire contents of `src/Keyborg.mts`** + +Overwrite `src/Keyborg.mts` with exactly the following content: + +```typescript +/*! + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. + */ + +import { + disposeFocusEvent, + KeyborgFocusInEvent, + KEYBORG_FOCUSIN, + setupFocusEvent, +} from "./FocusEvent.mts"; + +interface WindowWithKeyborg extends Window { + __keyborg?: { + core: KeyborgCoreHandle; + refs: { [id: string]: Keyborg }; + }; +} + +// When a key from dismissKeys is pressed and the focus is not moved during +// _dismissTimeout ms, keyboard navigation mode is dismissed. +const _dismissTimeout = 500; + +let _lastId = 0; + +export interface KeyborgProps { + // Keys to be used to trigger keyboard navigation mode. By default, any key + // will trigger it. Could be limited to, for example, just Tab (or Tab and + // arrow keys). + triggerKeys?: number[]; + // Keys to be used to dismiss keyboard navigation mode using keyboard (in + // addition to mouse clicks which dismiss it). For example, Esc could be used + // to dismiss. + dismissKeys?: number[]; +} + +export type KeyborgCallback = (isNavigatingWithKeyboard: boolean) => void; + +/** + * Internal handle for the per-window core state. Not part of the public API. + */ +interface KeyborgCoreHandle { + dispose(): void; + getNavigating(): boolean; + setNavigating(val: boolean): void; +} + +/** + * Used to determine the keyboard navigation state. + */ +export interface Keyborg { + /** + * @returns Whether the user is navigating with keyboard + */ + isNavigatingWithKeyboard(): boolean; + /** + * @param callback - Called when the keyboard navigation state changes + */ + subscribe(callback: KeyborgCallback): void; + /** + * @param callback - Registered with subscribe + */ + unsubscribe(callback: KeyborgCallback): void; + /** + * Manually set the keyboard navigation state + */ + setVal(isNavigatingWithKeyboard: boolean): void; +} + +// Augments the public Keyborg with internal methods invoked by the core's +// broadcast loop and by disposeKeyborg. Not exported. +interface KeyborgInternal extends Keyborg { + _notify(isNavigatingWithKeyboard: boolean): void; + _dispose(): void; +} + +function createKeyborgCore( + win: WindowWithKeyborg, + props?: KeyborgProps, +): KeyborgCoreHandle { + let currentWin: WindowWithKeyborg | undefined = win; + let isNavigating = false; + let isMouseOrTouchUsedTimer: number | undefined; + let dismissTimer: number | undefined; + let triggerKeys: Set | undefined; + let dismissKeys: Set | undefined; + + if (props) { + if (props.triggerKeys?.length) { + triggerKeys = new Set(props.triggerKeys); + } + if (props.dismissKeys?.length) { + dismissKeys = new Set(props.dismissKeys); + } + } + + const broadcast = (): void => { + const refs = currentWin?.__keyborg?.refs; + if (refs) { + for (const id of Object.keys(refs)) { + (refs[id] as KeyborgInternal)._notify(isNavigating); + } + } + }; + + const setNavigating = (val: boolean): void => { + if (isNavigating !== val) { + isNavigating = val; + broadcast(); + } + }; + + const shouldTrigger = (e: KeyboardEvent): boolean => { + // TODO Some rich text fields can allow Tab key for indentation so it + // doesn't need to be a navigation key. If there is a bug regarding that + // we should revisit. + if (e.key === "Tab") { + return true; + } + const active = currentWin?.document.activeElement as HTMLElement | null; + const isTriggerKey = !triggerKeys || triggerKeys.has(e.keyCode); + const isEditable = + active && + (active.tagName === "INPUT" || + active.tagName === "TEXTAREA" || + active.isContentEditable); + return isTriggerKey && !isEditable; + }; + + const shouldDismiss = (e: KeyboardEvent): boolean => { + return !!dismissKeys?.has(e.keyCode); + }; + + const scheduleDismiss = (): void => { + const w = currentWin; + if (!w) { + return; + } + if (dismissTimer) { + w.clearTimeout(dismissTimer); + dismissTimer = undefined; + } + const was = w.document.activeElement; + dismissTimer = w.setTimeout(() => { + dismissTimer = undefined; + const cur = w.document.activeElement; + if (was && cur && was === cur) { + // Esc was pressed, currently focused element hasn't changed. + // Just dismiss the keyboard navigation mode. + setNavigating(false); + } + }, _dismissTimeout); + }; + + const onFocusIn = (e: KeyborgFocusInEvent): void => { + // When the focus is moved not programmatically and without keydown events, + // it is likely that the focus is moved by screen reader (as it might + // swallow the events when the screen reader shortcuts are used). The + // screen reader usage is keyboard navigation. + if (isMouseOrTouchUsedTimer) { + return; + } + if (isNavigating) { + return; + } + const details = e.detail; + if (!details.relatedTarget) { + return; + } + if ( + details.isFocusedProgrammatically || + details.isFocusedProgrammatically === undefined + ) { + return; + } + setNavigating(true); + }; + + const onMouseOrTouch = (): void => { + const w = currentWin; + if (w) { + if (isMouseOrTouchUsedTimer) { + w.clearTimeout(isMouseOrTouchUsedTimer); + } + isMouseOrTouchUsedTimer = w.setTimeout(() => { + isMouseOrTouchUsedTimer = undefined; + }, 1000); + } + setNavigating(false); + }; + + const onMouseDown = (e: MouseEvent): void => { + if ( + e.buttons === 0 || + (e.clientX === 0 && e.clientY === 0 && e.screenX === 0 && e.screenY === 0) + ) { + // This is most likely an event triggered by the screen reader to + // perform an action on an element, do not dismiss the keyboard + // navigation mode. + return; + } + onMouseOrTouch(); + }; + + const onKeyDown = (e: KeyboardEvent): void => { + if (isNavigating) { + if (shouldDismiss(e)) { + scheduleDismiss(); + } + } else if (shouldTrigger(e)) { + setNavigating(true); + } + }; + + const doc = win.document; + doc.addEventListener(KEYBORG_FOCUSIN, onFocusIn, true); + doc.addEventListener("mousedown", onMouseDown, true); + win.addEventListener("keydown", onKeyDown, true); + doc.addEventListener("touchstart", onMouseOrTouch, true); + doc.addEventListener("touchend", onMouseOrTouch, true); + doc.addEventListener("touchcancel", onMouseOrTouch, true); + + setupFocusEvent(win); + + const dispose = (): void => { + const w = currentWin; + if (!w) { + return; + } + if (isMouseOrTouchUsedTimer) { + w.clearTimeout(isMouseOrTouchUsedTimer); + isMouseOrTouchUsedTimer = undefined; + } + if (dismissTimer) { + w.clearTimeout(dismissTimer); + dismissTimer = undefined; + } + disposeFocusEvent(w); + const d = w.document; + d.removeEventListener(KEYBORG_FOCUSIN, onFocusIn, true); + d.removeEventListener("mousedown", onMouseDown, true); + w.removeEventListener("keydown", onKeyDown, true); + d.removeEventListener("touchstart", onMouseOrTouch, true); + d.removeEventListener("touchend", onMouseOrTouch, true); + d.removeEventListener("touchcancel", onMouseOrTouch, true); + currentWin = undefined; + }; + + return { + dispose, + getNavigating: () => isNavigating, + setNavigating, + }; +} + +export function createKeyborg(win: Window, props?: KeyborgProps): Keyborg { + const kwin = win as WindowWithKeyborg; + const id = "k" + ++_lastId; + let localWin: WindowWithKeyborg | undefined = kwin; + let core: KeyborgCoreHandle | undefined; + let callbacks: KeyborgCallback[] = []; + + const existing = kwin.__keyborg; + if (existing) { + core = existing.core; + } else { + core = createKeyborgCore(kwin, props); + } + + const instance: KeyborgInternal = { + isNavigatingWithKeyboard() { + return !!core?.getNavigating(); + }, + subscribe(callback) { + callbacks.push(callback); + }, + unsubscribe(callback) { + const index = callbacks.indexOf(callback); + if (index >= 0) { + callbacks.splice(index, 1); + } + }, + setVal(val) { + core?.setNavigating(val); + }, + _notify(val) { + callbacks.forEach((cb) => cb(val)); + }, + _dispose() { + const wkb = localWin?.__keyborg; + if (wkb?.refs[id]) { + delete wkb.refs[id]; + if (Object.keys(wkb.refs).length === 0) { + wkb.core.dispose(); + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + delete localWin!.__keyborg; + } + } else if (process.env.NODE_ENV !== "production") { + console.error( + `Keyborg instance ${id} is being disposed incorrectly.`, + ); + } + callbacks = []; + core = undefined; + localWin = undefined; + }, + }; + + if (existing) { + existing.refs[id] = instance; + } else { + kwin.__keyborg = { + core, + refs: { [id]: instance }, + }; + } + + return instance; +} + +export function disposeKeyborg(instance: Keyborg): void { + (instance as KeyborgInternal)._dispose(); +} +``` + +- [ ] **Step 2: Verify the file has no class declarations or `this.` accesses** + +```bash +grep -nE "^class |^export class | this\\._| this\\.id " src/Keyborg.mts || echo "clean" +``` + +Expected: `clean`. If any match appears, re-copy the block above — the file was not fully overwritten. + +- [ ] **Step 3: Verify the public exports are preserved** + +```bash +grep -nE "^export " src/Keyborg.mts +``` + +Expected output (order may differ): + +``` +src/Keyborg.mts:NN:export interface KeyborgProps { +src/Keyborg.mts:NN:export type KeyborgCallback = (isNavigatingWithKeyboard: boolean) => void; +src/Keyborg.mts:NN:export interface Keyborg { +src/Keyborg.mts:NN:export function createKeyborg(win: Window, props?: KeyborgProps): Keyborg { +src/Keyborg.mts:NN:export function disposeKeyborg(instance: Keyborg): void { +``` + +These five exports must all be present. `index.mts` re-exports `Keyborg`, `KeyborgCallback`, `createKeyborg`, `disposeKeyborg` — all must resolve. + +--- + +### Task 3: Lint, format, build + +**Files:** none + +- [ ] **Step 1: Lint** + +```bash +npx eslint src/ tests/ +``` + +Expected: `ESLint: No issues found`. + +If lint complains about unused imports or types, it means the rewrite did not consume every declaration — double-check the file content from Task 2 matches exactly. + +- [ ] **Step 2: Format** + +```bash +npm run format +``` + +Expected: exits 0, no warnings. If prettier flags `src/Keyborg.mts`, run `npx prettier --write src/Keyborg.mts` and re-run format. + +- [ ] **Step 3: Build** + +```bash +npm run build +``` + +Expected: produces `dist/index.js`, `dist/index.cjs`, `dist/index.d.ts`, `dist/index.d.cts`, `dist/ts3.9/index.d.ts` with no errors. Typical output includes a size report per output. Confirm ESM `dist/index.js` is smaller than the PR 1 baseline (~15.10 kB). Expect further shrinkage; a grow is a red flag. + +If tsc reports a type error about `addEventListener` variance with `KeyborgFocusInEvent`, the project's `tsconfig.json` does not enable `strictFunctionTypes`, so the bivariant handler signature used above should compile. If it does not (for example, due to a dependency update tightening types), narrow `onFocusIn`'s parameter to `Event` and cast inside: + +```typescript +const onFocusIn = (ev: Event): void => { + const e = ev as KeyborgFocusInEvent; + // ... rest of the body, using `e` instead of the parameter +}; +``` + +Only apply this fallback if the build actually errors. + +--- + +### Task 4: Run Playwright tests + +**Files:** none + +- [ ] **Step 1: Run the full Playwright suite** + +```bash +npm test +``` + +Expected: 9 tests pass (Playwright auto-starts Storybook via `playwright.config.mts`' `webServer` entry). Anything failing is a regression — stop and investigate before measuring or committing. + +Most likely failure mode if introduced: the broadcast loop does not reach subscribers, meaning `subscribe` / `unsubscribe` bookkeeping is wrong. Re-read `_notify` and the `broadcast` → `_notify(isNavigating)` call site carefully. A second likely mode: the core event listeners were not removed on dispose — check that every `addEventListener` call in `createKeyborgCore` has a matching `removeEventListener` in `dispose`, using the exact same handler reference. + +--- + +### Task 5: Re-measure, commit, push, open PR + +**Files:** none (verification and publication) + +- [ ] **Step 1: Re-run bundle-size measurement** + +```bash +npm run bundle-size +``` + +Expected: each fixture's minified size is below the Task 1 baseline. If `all-exports` or `create-n-dispose` grew relative to the post-PR-1 baseline (6.606 kB / 6.370 kB minified), stop and investigate before committing. The refactor's primary value is bundle-size reduction. + +Copy the printed table into a local scratch note for the PR description. + +- [ ] **Step 2: Stage changes** + +```bash +git add src/Keyborg.mts +git status +``` + +Expected: `git status` shows exactly `src/Keyborg.mts` as modified. If other files appear, unstage them with `git restore --staged ` and investigate why they were touched. + +- [ ] **Step 3: Commit** + +```bash +git commit -m "$(cat <<'EOF' +perf: refactor Keyborg classes to closure-based factories + +Replaces class KeyborgCore and class Keyborg with factory functions. +Per-instance state moves from this._x fields (which minifiers cannot +mangle at target: "es2019") to closure locals (which they can). Public +API unchanged: createKeyborg, disposeKeyborg, and the Keyborg type +(now an interface) keep the same shape. WindowWithKeyborg.__keyborg +retains its outer { core, refs } shape. + +Co-Authored-By: Claude Opus 4.7 (1M context) +EOF +)" +``` + +Expected: commit succeeds. If a pre-commit hook runs format or lint and fails, fix the issue, re-stage, and create a new commit (do not amend). + +- [ ] **Step 4: Push the branch** + +```bash +git push -u origin perf/closure-refactor +``` + +Expected: branch published and tracking set. + +- [ ] **Step 5: Open the PR against `perf/drop-weakref-shim`** + +Because PR 2 is stacked on PR 1, it targets `perf/drop-weakref-shim` (not `main`). This keeps the monosize CI comparison meaningful against the PR 1 baseline. After PR 1 merges, retarget this PR's base to `main`. + +```bash +gh pr create --base perf/drop-weakref-shim --title "perf: refactor Keyborg classes to closure-based factories" --body "$(cat <<'EOF' +## Summary + +- Rewrites `src/Keyborg.mts` to use factory functions instead of classes. Per-instance state lives in closure variables, which the minifier can mangle; class property names like `this._triggerKeys` cannot be mangled at `target: "es2019"` because class fields are transpiled into constructor assignments. +- Public API unchanged. `Keyborg` becomes an exported **interface** (it was already only re-exported as a type from `index.mts`). `createKeyborg`, `disposeKeyborg`, and the `KeyborgCallback` type are unchanged. +- `WindowWithKeyborg.__keyborg` retains its outer `{ core, refs }` shape for any external consumer (e.g., Tabster) that reads it. The internal `core` handle shape changes: it is now `{ dispose, getNavigating, setNavigating }` instead of a `KeyborgCore` class instance with a getter/setter for `isNavigatingWithKeyboard`. +- PR 2 of the bundle-size reduction stack. Base branch is `perf/drop-weakref-shim` (PR 1) — retarget to `main` after PR 1 merges. + +## Bundle size + +Baseline = PR 1 tip. + +| Fixture | Before (min / gz) | After (min / gz) | Delta | +| -------------------------------------- | ----------------- | ---------------- | ----- | +| All exports | _paste_ | _paste_ | _paste_ | +| `createKeyborg()` & `disposeKeyborg()` | _paste_ | _paste_ | _paste_ | +| `KEYBORG_FOCUSIN` constant | _paste_ | _paste_ | _paste_ | + +## Notes for reviewers + +- The internal `core.isNavigatingWithKeyboard` getter/setter is gone. If any consumer reads `window.__keyborg.core.isNavigatingWithKeyboard` directly, that is an undocumented leak and would break here — please flag if aware of any such consumer. +- The `_id` values assigned to instances no longer interleave with core counters. This only shows up in the dev-mode `"Keyborg instance is being disposed incorrectly"` console.error string. + +## Test plan + +- [ ] `npx eslint src/ tests/` clean +- [ ] `npm run format` clean +- [ ] `npm run build` succeeds and `dist/index.js` shrinks vs PR 1 baseline +- [ ] `npm test` — 9/9 Playwright specs pass locally +- [ ] `npm run bundle-size` shows no fixture regression + +🤖 Generated with [Claude Code](https://claude.com/claude-code) +EOF +)" +``` + +Expected: `gh` prints the PR URL. + +- [ ] **Step 6: Fill in the bundle-size table** + +Edit the PR description (via GitHub UI or `gh pr edit`) and replace the `_paste_` placeholders with the baseline numbers from Task 1 Step 2 and the new numbers from Task 5 Step 1. Compute `Delta` as either bytes or percentage. + +--- + +## Out of scope for this PR + +- Raising the build target to ES2022 (PR 3 spike). +- Swapping tsdown for plain tsc (PR 4 spike). +- Any changes to `src/FocusEvent.mts` or `src/index.mts`. +- Any public API addition, removal, or rename. diff --git a/docs/superpowers/specs/2026-04-22-keyborg-bundle-size-design.md b/docs/superpowers/specs/2026-04-22-keyborg-bundle-size-design.md new file mode 100644 index 000000000..3eaf599de --- /dev/null +++ b/docs/superpowers/specs/2026-04-22-keyborg-bundle-size-design.md @@ -0,0 +1,120 @@ +# Keyborg Bundle-Size Reduction + +Date: 2026-04-22 +Status: Approved + +## Goals & Non-Goals + +**Goal.** Reduce `dist/index.js` minified and gzipped size, measured by `npm run bundle-size` (monosize + webpack fixtures in `bundle-size/`), without changing the public API. + +**Public API (must not change).** `createKeyborg`, `disposeKeyborg`, `Keyborg` (type), `KeyborgCallback`, `getLastFocusedProgrammatically`, `nativeFocus`, `KEYBORG_FOCUSIN`, `KEYBORG_FOCUSOUT`, `version`. + +**Non-goals.** Behavior changes, API redesign, dropping Shadow DOM support, raising the browserslist floor beyond the current `defaults, not IE 11`, changing the TS 3.9 `downlevel-dts` output path. + +**Constraints.** + +- Playwright tests in `tests/` stay green. +- All three bundle-size fixtures (`all-exports`, `create-n-dispose`, `focus-in-const`) keep compiling and must shrink or stay within ±0.5% noise. +- Build target stays **ES2019** for PR 1 and PR 2. PR 3 and PR 4 are measurement spikes; raise target or swap emitter only if the measurement justifies it. + +## Approach + +Four stacked PRs, low-risk first. Each PR is independently revertable and ships only if its measurement justifies it (for PRs 3 and 4). + +### PR 1 — Drop `WeakRefInstance` IE11 shim + +Mechanical replacement of the shim with native `WeakRef`. `.browserslistrc` already excludes IE 11, so the shim serves no supported target. + +Changes: + +- Delete `src/WeakRefInstance.mts`. +- In `src/FocusEvent.mts`, replace both `new WeakRefInstance(...)` call sites with `new WeakRef(...)` and update the two type references in `KeyborgFocusEventData` (`WeakRefInstance` → `WeakRef`, `Set>` → `Set>`). +- In `src/Keyborg.mts`, remove `import { Disposable }`, remove `implements Disposable`, and delete the unused `isDisposed()` method on `KeyborgCore` (the method is only called via the shim's fallback path, and its body `return !!this._win` is inverted — it returns `true` before dispose — which confirms it is unreachable in supported browsers). + +Expected win: small (~200–400 bytes minified), essentially free. + +### PR 2 — Refactor classes to closure-based factories + +Classes in `src/Keyborg.mts` do not minify well at `target: "es2019"`: class fields get transpiled to constructor assignments (`this._triggerKeys = …`) whose property names the minifier cannot mangle. Converting the two classes to factory functions moves per-instance state into closure variables, which minify to single-letter names. + +Changes: + +- `class KeyborgCore` → `createKeyborgCore(win, props)` returning an internal handle `{ dispose(): void; setNavigating(v: boolean): void; getNavigating(): boolean }`. The getter/setter pair for `isNavigatingWithKeyboard` collapses into explicit `get`/`set` functions on the handle. +- `class Keyborg` → factory `createKeyborg(win, props)` returning an object with methods `isNavigatingWithKeyboard`, `subscribe`, `unsubscribe`, `setVal`. The static `Keyborg.create` / `Keyborg.dispose` / `Keyborg.update` pattern collapses into module-scope helpers. +- `Keyborg` remains as an exported **type** (matching `index.mts`' current `export type { Keyborg }`). No runtime class was ever exported, so this is invisible to consumers. +- `WindowWithKeyborg.__keyborg`'s outer shape (`{ core, refs }`) is preserved. The `core` field's internal method surface changes, but `core` is not part of the documented API. +- Public method names (`isNavigatingWithKeyboard`, `subscribe`, `unsubscribe`, `setVal`, `dispose`) stay byte-identical because they are on the external interface. + +Expected win: the largest of the four PRs. + +### PR 3 — Measurement spike: raise `target` to ES2022 + +Temporary branch flips `target: "es2019"` → `"es2022"` in `tsdown.config.mts`. At ES2022, `#private` fields and class fields are native, optional chaining and nullish coalescing are not transpiled, and several other features that today emit helpers become direct syntax. + +Decision rule: ship the target bump if total minified shrinks ≥2% **and** gzipped does not regress on any fixture. Otherwise close the PR with the measured numbers recorded. + +Risk to flag in the PR description: raising the target does not affect `downlevel-dts`, which only rewrites emitted `.d.ts` files. + +### PR 4 — Measurement spike: plain `tsc` as emitter + +Independent from PR 3. Swap `tsdown` for plain `tsc` as the build emitter, keep `downlevel-dts` unchanged. + +Changes required if the spike proceeds: + +- Replace `env: { PKG_VERSION: pkg.version }` inlining (tsdown-only) with a generated `src/version.ts` written from `package.json` at build time, re-exported from `src/index.mts`. +- Update `npm run build` to `tsc && npx downlevel-dts ./dist ./dist/ts3.9` (or equivalent two-pass for `cjs` + `esm`). + +Decision rule: same as PR 3 — ship if total minified shrinks ≥2% and gzipped does not regress; otherwise close with a note. + +Risk to flag: tsc emits per-file, not bundled — the published shape becomes multi-file instead of one-file `dist/index.js`. Downstream bundlers tree-shake ESM fine, but any consumer doing `require("keyborg/dist/index.cjs")` at a pinned path would break. `package.json` `exports` currently only expose `.` and `./package.json`, so this is low risk but must be verified before merging. + +## Measurement Workflow + +Per-PR ritual: + +1. Branch off the previous PR's tip (or `main` for PR 1). +2. Run `npm run build && npm run bundle-size` before making changes — captures "before" numbers. +3. Make the changes. +4. Re-run `npm run bundle-size`. +5. Paste a before/after table into the PR description covering all three fixtures (minified + gzipped). Headline numbers are `all-exports` and `create-n-dispose`; `focus-in-const` is a noise check. +6. If any fixture grows, stop and investigate before pushing — do not paper over regressions. + +CI. `bundle-size.yml` posts comparisons to PRs via `monosize-storage-git` against the base branch. Stacked PRs target the **previous PR's branch** as their base so the comparison is meaningful, then retarget to `main` after the parent merges. Call this out in each PR description. + +"Done" for each PR: + +- Playwright tests green (`npm test`). +- `npm run lint` and `npm run format` clean. +- Bundle size shrinks or stays within ±0.5% noise for the trivial `focus-in-const` fixture. + +## Risks + +**R1 — PR 2 behavioral drift.** The class-to-closure refactor touches every line of `Keyborg.mts`. Mitigation: keep method signatures, call order, and externally-visible identifiers byte-identical. Only internal state names move from `this._x` to closure locals. + +**R2 — Private closure state vs. debugging.** Closure-based factories produce flatter stack traces than classes. Mitigation: name the factory functions (`createKeyborg`, `createKeyborgCore`) so stack frames stay readable; avoid anonymous handlers. + +**R3 — `__keyborg` global shape is an undocumented contract.** `WindowWithKeyborg.__keyborg` is attached to `window`. External tools (Tabster is referenced in `FocusEvent.mts` line 248) may rely on the shape. Mitigation: PR 2 preserves the outer `{ core, refs }` shape. The internal `core` surface changes; if a consumer reads `core.isNavigatingWithKeyboard`, that is an undocumented leak — flag it in the PR for the maintainer to confirm. + +**R4 — PR 3 target bump affecting emitted helpers for unused features.** Unlikely given the surface (no async generators, no decorators). The measurement spike itself covers this. + +## Rollout + +Branch naming (each stacked off the prior): + +- `perf/drop-weakref-shim` → PR 1 (base: `main`) +- `perf/closure-refactor` → PR 2 (base: `perf/drop-weakref-shim`) +- `perf/es2022-spike` → PR 3 (base: `perf/closure-refactor`) +- `perf/tsc-emit-spike` → PR 4 (base: `main` after PR 2 merges; independent of PR 3) + +PR 3 and PR 4 are independent measurements against the post-PR-2 baseline and can merge in either order, both, or neither. + +Merge order: PR 1 → retarget PR 2 base to `main` → merge PR 2 → open PR 3 and PR 4 off updated `main` → merge or close each based on its measurement. + +Rollback: each PR is a single logical change, `git revert `. PR 2 carries the highest revert risk; the Playwright suite is the tripwire. + +## Out of Scope + +- Raising the browserslist floor. +- Switching to a different bundler beyond the `tsc` spike in PR 4. +- Changing the TS 3.9 `downlevel-dts` typings path. +- Any API additions or removals. diff --git a/src/FocusEvent.mts b/src/FocusEvent.mts index 504df8a25..df950e8cc 100644 --- a/src/FocusEvent.mts +++ b/src/FocusEvent.mts @@ -2,8 +2,6 @@ * Copyright (c) Microsoft Corporation. All rights reserved. * Licensed under the MIT License. */ -import { WeakRefInstance } from "./WeakRefInstance.mts"; - export const KEYBORG_FOCUSIN = "keyborg:focusin"; export const KEYBORG_FOCUSOUT = "keyborg:focusout"; @@ -17,8 +15,8 @@ interface KeyborgFocus { interface KeyborgFocusEventData { focusInHandler: (e: FocusEvent) => void; focusOutHandler: (e: FocusEvent) => void; - lastFocusedProgrammatically?: WeakRefInstance; - shadowTargets: Set>; + lastFocusedProgrammatically?: WeakRef; + shadowTargets: Set>; } /** @@ -101,7 +99,7 @@ export function setupFocusEvent(win: Window): void { kwin.HTMLElement.prototype.focus = focus; - const shadowTargets: Set> = new Set(); + const shadowTargets: Set> = new Set(); const focusOutHandler = (e: FocusEvent) => { const target = e.target as HTMLElement; @@ -227,7 +225,7 @@ export function setupFocusEvent(win: Window): void { shadowRoot.addEventListener("focusin", focusInHandler, true); shadowRoot.addEventListener("focusout", focusOutHandler, true); - shadowTargets.add(new WeakRefInstance(shadowRoot)); + shadowTargets.add(new WeakRef(shadowRoot)); return; } @@ -281,9 +279,7 @@ export function setupFocusEvent(win: Window): void { .__keyborgData; if (keyborgNativeFocusEvent) { - keyborgNativeFocusEvent.lastFocusedProgrammatically = new WeakRefInstance( - this, - ); + keyborgNativeFocusEvent.lastFocusedProgrammatically = new WeakRef(this); } // eslint-disable-next-line prefer-rest-params diff --git a/src/Keyborg.mts b/src/Keyborg.mts index 02f7f588b..0af928dea 100644 --- a/src/Keyborg.mts +++ b/src/Keyborg.mts @@ -9,366 +9,315 @@ import { KEYBORG_FOCUSIN, setupFocusEvent, } from "./FocusEvent.mts"; -import { Disposable } from "./WeakRefInstance.mts"; interface WindowWithKeyborg extends Window { __keyborg?: { - core: KeyborgCore; + core: KeyborgCoreHandle; refs: { [id: string]: Keyborg }; }; } -const _dismissTimeout = 500; // When a key from dismissKeys is pressed and the focus is not moved -// during _dismissTimeout time, dismiss the keyboard navigation mode. +// When a key from dismissKeys is pressed and the focus is not moved during +// _dismissTimeout ms, keyboard navigation mode is dismissed. +const _dismissTimeout = 500; let _lastId = 0; export interface KeyborgProps { - // Keys to be used to trigger keyboard navigation mode. By default, any key will trigger - // it. Could be limited to, for example, just Tab (or Tab and arrow keys). + // Keys to be used to trigger keyboard navigation mode. By default, any key + // will trigger it. Could be limited to, for example, just Tab (or Tab and + // arrow keys). triggerKeys?: number[]; - // Keys to be used to dismiss keyboard navigation mode using keyboard (in addition to - // mouse clicks which dismiss it). For example, Esc could be used to dismiss. + // Keys to be used to dismiss keyboard navigation mode using keyboard (in + // addition to mouse clicks which dismiss it). For example, Esc could be used + // to dismiss. dismissKeys?: number[]; } export type KeyborgCallback = (isNavigatingWithKeyboard: boolean) => void; /** - * Manages a collection of Keyborg instances in a window/document and updates keyborg state + * Internal handle for the per-window core state. Not part of the public API. */ -class KeyborgCore implements Disposable { - readonly id: string; - - private _win?: WindowWithKeyborg; - private _isMouseOrTouchUsedTimer: number | undefined; - private _dismissTimer: number | undefined; - private _triggerKeys?: Set; - private _dismissKeys?: Set; - private _isNavigatingWithKeyboard_DO_NOT_USE = false; - - constructor(win: WindowWithKeyborg, props?: KeyborgProps) { - this.id = "c" + ++_lastId; - this._win = win; - const doc = win.document; +interface KeyborgCoreHandle { + dispose(): void; + getNavigating(): boolean; + setNavigating(val: boolean): void; +} - if (props) { - const triggerKeys = props.triggerKeys; - const dismissKeys = props.dismissKeys; +/** + * Used to determine the keyboard navigation state. + */ +export interface Keyborg { + /** + * @returns Whether the user is navigating with keyboard + */ + isNavigatingWithKeyboard(): boolean; + /** + * @param callback - Called when the keyboard navigation state changes + */ + subscribe(callback: KeyborgCallback): void; + /** + * @param callback - Registered with subscribe + */ + unsubscribe(callback: KeyborgCallback): void; + /** + * Manually set the keyboard navigation state + */ + setVal(isNavigatingWithKeyboard: boolean): void; +} - if (triggerKeys?.length) { - this._triggerKeys = new Set(triggerKeys); - } +// Augments the public Keyborg with internal methods invoked by the core's +// broadcast loop and by disposeKeyborg. Not exported. +interface KeyborgInternal extends Keyborg { + _notify(isNavigatingWithKeyboard: boolean): void; + _dispose(): void; +} - if (dismissKeys?.length) { - this._dismissKeys = new Set(dismissKeys); - } +function createKeyborgCore( + win: WindowWithKeyborg, + props?: KeyborgProps, +): KeyborgCoreHandle { + let currentWin: WindowWithKeyborg | undefined = win; + let isNavigating = false; + let isMouseOrTouchUsedTimer: number | undefined; + let dismissTimer: number | undefined; + let triggerKeys: Set | undefined; + let dismissKeys: Set | undefined; + + if (props) { + if (props.triggerKeys?.length) { + triggerKeys = new Set(props.triggerKeys); } - - doc.addEventListener(KEYBORG_FOCUSIN, this._onFocusIn, true); // Capture! - doc.addEventListener("mousedown", this._onMouseDown, true); // Capture! - win.addEventListener("keydown", this._onKeyDown, true); // Capture! - - doc.addEventListener("touchstart", this._onMouseOrTouch, true); // Capture! - doc.addEventListener("touchend", this._onMouseOrTouch, true); // Capture! - doc.addEventListener("touchcancel", this._onMouseOrTouch, true); // Capture! - - setupFocusEvent(win); - } - - get isNavigatingWithKeyboard() { - return this._isNavigatingWithKeyboard_DO_NOT_USE; - } - - set isNavigatingWithKeyboard(val: boolean) { - if (this._isNavigatingWithKeyboard_DO_NOT_USE !== val) { - this._isNavigatingWithKeyboard_DO_NOT_USE = val; - this.update(); + if (props.dismissKeys?.length) { + dismissKeys = new Set(props.dismissKeys); } } - dispose(): void { - const win = this._win; - - if (win) { - if (this._isMouseOrTouchUsedTimer) { - win.clearTimeout(this._isMouseOrTouchUsedTimer); - this._isMouseOrTouchUsedTimer = undefined; - } - - if (this._dismissTimer) { - win.clearTimeout(this._dismissTimer); - this._dismissTimer = undefined; + const broadcast = (): void => { + const refs = currentWin?.__keyborg?.refs; + if (refs) { + for (const id of Object.keys(refs)) { + (refs[id] as KeyborgInternal)._notify(isNavigating); } + } + }; - disposeFocusEvent(win); - - const doc = win.document; - - doc.removeEventListener(KEYBORG_FOCUSIN, this._onFocusIn, true); // Capture! - doc.removeEventListener("mousedown", this._onMouseDown, true); // Capture! - win.removeEventListener("keydown", this._onKeyDown, true); // Capture! - - doc.removeEventListener("touchstart", this._onMouseOrTouch, true); // Capture! - doc.removeEventListener("touchend", this._onMouseOrTouch, true); // Capture! - doc.removeEventListener("touchcancel", this._onMouseOrTouch, true); // Capture! - - delete this._win; + const setNavigating = (val: boolean): void => { + if (isNavigating !== val) { + isNavigating = val; + broadcast(); } - } + }; - isDisposed(): boolean { - return !!this._win; - } + const shouldTrigger = (e: KeyboardEvent): boolean => { + // TODO Some rich text fields can allow Tab key for indentation so it + // doesn't need to be a navigation key. If there is a bug regarding that + // we should revisit. + if (e.key === "Tab") { + return true; + } + const active = currentWin?.document.activeElement as HTMLElement | null; + const isTriggerKey = !triggerKeys || triggerKeys.has(e.keyCode); + const isEditable = + active && + (active.tagName === "INPUT" || + active.tagName === "TEXTAREA" || + active.isContentEditable); + return isTriggerKey && !isEditable; + }; - /** - * Updates all keyborg instances with the keyboard navigation state - */ - update(): void { - const keyborgs = this._win?.__keyborg?.refs; + const shouldDismiss = (e: KeyboardEvent): boolean => { + return !!dismissKeys?.has(e.keyCode); + }; - if (keyborgs) { - for (const id of Object.keys(keyborgs)) { - Keyborg.update(keyborgs[id], this.isNavigatingWithKeyboard); - } + const scheduleDismiss = (): void => { + const w = currentWin; + if (!w) { + return; } - } + if (dismissTimer) { + w.clearTimeout(dismissTimer); + dismissTimer = undefined; + } + const was = w.document.activeElement; + dismissTimer = w.setTimeout(() => { + dismissTimer = undefined; + const cur = w.document.activeElement; + if (was && cur && was === cur) { + // Esc was pressed, currently focused element hasn't changed. + // Just dismiss the keyboard navigation mode. + setNavigating(false); + } + }, _dismissTimeout); + }; - private _onFocusIn = (e: KeyborgFocusInEvent) => { + const onFocusIn = (e: KeyborgFocusInEvent): void => { // When the focus is moved not programmatically and without keydown events, - // it is likely that the focus is moved by screen reader (as it might swallow - // the events when the screen reader shortcuts are used). The screen reader - // usage is keyboard navigation. - - if (this._isMouseOrTouchUsedTimer) { - // There was a mouse or touch event recently. + // it is likely that the focus is moved by screen reader (as it might + // swallow the events when the screen reader shortcuts are used). The + // screen reader usage is keyboard navigation. + if (isMouseOrTouchUsedTimer) { return; } - - if (this.isNavigatingWithKeyboard) { + if (isNavigating) { return; } - const details = e.detail; - if (!details.relatedTarget) { return; } - if ( details.isFocusedProgrammatically || details.isFocusedProgrammatically === undefined ) { - // The element is focused programmatically, or the programmatic focus detection - // is not working. return; } + setNavigating(true); + }; - this.isNavigatingWithKeyboard = true; + const onMouseOrTouch = (): void => { + const w = currentWin; + if (w) { + if (isMouseOrTouchUsedTimer) { + w.clearTimeout(isMouseOrTouchUsedTimer); + } + isMouseOrTouchUsedTimer = w.setTimeout(() => { + isMouseOrTouchUsedTimer = undefined; + }, 1000); + } + setNavigating(false); }; - private _onMouseDown = (e: MouseEvent): void => { + const onMouseDown = (e: MouseEvent): void => { if ( e.buttons === 0 || (e.clientX === 0 && e.clientY === 0 && e.screenX === 0 && e.screenY === 0) ) { - // This is most likely an event triggered by the screen reader to perform - // an action on an element, do not dismiss the keyboard navigation mode. + // This is most likely an event triggered by the screen reader to + // perform an action on an element, do not dismiss the keyboard + // navigation mode. return; } - - this._onMouseOrTouch(); + onMouseOrTouch(); }; - private _onMouseOrTouch = (): void => { - const win = this._win; - - if (win) { - if (this._isMouseOrTouchUsedTimer) { - win.clearTimeout(this._isMouseOrTouchUsedTimer); + const onKeyDown = (e: KeyboardEvent): void => { + if (isNavigating) { + if (shouldDismiss(e)) { + scheduleDismiss(); } - - this._isMouseOrTouchUsedTimer = win.setTimeout(() => { - delete this._isMouseOrTouchUsedTimer; - }, 1000); // Keeping the indication of mouse or touch usage for some time. + } else if (shouldTrigger(e)) { + setNavigating(true); } - - this.isNavigatingWithKeyboard = false; }; - private _onKeyDown = (e: KeyboardEvent): void => { - const isNavigatingWithKeyboard = this.isNavigatingWithKeyboard; + const doc = win.document; + doc.addEventListener(KEYBORG_FOCUSIN, onFocusIn, true); + doc.addEventListener("mousedown", onMouseDown, true); + win.addEventListener("keydown", onKeyDown, true); + doc.addEventListener("touchstart", onMouseOrTouch, true); + doc.addEventListener("touchend", onMouseOrTouch, true); + doc.addEventListener("touchcancel", onMouseOrTouch, true); - if (isNavigatingWithKeyboard) { - if (this._shouldDismissKeyboardNavigation(e)) { - this._scheduleDismiss(); - } - } else { - if (this._shouldTriggerKeyboardNavigation(e)) { - this.isNavigatingWithKeyboard = true; - } - } - }; + setupFocusEvent(win); - /** - * @returns whether the keyboard event should trigger keyboard navigation mode - */ - private _shouldTriggerKeyboardNavigation(e: KeyboardEvent) { - // TODO Some rich text fields can allow Tab key for indentation so it doesn't - // need to be a navigation key. If there is a bug regarding that we should revisit - if (e.key === "Tab") { - return true; + const dispose = (): void => { + const w = currentWin; + if (!w) { + return; } + if (isMouseOrTouchUsedTimer) { + w.clearTimeout(isMouseOrTouchUsedTimer); + isMouseOrTouchUsedTimer = undefined; + } + if (dismissTimer) { + w.clearTimeout(dismissTimer); + dismissTimer = undefined; + } + disposeFocusEvent(w); + const d = w.document; + d.removeEventListener(KEYBORG_FOCUSIN, onFocusIn, true); + d.removeEventListener("mousedown", onMouseDown, true); + w.removeEventListener("keydown", onKeyDown, true); + d.removeEventListener("touchstart", onMouseOrTouch, true); + d.removeEventListener("touchend", onMouseOrTouch, true); + d.removeEventListener("touchcancel", onMouseOrTouch, true); + currentWin = undefined; + }; - const activeElement = this._win?.document - .activeElement as HTMLElement | null; - const isTriggerKey = !this._triggerKeys || this._triggerKeys.has(e.keyCode); - - const isEditable = - activeElement && - (activeElement.tagName === "INPUT" || - activeElement.tagName === "TEXTAREA" || - activeElement.isContentEditable); - - return isTriggerKey && !isEditable; - } + return { + dispose, + getNavigating: () => isNavigating, + setNavigating, + }; +} - /** - * @returns whether the keyboard event should dismiss keyboard navigation mode - */ - private _shouldDismissKeyboardNavigation(e: KeyboardEvent) { - return this._dismissKeys?.has(e.keyCode); +export function createKeyborg(win: Window, props?: KeyborgProps): Keyborg { + const kwin = win as WindowWithKeyborg; + const id = "k" + ++_lastId; + let localWin: WindowWithKeyborg | undefined = kwin; + let core: KeyborgCoreHandle | undefined; + let callbacks: KeyborgCallback[] = []; + + const existing = kwin.__keyborg; + if (existing) { + core = existing.core; + } else { + core = createKeyborgCore(kwin, props); } - private _scheduleDismiss(): void { - const win = this._win; - - if (win) { - if (this._dismissTimer) { - win.clearTimeout(this._dismissTimer); - this._dismissTimer = undefined; + const instance: KeyborgInternal = { + isNavigatingWithKeyboard() { + return !!core?.getNavigating(); + }, + subscribe(callback) { + callbacks.push(callback); + }, + unsubscribe(callback) { + const index = callbacks.indexOf(callback); + if (index >= 0) { + callbacks.splice(index, 1); } - - const was = win.document.activeElement; - - this._dismissTimer = win.setTimeout(() => { - this._dismissTimer = undefined; - - const cur = win.document.activeElement; - - if (was && cur && was === cur) { - // Esc was pressed, currently focused element hasn't changed. - // Just dismiss the keyboard navigation mode. - this.isNavigatingWithKeyboard = false; + }, + setVal(val) { + core?.setNavigating(val); + }, + _notify(val) { + callbacks.forEach((cb) => cb(val)); + }, + _dispose() { + const wkb = localWin?.__keyborg; + if (wkb?.refs[id]) { + delete wkb.refs[id]; + if (Object.keys(wkb.refs).length === 0) { + wkb.core.dispose(); + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + delete localWin!.__keyborg; } - }, _dismissTimeout); - } - } -} - -/** - * Used to determine the keyboard navigation state - */ -export class Keyborg { - private _id: string; - private _win?: WindowWithKeyborg; - private _core?: KeyborgCore; - private _cb: KeyborgCallback[] = []; - - static create(win: WindowWithKeyborg, props?: KeyborgProps): Keyborg { - return new Keyborg(win, props); - } - - static dispose(instance: Keyborg): void { - instance.dispose(); - } - - /** - * Updates all subscribed callbacks with the keyboard navigation state - */ - static update(instance: Keyborg, isNavigatingWithKeyboard: boolean): void { - instance._cb.forEach((callback) => callback(isNavigatingWithKeyboard)); - } - - private constructor(win: WindowWithKeyborg, props?: KeyborgProps) { - this._id = "k" + ++_lastId; - this._win = win; - - const current = win.__keyborg; - - if (current) { - this._core = current.core; - current.refs[this._id] = this; - } else { - this._core = new KeyborgCore(win, props); - win.__keyborg = { - core: this._core, - refs: { [this._id]: this }, - }; - } - } - - private dispose(): void { - const current = this._win?.__keyborg; - - if (current?.refs[this._id]) { - delete current.refs[this._id]; - - if (Object.keys(current.refs).length === 0) { - current.core.dispose(); - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - delete this._win!.__keyborg; + } else if (process.env.NODE_ENV !== "production") { + console.error( + `Keyborg instance ${id} is being disposed incorrectly.`, + ); } - } else if (process.env.NODE_ENV !== "production") { - console.error( - `Keyborg instance ${this._id} is being disposed incorrectly.`, - ); - } - - this._cb = []; - delete this._core; - delete this._win; - } - - /** - * @returns Whether the user is navigating with keyboard - */ - isNavigatingWithKeyboard(): boolean { - return !!this._core?.isNavigatingWithKeyboard; - } - - /** - * @param callback - Called when the keyboard navigation state changes - */ - subscribe(callback: KeyborgCallback): void { - this._cb.push(callback); - } - - /** - * @param callback - Registered with subscribe - */ - unsubscribe(callback: KeyborgCallback): void { - const index = this._cb.indexOf(callback); - - if (index >= 0) { - this._cb.splice(index, 1); - } - } + callbacks = []; + core = undefined; + localWin = undefined; + }, + }; - /** - * Manually set the keyboard navigtion state - */ - setVal(isNavigatingWithKeyboard: boolean): void { - if (this._core) { - this._core.isNavigatingWithKeyboard = isNavigatingWithKeyboard; - } + if (existing) { + existing.refs[id] = instance; + } else { + kwin.__keyborg = { + core, + refs: { [id]: instance }, + }; } -} -export function createKeyborg(win: Window, props?: KeyborgProps): Keyborg { - return Keyborg.create(win, props); + return instance; } -export function disposeKeyborg(instance: Keyborg) { - Keyborg.dispose(instance); +export function disposeKeyborg(instance: Keyborg): void { + (instance as KeyborgInternal)._dispose(); } diff --git a/src/WeakRefInstance.mts b/src/WeakRefInstance.mts deleted file mode 100644 index 26e940dde..000000000 --- a/src/WeakRefInstance.mts +++ /dev/null @@ -1,54 +0,0 @@ -/*! - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the MIT License. - */ - -// IE11 compat, checks if WeakRef is supported -export const _canUseWeakRef = typeof WeakRef !== "undefined"; - -/** - * Allows disposable instances to be used - */ -export interface Disposable { - isDisposed?(): boolean; -} - -/** - * WeakRef wrapper around a HTMLElement that also supports IE11 - * @see {@link https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/WeakRef} - * @internal - */ -export class WeakRefInstance { - private _weakRef?: WeakRef; - private _instance?: T; - - constructor(instance: T) { - if (_canUseWeakRef && typeof instance === "object") { - this._weakRef = new WeakRef(instance); - } else { - this._instance = instance; - } - } - - /** - * @see {@link https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/WeakRef/deref} - */ - deref(): T | undefined { - let instance: T | undefined; - - if (this._weakRef) { - instance = this._weakRef?.deref(); - - if (!instance) { - delete this._weakRef; - } - } else { - instance = this._instance; - if ((instance as Disposable)?.isDisposed?.()) { - delete this._instance; - } - } - - return instance; - } -} diff --git a/tsdown.config.mts b/tsdown.config.mts index aa203d3b3..7893db9de 100644 --- a/tsdown.config.mts +++ b/tsdown.config.mts @@ -4,7 +4,7 @@ import pkg from "./package.json"; export default defineConfig({ entry: ["src/index.mts"], format: ["cjs", "es"], - target: "es2019", + target: "es2022", env: { PKG_VERSION: pkg.version,