Commit Graph

2 Commits

Author SHA1 Message Date
simoleo89 fd1835ca5d Enable Zustand (proposal #5) + convert isCreatingRoom singleton
Phase 2 of the refactor plan in docs/ARCHITECTURE.md.

Install
- yarn add zustand (^5, matches React 19 peer requirement).

Wiring
- src/state/createNitroStore.ts: replaces the previous prototype
  (which threw on call) with a re-export of zustand's `create` under
  the project-local name `createNitroStore`. Comments document the
  convention (one store per domain, subscribe to slices not the whole
  store).

First migration target
- src/components/navigator/views/navigatorRoomCreatorStore.ts (new):
  a Zustand store with `isCreating: boolean` and `beginCreate()` —
  the latter latches the flag to true, dispatches an internal
  setTimeout to auto-reset after 5s, and replaces any in-flight timer
  on re-entry. The timer handle lives in the store's closure, so a
  remount of the view doesn't reset the lockout and StrictMode's
  double-mount no longer schedules two pending timers.
- src/components/navigator/views/NavigatorRoomCreatorView.tsx:
  removes the two module-level `let` variables that the React Compiler
  was flagging ("Writing to a variable defined outside a component is
  not allowed"). The component now reads `isCreating` via a slice
  subscription and calls `beginCreate()` from the click handler. The
  imperative guard (`if (isCreating) return`) uses
  `useRoomCreatorStore.getState()` so it reads the latest value
  synchronously without being a stale closure.
- Also cleans up `FC<{}>` -> `FC` while touching the file.

Verification
- yarn eslint on the three touched files: 1 pre-existing error
  (the `setCategory(categories[0].id)` set-state-in-effect on the
  categories hook, deliberately left as-is in Phase C — it's the
  "init from late-arriving async data" pattern; baseline matches).
- yarn tsc: clean.

Migration path (per docs/ARCHITECTURE.md)
- This is the smallest possible Zustand pilot (~30 lines), chosen
  because the let-singleton anti-pattern was the most obvious quick
  win and the React Compiler was already complaining about it.
- Next adoption targets (cross-feature UI state): the toolbar's
  active-window state (currently inside scattered Contexts), the
  notification center's open-state, the catalog's currentPage/selection
  state (after the god-hook split).

https://claude.ai/code/session_01GrR87LAqnAEyKG2ZbmQt5Q
2026-05-11 16:31:53 +00:00
simoleo89 48d62c5c6b Architecture refactor: docs + 5 pilot implementations + error boundary
This is the structural plan promised in the previous session, with concrete
pilots for all five proposals + the bonus error-boundary work.

== docs/ARCHITECTURE.md (new, ~370 lines)

Living document describing:
- where the project stands today (event-bus pattern friction with React 19,
  god-hooks, oversized files);
- the five proposed structural improvements with the why/how/status of each;
- what's already in place across this branch;
- recommended order for the next refactor PRs.

This is the deliverable the rest of this commit references.

== Proposal #3 + #4 pilots: src/features/doorbell/ (new)

Concrete feature-folder migration on the doorbell widget (chosen because
it's small enough to migrate end-to-end in one commit).

  src/features/doorbell/
    index.ts                    public API
    views/DoorbellWidgetView.tsx
    hooks/useDoorbellState.ts   reduces 3 events into a users array (data only)
    hooks/useDoorbellActions.ts answer(name, flag) (imperative actions only)

The split (data vs actions) is the pattern proposal #4 wants applied to
useCatalog/useChat/useWiredTools later. The original useDoorbellWidget had
both concerns + a buggy `useEffect(() => setIsVisible(!!users.length), [users])`
derive-state-in-effect. The new view computes visibility in render.

Compat shims kept so existing imports keep working:
- src/components/room/widgets/doorbell/DoorbellWidgetView.tsx -> 1-line re-export
- src/hooks/rooms/widgets/useDoorbellWidget.ts -> deprecated wrapper around
  the two new hooks, returning the same { users, answer } shape.

== Proposal #2 prototype: src/api/nitro-query/ (new)

Adapter outline for wrapping composer/parser request-response pairs in
TanStack Query. Not yet enabled because @tanstack/react-query is not in
package.json. The file documents the activation steps:

  yarn add @tanstack/react-query @tanstack/react-query-devtools
  + mount QueryClientProvider in src/index.tsx

awaitNitroResponse() throws with a helpful pointer to the doc section if
called before activation, so accidental adoption fails loudly.

== Proposal #5 skeleton: src/state/createNitroStore.ts (new)

Same pattern: skeleton + activation instructions. Not yet enabled because
zustand is not in package.json.

  yarn add zustand
  + replace the throw with `import { create } from 'zustand'; export const createNitroStore = create;`

The doc inside the file shows the recommended slice shape and points to
the suggested first migration target (the let isCreatingRoom singleton in
NavigatorRoomCreatorView).

== Bonus: WidgetErrorBoundary

src/common/error-boundary/WidgetErrorBoundary.tsx wraps react-error-boundary
with a sensible default (silent fallback, NitroLogger.error). Re-exported
from src/common/index.ts.

Applied as the umbrella around RoomWidgetsView's children — a widget
crash in a room (e.g. malformed pet data) now degrades gracefully
instead of unmounting the whole UI.

== Verification

- yarn eslint on all new + modified files: 0 errors / 0 warnings introduced.
  RoomWidgetsView still has its 1 pre-existing FC<{}> error (1 before, 1 after).
- yarn tsc on all new files: clean (only project-wide pre-existing
  TS2307 about @nitrots/nitro-renderer not installed locally remains).
- No regressions: existing imports of DoorbellWidgetView and
  useDoorbellWidget keep resolving via the compat shims.

== What's NOT in this commit (intentionally)

- Mass adoption of the new patterns elsewhere — left as follow-up PRs in
  the order documented in ARCHITECTURE.md "How to pick the next refactor PR".
- Installation of @tanstack/react-query / zustand — explicit team decision,
  not the LLM's to make.
- Test infrastructure (Vitest setup) — listed as the #1 missing piece in
  the doc, but a separate PR.

https://claude.ai/code/session_01GrR87LAqnAEyKG2ZbmQt5Q
2026-05-11 16:31:52 +00:00