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
14 KiB
Architecture & Refactor Plan
Status: living document, last updated 2026-05-10. This file describes the structural direction the codebase is moving in. Read it before starting a non-trivial refactor — half the value comes from staying consistent, not from each individual change.
Table of contents
- Where the project stands today
- Five structural improvements
- Bonus: error boundaries
- What's already in place
- How to pick the next refactor PR
Where the project stands today
The codebase is a React 19.2 client for the Nitro renderer (Habbo-style hotel client). Most of the architectural pressure comes from the renderer's event-bus + composer/parser model: the UI talks to the server by sending composers and listening to incoming message events. Almost every piece of state in this app is "the latest value seen on a given event".
That model creates two kinds of friction with modern React:
useEffecteverywhere —react-hooks/set-state-in-effectreports ~328 violations across ~280 files. Most are legitimate event-driven updates, but the pattern hides the intent (it reads as "imperative setState on mount/effect" rather than "subscribe to a stream").- God-hooks —
useCatalog(~1100 lines),useChat,useWiredTools,useInventoryFurniall bundle data fetching, UI state, side effects, and computed values into a single export. Components import the whole thing for one field; the React Compiler skips memoization.
Two big files (WiredCreatorToolsView.tsx 4493→3901 lines,
LoginView.tsx 1700) further compound the problem: the Compiler logs
"Compilation Skipped: Existing memoization could not be preserved", which
means manual useMemo/useCallback are not even helping.
The improvements below are ordered so that each one makes the next one easier.
Five structural improvements
1. Event subscriptions as derived state
Problem. Pattern repeated hundreds of times:
const [foo, setFoo] = useState(initial);
useNitroEvent(SomeEvent, e => setFoo(e.payload));
or with the message channel:
const [data, setData] = useState(null);
useMessageEvent(SomeParser, e => {
const parser = e.getParser();
if (!parser) return;
setData(parser.field);
});
The shape of the code obscures the intent ("foo IS the latest event payload")
and makes the lint think we're doing imperative setState in an effect.
Solution. Two thin hooks (src/hooks/events/useNitroEventState.ts
and useMessageEventState.ts):
const foo = useNitroEventState(SomeEvent, e => e.payload, initial);
const data = useMessageEventState(SomeParser, e => e.getParser()?.field ?? null, null);
Internally the selector closure is held in a ref refreshed in commit phase
(useLayoutEffect), so a new selector identity per render does not force
re-subscription. The listener is registered once.
Status. Implemented + 1 pilot adoption (OfferView.tsx).
Adoption. Organic: when a contributor sees a clean "derive-from-single-event" case, they convert it. Do not sweep-replace. The majority of existing subscriptions have side effects, multi-state updates, conditional filters, or state-machine semantics that lose information when forced into a single selector.
Companion to add later. A useNitroEventReducer<S, T>(events, reducer, initial)
for the cases where multiple events affect one state slice
(see useDoorbellWidget — three events, one users array).
2. Server requests as queries
Problem. A request/response pair against the server today looks like:
useEffect(() => {
SendMessageComposer(new GetXComposer());
}, []);
useMessageEvent(YParser, e => {
setData(e.getParser().data);
});
There is no caching, no deduplication, no retry, no loading or error state, no devtools. Every consumer rolls its own. The same request fires multiple times if multiple components mount it.
Solution. Wrap composer/parser pairs in a TanStack Query adapter
(@tanstack/react-query is in the same family as @tanstack/react-virtual
which is already a dependency):
const { data, isLoading } = useNitroQuery({
request: () => new GetXComposer(),
parser: YParser,
select: e => e.getParser().data,
});
Status. Adapter prototype written (src/api/nitro-query/createNitroQuery.ts).
Not wired up because @tanstack/react-query is not yet installed —
deliberately left as a yarn add step the team can approve.
To enable.
yarn add @tanstack/react-query @tanstack/react-query-devtools
Then mount the provider in src/index.tsx:
<QueryClientProvider client={queryClient}>
<App />
<ReactQueryDevtools initialIsOpen={false} />
</QueryClientProvider>
Migration order suggested.
- Read-only catalog data (
useCatalogpage fetches) — biggest win, lowest risk because it's mostly read. - Inventory tabs.
- Navigator search results.
- Marketplace listings.
Push messages (events the server emits without the client asking) keep
using useMessageEventState — they're not requests.
3. Feature folders
Problem. The current layout splits ownership across three trees:
src/components/wired-tools/ (views)
src/hooks/wired-tools/ (hooks)
src/api/wired/ (utility functions, mixed with the wired runtime)
A change to "the wired-tools panel" touches all three. Discoverability is
poor: a new contributor reading WiredCreatorToolsView.tsx cannot guess
useWiredTools lives 4 directory levels away.
Solution. Feature folders. Each feature owns its complete vertical slice:
src/features/wired-tools/
├── index.ts (public API: only what other features can import)
├── views/ (React components)
├── hooks/ (feature-local hooks)
├── state/ (zustand slices, when they exist)
├── types.ts
├── constants.ts
└── helpers.ts
Rule. A feature folder may import:
- React, third-party libs, the renderer SDK
src/common/(UI primitives)src/api/(cross-cutting helpers —LocalizeText,SendMessageComposer)- Other features only via their public
index.ts
A feature folder must not reach into another feature's internals.
Status. Pilot done on src/features/doorbell/ (the doorbell widget,
small enough to migrate cleanly in one PR). The legacy
src/components/room/widgets/doorbell/DoorbellWidgetView.tsx and
src/hooks/rooms/widgets/useDoorbellWidget.ts are kept as compat-shim
re-exports (one line each) so existing import paths still work — they can
be deleted in a follow-up PR.
Migration order suggested. Smallest features first to validate the pattern, then bigger:
- doorbell (done)
- campaign, ads, mod-tools (each <500 lines)
- notification-center, help, hc-center
- catalog, inventory, navigator, wired-tools (multi-thousand lines each)
A jscodeshift codemod could rewrite import paths in bulk, but each
feature's relative-path imports (../../api, etc.) need to be re-targeted
to the new depth — codemod-able but verify by running tsc per feature.
4. Splitting god-hooks
Problem. useCatalog.ts is ~1100 lines. It owns:
- Server fetch lifecycle (request/parser pairs)
- UI state (selected page, current product, filters)
- Side effects (purchases, gift composer dispatch)
- Computed values (pricing display, page tree)
- Cross-cutting helpers (currency lookup, club level checks)
Every component that imports useCatalog() for one field re-runs the
whole thing. The Compiler can't memoize it (too large). Tests can't be
written against a single concern.
Solution. Split by responsibility, not by entity:
useCatalogData() // server data, returns { pages, currentPage, isLoading }
useCatalogUiState() // ui state, returns { selectedNode, setSelectedNode, filters, ... }
useCatalogActions() // imperative actions, returns { purchase, gift, openOffer }
Inside, useCatalogData uses useNitroQuery (#2). useCatalogUiState uses
a Zustand slice (#5). useCatalogActions is a stateless export — just
functions that compose composers.
Status. Pilot done on useDoorbellWidget:
src/features/doorbell/hooks/useDoorbellState.ts— the users list, derived from three events usinguseNitroEventReducer-like pattern.src/features/doorbell/hooks/useDoorbellActions.ts—answer(name, flag).
It's a small hook so the split looks almost theatrical, but the shape is
the same one we want to apply to useCatalog.
Migration order suggested. Largest pain first, moving down:
useCatalog(~1100 LOC) — but only after #2 is enabled (server fetches collapse to a fewuseNitroQuerycalls, removing 60% of the file).useChatInputWidget(~500 LOC)useWiredTools(~600 LOC)useInventoryFurni(~300 LOC)
5. Unified UI store
Problem. Cross-feature UI state lives in:
- React Context (e.g.
UiSettingsContext) - Custom hooks with module-level singletons (
useNavigator's implicit cache) let foo = ...module-level mutable variables — flagged by the React Compiler as "Writing to a variable defined outside a component or hook is not allowed" (currently 5+ violations)localStoragereads in effects
There is no single source of truth, no devtools, no time-travel.
Solution. Adopt Zustand for cross-feature UI state. Each feature owns one slice:
// src/features/wired-tools/state/wiredToolsSlice.ts
export const useWiredToolsStore = create<WiredToolsState>()((set) => ({
activeTab: 'monitor',
setActiveTab: (tab) => set({ activeTab: tab }),
// ...
}));
Components subscribe to specific keys (Zustand re-renders only the subscribers whose selected slice changed):
const activeTab = useWiredToolsStore(s => s.activeTab);
This eliminates the let isCreatingRoom = false module-level pattern and
makes the state ispezionable in dev tools.
Status. Skeleton written (src/state/createNitroStore.ts), not yet
adopted — zustand is not yet installed. Same reason as #2: deliberately
a follow-up yarn add step.
To enable.
yarn add zustand
Then convert the smallest singleton first (suggestion: the
isCreatingRoom/createRoomTimeout pair in
NavigatorRoomCreatorView.tsx — it's a clean 5-line conversion).
Do not wholesale-replace Context. Some Contexts (theming, i18n) are fine as-is. Zustand is for application state, not configuration state.
Bonus: error boundaries
react-error-boundary is already a dependency. A widget crashing in a
room (e.g. malformed pet data in InfoStandWidgetFurniView) currently
takes down the whole UI.
Solution. Wrap each widget root in <ErrorBoundary fallback={null} onError={NitroLogger.error}>.
Implementation lives at src/common/error-boundary/WidgetErrorBoundary.tsx.
Status. Implemented + applied to RoomWidgetsView as the umbrella for
all in-room widgets. A widget crash now degrades gracefully (the offending
widget disappears) instead of unmounting the room.
A more granular pass could wrap each individual widget for finer-grained fallbacks, but the umbrella alone already prevents the worst class of failures.
What's already in place
The current branch (claude/update-react-typescript-He2rs) has applied:
- React 19.2 / TypeScript 7 (Native preview) / ESLint 10 / React Hooks v7 / React Compiler 1.0 — toolchain bump, all warnings audited.
- Form Actions —
<form action={...}>+useActionStateadopted inLoginView.tsx(login, register, forgot dialogs). useEffectEvent— adopted inApp.tsx,FurniEditorSearchView,NotificationBadgeReceivedBubbleView,NavigatorRoomSettingsRightsTabView,UiSettingsContextto clear allreact-hooks/exhaustive-depswarnings.- Targeted
set-state-in-effectcleanup —CatalogHeaderView(pure derive),NavigatorRoomCreatorView(lazy state init),LoginView(track-previous-prop reset),ChooserWidgetView(callback inuseEffectEvent). WiredCreatorToolsViewsplit — types/constants/helpers extracted to sibling files; main view 4493 → 3901 lines.- Pattern #1 (
useNitroEventState) — implemented + 1 pilot. - Pattern #3 (feature folder) — pilot on
src/features/doorbell/. - Pattern #4 (split god-hook) — pilot on the doorbell hook.
- Pattern #2 (
useNitroQuery) — adapter prototype written, not yet enabled (needsyarn add @tanstack/react-query). - Pattern #5 (Zustand store) — skeleton written, not yet enabled
(needs
yarn add zustand). - Bonus (error boundaries) —
WidgetErrorBoundaryapplied atRoomWidgetsView.
How to pick the next refactor PR
Order of value/risk for the next contributor:
- Enable React Query (
yarn add @tanstack/react-query) and migrate one read-onlyuseCatalogfetch as a second pilot. Highest impact, low risk. - Migrate one mid-sized feature to feature folders (e.g.
mod-toolsorcampaign). Mostly mechanical, validates the pattern at a real scale. - Enable Zustand and migrate the
let isCreatingRoom/createRoomTimeoutsingleton inNavigatorRoomCreatorView. Trivial, makes the Compiler stop complaining about cross-component variable writes. - Add tests (still the #1 thing missing — see "What I'd fix" notes). Vitest + jsdom + a tiny mock layer for the renderer would unblock every refactor below.
- Split
useCatalog— the biggest god-hook. Only do this after #1 and #5 in this list (React Query removes 60% of the file's responsibility, Zustand handles its UI state).
Anything else (the per-tab WiredCreatorTools split, the
react-compiler/react-compiler warnings, the set-state-in-effect
sweep, the LoginView dialog split) is a downstream consequence of these
five — easier and safer once the foundations are in place.