usePollWidget bundled two unrelated responsibilities:
- three useNitroEvent listeners that bridge RoomSessionPollEvent
(OFFER / ERROR / CONTENT) onto the UI event bus via DispatchUiEvent
— pure side-effects, zero local state, should mount once;
- three imperative actions (startPoll, rejectPoll, answerPoll) that
every consumer wants, but which shouldn't re-register the listeners.
In practice the only consumer of usePollWidget was useWordQuizWidget,
which needed only `answerPoll` — but pulled in the three subscriptions
as a side effect every time the word-quiz widget rendered. That's the
classic god-hook anti-pattern this proposal targets.
Split (mirrors the doorbell pattern already in place):
- src/hooks/rooms/widgets/usePollSubscriptions.ts (new): the three
bridge listeners, returns void. Should be mounted ONCE at the
highest stable level above poll-aware UI (room widgets root). For
now still mounted by the shim — follow-up PR can move it.
- src/hooks/rooms/widgets/usePollActions.ts (new): the three
imperative actions. Defensive `?.` on roomSession so a poll action
during a room transition no longer crashes.
- src/hooks/rooms/widgets/usePollWidget.ts: kept as a deprecated shim
that composes both — preserves the old `{ startPoll, rejectPoll,
answerPoll }` shape so existing consumers don't break.
- src/hooks/rooms/widgets/useWordQuizWidget.ts: migrated to import
usePollActions directly. The word-quiz widget no longer registers
poll subscriptions transitively — its render no longer has the side
effect of subscribing to three renderer events.
Doc
- docs/ARCHITECTURE.md "What's already in place": records both god-hook
splits (doorbell + poll), the now-enabled React Query and Zustand,
and the test infrastructure. Removes the "not yet enabled" markers
for #2 and #5.
- "How to pick the next refactor PR": rewritten to reflect that the
foundations are done. New priority order:
1. migrate useCatalog's read-only fetches to useNitroQuery,
2. hoist usePollSubscriptions to room-session level,
3. split useCatalog along the doorbell/poll lines,
4. broaden Vitest coverage,
5. per-tab WiredCreatorToolsView split.
Verification
- yarn eslint on the touched files: 0 errors / 0 warnings.
- yarn test: 22/22 passing, 2 files, ~1.0s.
- Existing useWordQuizWidget consumers (RoomWidgetsView ->
WordQuizWidgetView) unaffected — they import from the barrel which
still re-exports the same shape.
https://claude.ai/code/session_01GrR87LAqnAEyKG2ZbmQt5Q
Decision: the src/features/<feature>/ layout introduced as proposal #3
(pilot on doorbell in 8ec9d27) is not the convention the team wants. The
existing src/components/<area>/ + src/hooks/<area>/ split is the one
that stays.
What's reverted
- src/features/doorbell/ is removed entirely. The doorbell view and the
two hooks move back under the classic paths:
src/features/doorbell/views/DoorbellWidgetView.tsx
-> src/components/room/widgets/doorbell/DoorbellWidgetView.tsx
src/features/doorbell/hooks/useDoorbellState.ts
-> src/hooks/rooms/widgets/useDoorbellState.ts
src/features/doorbell/hooks/useDoorbellActions.ts
-> src/hooks/rooms/widgets/useDoorbellActions.ts
- The compat shims that lived in those classic paths are dropped now
that the real files are back.
- src/hooks/rooms/widgets/index.ts adds the two new hooks alongside the
existing useDoorbellWidget shim (kept as a deprecated wrapper so any
external consumer importing the old shape via the barrel keeps
working).
What's preserved
- The split between data and actions (proposal #4) — useDoorbellState
and useDoorbellActions remain two separate hooks. This was the actual
improvement, and it's independent of where the files sit.
- The bug fixes from 8ec9d27 (close button race, optimistic-remove
rollback) — both still present, just in the new path.
- src/state/createNitroStore.ts and src/api/nitro-query/createNitroQuery.ts
are left where they are. They aren't feature folders; they're
cross-cutting framework code (Zustand skeleton, React Query adapter
prototype) that any feature can consume.
Doc
- docs/ARCHITECTURE.md section #3 is rewritten to record the decision
rather than recommend the layout. It now describes the convention to
follow:
* views under src/components/<area>/<feature>/
* hooks under src/hooks/<area>/<feature?>/ (siblings, not subfolders
per widget)
* sibling .types/.constants/.helpers files for view-specific code
(e.g. WiredCreatorTools.*.ts)
- "What's already in place" and "Recently fixed" sections updated to
point at the new paths.
- "How to pick the next refactor PR" no longer mentions feature-folder
migration as an option.
Note: the five extra feature folders started this session (reconnect,
nitropedia, ads, hc-center, campaign) were never committed; they only
existed in the working tree and have been restored from HEAD.
Verification
- find src/features -type f -> 0 (directory removed).
- npx tsc --noEmit on all touched files: clean (only the project-wide
pre-existing TS2307 about @nitrots/nitro-renderer not installed
locally remains, same as before).
- npx eslint on all touched files: 0 errors, 0 warnings.
https://claude.ai/code/session_01GrR87LAqnAEyKG2ZbmQt5Q
These are the bugs surfaced during the structural work that are simple
enough to fix in isolation. Larger ones (race conditions that need
session-token tracking, async-fetch ordering) are deferred and documented
in docs/ARCHITECTURE.md "Known logic bugs" — the repo has Issues
disabled, so the doc is the issue board.
== Fix: room history wiped on every tab close
src/components/room/widgets/room-tools/RoomToolsWidgetView.tsx had a
useEffect that registered a `beforeunload` handler calling
`window.localStorage.removeItem('nitro.room.history')`. The whole point
of localStorage is to persist across sessions; wiping it on tab close is
either a leftover debug call or a misunderstanding of the API.
Removed the handler. History now persists across browser sessions, which
matches user expectations. If "session-only" was the intent, the right
primitive is `sessionStorage` (not localStorage + cleanup) — left as a
note in the doc.
== Fix: AvatarInfoPetTrainingPanelView null-pointer on session change
src/components/room/widgets/avatar-info/AvatarInfoPetTrainingPanelView.tsx
read `roomSession.userDataManager.getPetData(parser.petId)` without
guarding for `roomSession` being null. The PetTrainingPanelMessageEvent
can arrive during a room transition when `roomSession` is briefly null,
crashing the widget. Added `?.` chain on both `roomSession` and
`userDataManager`.
== Doc: known logic bugs section
Two open issues documented for follow-up:
- MainView.tsx CREATED/ENDED race — needs session-token tracking, fits
cleanly into the future useNitroEventReducer companion to proposal #1.
- LayoutFurniImageView / LayoutAvatarImageView async fetch ordering —
needs request-id refs, or solves itself once React Query (proposal #2)
is enabled and the image fetch becomes a query keyed on props.
Plus a "recently fixed" subsection that records the four bugs already
addressed in this branch (doorbell close button, doorbell optimistic
remove, room history wipe, pet panel null-pointer) so the next reader
knows what changed and why.
== Verification
- yarn eslint on the two modified files: same error count before and
after (5 pre-existing set-state-in-effect on RoomToolsWidgetView,
none introduced).
- yarn tsc on the two modified files: clean.
https://claude.ai/code/session_01GrR87LAqnAEyKG2ZbmQt5Q
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
simoleo89