Skip to content

perf: drop unused code paths (event.details alias, KeyborgProps, IE9 probe)#174

Closed
layershifter wants to merge 3 commits into
microsoft:mainfrom
layershifter:perf/drop-unused-paths
Closed

perf: drop unused code paths (event.details alias, KeyborgProps, IE9 probe)#174
layershifter wants to merge 3 commits into
microsoft:mainfrom
layershifter:perf/drop-unused-paths

Conversation

@layershifter
Copy link
Copy Markdown
Member

Summary

Three independent trims that remove code paths none of keyborg's known consumers exercise. Upstreams the changes microsoft/tabster#539 currently applies via patch-package against keyborg@2.14.0's installed bundle.

The commit is split into three commits for clean review and bisection:

  1. event.details alias on KeyborgFocusInEvent — the @deprecated alias of event.detail. Tabster reads e.detail exclusively (verified across src/State/FocusedElement.ts and the rest of the codebase). Drops the field on the interface plus the per-dispatch event.details = details assignment in setupFocusEvent.

  2. KeyborgProps (triggerKeys / dismissKeys) + dismiss timer machineryKeyborgProps was never re-exported from src/index.mts and no known consumer passes props to createKeyborg. Removes the interface, the triggerKeys/dismissKeys Sets, shouldDismiss/scheduleDismiss/dismissTimer/_dismissTimeout, and collapses onKeyDown to a single trigger guard.

  3. canOverrideNativeFocus runtime probe + _canOverrideNativeFocus flag — the probe detected pre-IE9 environments where reassigning HTMLElement.prototype.focus was ignored. Every browser keyborg currently supports honours the override, so the gated details.isFocusedProgrammatically write becomes unconditional. Semantically identical when the override works, which is the only case we still ship for.

Public API impact

  • KeyborgFocusInEvent interface no longer carries details?: KeyborgFocusInEventDetails. The field has been marked @deprecated for several releases; consumers should use event.detail.
  • createKeyborg(win, props?)createKeyborg(win). KeyborgProps type is removed. Not previously re-exported, so the only breakage is callers passing the second argument (the type narrowing already silently ignored unknown shapes — passing undefined continues to work since the parameter is gone entirely).
  • No runtime contract changes on __keyborg.core or __keyborg.refs.

Worth treating as a minor or major bump depending on how the maintainers want to handle the dropped details field.

Bundle deltas (monosize)

                                    Before        After          Δ
All exports                         3.974 kB min  3.363 kB min   −611 B (−15.4%)
                                    1.627 kB gz   1.409 kB gz    −218 B (−13.4%)

createKeyborg + disposeKeyborg      3.806 kB min  3.195 kB min   −611 B (−16.1%)
                                    1.586 kB gz   1.368 kB gz    −218 B (−13.7%)

KEYBORG_FOCUSIN constant            64 B / 80 B   64 B / 80 B    unchanged

Matches microsoft/tabster#539's measurement of −590 B on keyborg's slice of the Tabster bundle within ~20 B of slack.

Tests

Existing Playwright suite (14 tests covering focus behavior, focus-in events, shadow DOM nesting, and cross-version interop) passes unchanged.

Test plan

  • yarn build (CJS + ESM, types)
  • yarn lint
  • yarn format
  • yarn test (14/14)
  • yarn bundle-size
  • Reviewer: decide on semver level (minor with deprecation removal call-out vs. major)

🤖 Generated with Claude Code

layershifter and others added 3 commits May 18, 2026 12:15
`event.details` was kept as an `@deprecated` alias of `event.detail` for
Tabster and other early consumers. The known consumer chain (Tabster's
src/State/FocusedElement.ts and elsewhere) reads `e.detail` exclusively,
and the field has been marked deprecated for multiple releases. Dropping
the field plus the per-dispatch assignment trims a few bytes from every
focus-in event handler call site.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
KeyborgProps was the only argument to `createKeyborg`/`createKeyborgCore`,
gating two features:

* `triggerKeys` — a custom set of keys that should opt the window into
  keyboard-navigation mode in place of "any non-Tab key on a non-editable
  element". Never used by any known consumer; the default predicate is
  already the right behaviour for screen-reader and Tab-driven flows.
* `dismissKeys` — a key set that scheduled a 500 ms timer to dismiss
  keyboard mode if focus did not move (typically wired to Escape). This
  hand-rolled latch duplicated work consumers already do at the
  application level.

Drop the props interface, the `shouldDismiss`/`scheduleDismiss`/
`dismissTimer`/`_dismissTimeout` machinery, and the `triggerKeys`
membership check in `shouldTrigger`. `onKeyDown` collapses to a single
guard: enter keyboard mode if not already in it and the key should
trigger.

`KeyborgProps` was never re-exported from `src/index.mts`, so the public
surface stays compatible aside from the (still-typed) extra `createKeyborg`
parameter going away.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The `canOverrideNativeFocus` helper detected whether reassigning
`HTMLElement.prototype.focus` was respected by the browser. The check
was guarding against pre-IE9 behaviour where prototype overrides were
ignored.

Every browser keyborg currently supports honours the override, so the
`_canOverrideNativeFocus` flag is effectively always true after the
first call. Replace the conditional `details.isFocusedProgrammatically`
write with an unconditional one — semantically identical when the
override works, which is the only case we still ship for — and drop the
probe entirely.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown

📊 Bundle size report

Package & Exports Baseline (minified/GZIP) PR Change
keyborg
All exports
3.974 kB
1.627 kB
3.363 kB
1.409 kB
-611 B
-218 B
keyborg
createKeyborg() & disposeKeyborg()
3.806 kB
1.586 kB
3.195 kB
1.368 kB
-611 B
-218 B
Unchanged fixtures
Package & Exports Size (minified/GZIP)
keyborg
KEYBORG_FOCUSIN constant
64 B
80 B
🤖 This report was generated against dfd7fe32327b37d8746c44aef6cebeda1ecc2397

@layershifter
Copy link
Copy Markdown
Member Author

Splitting into three focused PRs, one per trim, for independent review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant