refactor(core): collapse activeElement & focusManager indirection#16
Merged
Conversation
- Replace the mutable focusPath array with a real solid signal; updateFocusPath now sets it through the existing _signalWrapper, eliminating the mirror effect the primitive used to maintain. - Collapse the two-layer useFocusManager (core + primitive) into a single function in core that captures its calling owner via getOwner/runWithOwner, attaches its own document listeners, and self-registers cleanup via onCleanup. - Reduce primitives/useFocusManager.ts to a thin re-export shim so existing imports (announcer, etc.) keep working. One source of truth for activeElement and focusPath, one useFocusManager, no options-object/core-wrapper indirection. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
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
1 participant
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.
Summary
Two passes of cleanup against the
src/coreboundary, focused on the activeElement / focus-path machinery:Collapse
Config.setActiveElementindirection. The solid signal for the active element used to live insrc/activeElement.ts, withcore/focusManager.tsnotifying it via a settableConfig.setActiveElementcallback that the primitiveuseFocusManagerre-wrapped to inject owner context. That's three layers for one signal write. The signal now lives incore/focusManager.ts, theConfig.setActiveElementfield is gone, andsrc/activeElement.tsis deleted.Merge the primitive
useFocusManagerinto core. Now that core already importssolid-js(for the signal), the split into a core function + primitive wrapper was historical. The user-facinguseFocusManagerlives in core, captures its calling owner itself, and self-registers cleanup.focusPathis now a real solid signal —updateFocusPathupdates it through the same owner-context wrapper used for key events, replacing the previous mirror effect the primitive maintained.src/primitives/useFocusManager.tsis reduced to a 7-line named re-export so existing import paths (announcer, etc.) keep working untouched.What changes for callers
activeElement,setActiveElement,useFocusManager,focusPath,KeyMap,KeyHoldOptionsare all still exported from the same import paths (@solidtv/solidand@solidtv/solid/primitives).Config.setActiveElementis removed from theConfiginterface. If anything outside this repo was assigning to it, that assignment is now dead code and the assignment site can be deleted.Why
One source of truth for active element and focus path. No callback re-binding, no two-layer
useFocusManagerCore/useFocusManager, no separate signal that has to be kept in sync via an effect. The owner-context dance that the primitive used to perform is now four lines insideuseFocusManager.Test plan
npm run tsc— cleannpm test— 120/120 passnpm run lint— 0 errors (pre-existing warnings only).setFocus(), announcer focus-path subscription🤖 Generated with Claude Code