Skip to content

Refactor App.tsx into 5 focused hooks#15

Merged
rulkens merged 12 commits intomainfrom
refactor/app-hooks
May 6, 2026
Merged

Refactor App.tsx into 5 focused hooks#15
rulkens merged 12 commits intomainfrom
refactor/app-hooks

Conversation

@rulkens
Copy link
Copy Markdown
Owner

@rulkens rulkens commented May 6, 2026

Summary

Extract the 909-line src/components/App/App.tsx into 5 focused custom hooks under src/hooks/, leaving App.tsx as a 455-line wiring layer. Pure refactor — no user-visible behavior change.

Hooks created

Hook Owns
useFamousMeta loadFamousSidecars() once at mount; returns { famousMeta, famousXrefs }
useAliasIndex (+ pure buildAliasIndex helper, 5 unit tests) Lazy palette alias load on first Cmd+K
useKeyboardShortcuts Global keydown listener (Cmd+K / Esc / f / h / l / /); takes setPaletteOpen directly so the effect doesn't rebind every render. Form-field guard now includes SELECT
useEngineSettings 17 settings useStates + 15 EngineCallbacks echo callbacks bundled into one
useEngine canvasRef, handleRef, the createEngine startup useEffect, and 9 engine-driven state slices (status, hovered, selected, focused, scale, fps, sourceCounts, loadProgress, currentTier)

Plus dead-code cleanup: removed write-only SpaceMouse useStates, unused isWebHIDSupported import, and hoisted initialMobile to a useState lazy initializer so the SSR guard runs once.

App.tsx: 909 → 455 lines (~50% reduction).

Test count: 826 → 831 (+5 from buildAliasIndex unit tests).

Plan + workflow

Implementation plan: docs/superpowers/plans/2026-05-06-app-tsx-hook-refactor.md. Each task ran through implement → spec compliance review → code quality review (when meaningful) → fix any issues. Final whole-implementation review is in progress; any flagged issues will land as follow-up commits on this branch.

Test plan

  • npm run typecheck clean
  • npm test — 831/831 pass
  • Visual smoke: canvas renders; Cmd+K opens palette; click pin works; f focuses; h home; SettingsPanel sliders apply (point size, brightness, exposure, tone curve); deep-link #focus=m81 lands on M81; tier swap works; survey toggles work
  • Confirm no regression in the SpaceMouse path (it's gated off in the UI; the engine-side wiring still works if re-enabled)

Follow-up (separate PR)

Unrelated bug observed during testing: fallbackOrientation.ts:33 throws "Cannot mix BigInt and other types" on hover for some galaxies — likely a non-BigInt objID slipping through from a synthetic or misconstructed cloud. Defensive coercion at hashSeed entry will fix it. Tracking separately.

🤖 Generated with Claude Code

rulkens and others added 11 commits May 6, 2026 12:33
Move the famous-sidecar load (famous_meta.json + famous_xrefs.json) out
of App.tsx into a dedicated hook.  Pure relocation, no behavior change.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
useFamousMeta returns readonly FamousMetaEntry[]; CommandPalette's
entries prop accepts readonly.  Aligns with useFocusUrlSync and
EngineHandle.selectByAlias which already use readonly.  No mutation
in CommandPalette to relax for.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Move the lazy palette-alias load out of App.tsx.  The pure
buildAliasIndex helper gets full unit coverage in node; the React glue
is a thin useEffect wrapper.  No behavior change.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Add a comment on the useEffect dep array explaining why engineHandleRef
is listed (stable ref, linter satisfaction).  Re-export
AliasIndexEntry alongside UseAliasIndexReturn so callers don't need a
second import path, matching the useFocusUrlSync re-export pattern.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Six-task plan to extract App.tsx (~909 lines) into 5 focused hooks
under src/hooks/.  Each task is self-contained: useFamousMeta,
useAliasIndex, useKeyboardShortcuts, useEngineSettings, useEngine,
plus a final cleanup pass.  Tasks 1 and 2 already landed on this
branch.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Move the global keydown listener (Esc / f / h / l / Cmd+K / /) out of
App.tsx into a dedicated hook.  Pure relocation, no behavior change.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
An inline arrow `() => setPaletteOpen(true)` at the call site was a
fresh identity each render and caused the hook's effect to re-bind
the window keydown listener on every App render.  Take the React
setter directly — it's a stable reference, so the effect re-binds
only when `selected` or `paletteOpen` actually change.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The original App.tsx keydown effect only guarded INPUT / TEXTAREA /
contenteditable.  A focused <select> still receives letter keystrokes
on most platforms (jump-to-option), so hitting `h` while the BiasMode
or ToneMapCurve select was focused would unexpectedly yank the camera
home.  Add SELECT to the guard.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Move ~17 settings useStates and their EngineCallbacks echo slice into
a single hook.  App.tsx now spreads settingsCallbacks into createEngine.
The three App-owned setters (filaments enabled / intensity, exposure)
are returned from the hook for SettingsPanel onChange wiring.  Pure
relocation, no behavior change.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Move createEngine startup, canvasRef, handleRef, and engine-driven
state (status, hovered, selected, focused, scale, fps, sourceCounts,
loadProgress, currentTier) out of App.tsx into a dedicated hook.
useEngine accepts extraCallbacks so useEngineSettings can layer in
its echo callbacks.  Pure relocation, no behavior change.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The SpaceMouse panel section was gated off long ago; its connected /
sensitivity useStates were write-only.  isWebHIDSupported import was
unused.  initialMobile / initialPanelsOpen were re-evaluated every
render despite being mount-time constants — wrap initialMobile in a
useState lazy initializer so the SSR guard + viewport read run once.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@cloudflare-workers-and-pages
Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
skymap f7591a1 Commit Preview URL

Branch Preview URL
May 06 2026, 11:20 AM

@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented May 6, 2026

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
skymap 3d05c30 Commit Preview URL

Branch Preview URL
May 06 2026, 11:23 AM

Address Important + Minor items from the final whole-implementation
review of the hook refactor:

- Drop the dead `pendingTarget` destructure from useFocusUrlSync (the
  hook still runs for its side effects; pendingTarget is reserved for
  a future tier-mismatch banner that doesn't exist yet).
- Reconcile the exposure-handler comment with reality: the optimistic
  setExposure(value) IS needed for snappy slider tracking; explain why
  this differs from the discrete tone-curve / bias-mode controls.
- Rewrite the App.tsx module-level docblock to describe the hook-
  delegation architecture instead of the pre-refactor structure.
- Extend the useEngine effect's lint-suppression comment to name
  `currentTier` alongside `extraCallbacks` so a future reader doesn't
  have to wonder why both are intentionally captured-not-listed.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@rulkens rulkens merged commit dcdccce into main May 6, 2026
2 checks passed
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