Updates the proposal #1 section to reflect the four companion hooks now in src/hooks/events/ (useNitroEventReducer, useMessageEventReducer, useExternalSnapshot, on top of the existing *State hooks) and marks the InfoStand + Inventory pilots from the original Fase 2 plan as adopted. Adds the convention note for state owned outside the listener: keep useState + useMessageEvent and extract the reducer as a pure function, citing the two new reducer modules as reference.
27 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 + adopted in OfferView.tsx, useAvatarInfoWidget
(figure/badges/group merge), and useInventoryFurni (extracted pure
reducers consumed by useMessageEvent setters).
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.
Companions (all implemented in src/hooks/events/):
useNitroEventReducer<S, T>(types, reducer, initial)— multiple event types collapsing into one owned state slice (analogous touseReducerbut driven by renderer events).useMessageEventReducer<S, T>(eventTypes, reducer, initial)— same shape on the server message channel; accepts a single type or an array of types that all feed the same reducer.useExternalSnapshot<T>(subscribe, getSnapshot)—useSyncExternalStorewrapper pairing the renderer'sEventDispatcher.subscribe()with thegetXxxSnapshot()getters added in renderer 2.1.0. Use this for readonly views over manager state (getUserDataSnapshot,getActiveRoomSessionSnapshot).
For state owned outside the listener (the useState + setState(prev => applyX(prev, event)) pattern), keep using useNitroEvent /
useMessageEvent and extract the reducer as a pure function for
testability. See src/hooks/inventory/useInventoryFurni.reducers.ts and
src/hooks/rooms/widgets/avatarInfo.reducers.ts for the convention.
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 (adopted) — rejected, keep the current layout
Update: an earlier version of this document proposed a
src/features/<feature>/layout (vertical slices). The pilot on the doorbell widget showed that the existingsrc/components/<area>/+src/hooks/<area>/split is the convention the team wants to keep. The pilot has been rolled back; this section is left as a record of the decision.
Current convention (the one to follow):
- Views live under
src/components/<area>/<feature>/*.tsx(e.g.src/components/room/widgets/doorbell/DoorbellWidgetView.tsx). - Hooks live under
src/hooks/<area>/<feature?>/*.ts(e.g.src/hooks/rooms/widgets/useDoorbellState.ts). Multiple hooks for the same widget go in the same folder as siblings, not in a per-widget subfolder. - Pure helpers / constants / types that are specific to one view
go in sibling files next to the view (see
src/components/wired-tools/WiredCreatorTools.{types,constants,helpers}.tsfor the established pattern). - Cross-cutting utilities continue to live under
src/api/andsrc/common/.
Discoverability is acceptable as long as the naming is consistent —
useDoorbellState / useDoorbellActions / DoorbellWidgetView are
greppable in seconds even though they live in three separate directory
trees.
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/hooks/rooms/widgets/useDoorbellState.ts— the users list, derived from three events using auseNitroEventReducer-like pattern.src/hooks/rooms/widgets/useDoorbellActions.ts—answer(name, flag).src/hooks/rooms/widgets/useDoorbellWidget.tskept as a deprecated shim that composes the two so existing consumers don't break.
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/state/wired-tools.ts (or src/components/wired-tools/wiredToolsStore.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 (feat/react19-modernization, PR #2) has applied:
Toolchain
- React 19.2 /
react-dom19.2 /@types/react19.2. - TS 6 for build + TS 7 native preview (
tsgo) foryarn typecheck. - ESLint 10 +
typescript-eslint8 +eslint-plugin-react-hooks@7+eslint-plugin-react-compiler. - Vite 8 + React Compiler 1.0 (
babel-plugin-react-compiler). <StrictMode>mounted;App.tsxmade idempotent for the double-mount.
React 19 idioms
forwardRef→refprop on 7 layout/component files (11 call sites).<Ctx.Provider>→<Ctx>on 6 contexts.- Native
<script>inTurnstileWidget,ExternalPluginLoader,GoogleAdsView. - Form Actions (
useActionState+useFormStatus) for the inline Login/Register/Forgot dialogs inLoginView.tsx. Legacy non-Action versions incomponents/login/components/removed as dead code. useEffectEventinApp.tsx,FurniEditorSearchView,NotificationBadgeReceivedBubbleView,NavigatorRoomSettingsRightsTabView,UiSettingsContext,TurnstileWidget— clears all remainingexhaustive-depswarnings.- Targeted
set-state-in-effectfixes:CatalogHeaderView(pure derive),NavigatorRoomCreatorView(lazy state init),LoginView(track-previous-prop reset),ChooserWidgetView(callback inuseEffectEvent).
Patterns + adoption (proposals #1, #2, #4, #5)
useNitroEventState/useMessageEventState+ companions (proposal #1) — adapters insrc/hooks/events/. Selectors are held in auseLayoutEffect-refreshed ref (Dan Abramov's use-event-callback pattern) so the listener stays mounted across renders. Companions for the multi-event → single state-slice case:useNitroEventReducer,useMessageEventReducer, plususeExternalSnapshot(a typed wrapper ofuseSyncExternalStorefor the renderer'sEventDispatcher.subscribe()+getXxxSnapshot()getters added inNitro_Render_V32.1.0). Pilots:OfferView(single-event),useAvatarInfoWidget(3 listeners for figure/badges/group merged via pure reducers — moved out ofInfoStandWidgetUserView, killing 3CloneObjectcalls), anduseInventoryFurni(4 message listeners + fragment buffer refactored to pure reducers; the module-levelfurniMsgFragmentsis now auseRefand the deadFurniturePostItPlacedEventhandler dropped).useNitroQuery(proposal #2) — enabled.@tanstack/react-query+ devtools installed;QueryClientProvidermounted insrc/index.tsx. Adapter atsrc/api/nitro-query/createNitroQuery.tswithselect,accept(correlation-key filter),timeoutMs,staleTime, plus a lower-levelawaitNitroResponse()for imperative use. Pilots:OfferView,CatalogLayoutRoomAdsView,ModToolsChatlogView,CfhChatlogView,useGiftConfiguration(replaces theGiftWrappingConfigurationEventlistener + eager composer dispatch that lived inuseCatalog; consumed directly byCatalogGiftView).- Layout / feature folders (proposal #3) — rejected. The existing
src/components/<area>/<feature>/(views) +src/hooks/<area>/<feature?>/(flat hook files) is the layout that stays. See section 3 above for the full rule. - God-hook split (proposal #4) — applied to:
- doorbell:
useDoorbellState+useDoorbellActions+ shim. - poll:
usePollSubscriptions(mounted once inRoomWidgetsView)usePollActions+ shim.useWordQuizWidgetwas migrated to importusePollActionsdirectly so it doesn't pull subscriptions.
- furni chooser:
useFurniChooserState+useFurniChooserActions- shim. Helper
buildWallItem/buildFloorItemdedupes ~50 lines of inlineRoomObjectItemconstruction (typed viaIRoomObject; the deadsessionDataManager.getUserDatafallback dropped — the method never existed).
- shim. Helper
- user chooser:
useUserChooserState+useUserChooserActions- shim. Helper
buildUserItem. Adds?.guards onroomSession?.userDataManager?to avoid the room-transition NPE pattern.
- shim. Helper
- friend request:
useFriendRequestState(3 useState + 2 event bridges + 1 derive effect) +useFriendRequestActions(thin adapter on the friends store) + shim. ExportsActiveFriendRequesttype.
- doorbell:
- Zustand (proposal #5) — enabled.
zustandinstalled; factory atsrc/state/createNitroStore.ts. First adoption: thelet isCreatingRoom/createRoomTimeoutmodule-level pair inNavigatorRoomCreatorViewreplaced byuseRoomCreatorStore(timer lives in the store closure, survives StrictMode double-mount).
WiredCreatorToolsView decomposition
- Top-level constants/types/helpers extracted to sibling files
(
WiredCreatorTools.{types,constants,helpers}.ts). - All four tab JSX bodies extracted into sibling components:
WiredMonitorTabViewWiredInspectionTabViewWiredVariablesTabViewWiredToolsSettingsTabView(already separate from before this PR)
- The three Monitor-tab overlay popups guarded by
{ false && ... }were dead duplicates of the live overlays mounted at the root level — dropped. - Main view: 4493 → 3544 lines (−21%).
Tests
- Vitest 3 + jsdom +
@testing-library/react+@testing-library/jest-domconfigured. Separatevitest.config.mtsso the runner doesn't drag in the renderer SDK aliases fromvite.config.mjs. - 83 cases passing across 7 test files:
WiredCreatorTools.helpers.test.ts(18) — formatters + snapshot factory.navigatorRoomCreatorStore.test.ts(4) — Zustand store invariants with fake timers.api-utils.test.ts(27) —ConvertSeconds,LocalizeShortNumber,CloneObject,GetWiredTimeLocale,WiredDateToString,PrefixUtils.api-utils-extra.test.ts(16) —ColorUtils,FixedSizeStack,LocalizeFormattedNumber.friendly-time.test.ts(12) —FriendlyTimewith a deterministicLocalizeTextmock (cuts the transitive renderer-SDK import).dedupeBadges.test.ts(6) — slot-preserving badge dedup (covers the helper used by the InfoStand pilot).
yarn test+yarn test:watchscripts added.
Logic bug fixes
- Doorbell close button didn't close while users were pending
(
useEffect(() => setIsVisible(!!users.length))overrode the close). - Doorbell
answer()removed users locally before the server confirmed viaRSDE_ACCEPTED/RSDE_REJECTED, desyncing on network drop. RoomToolsWidgetViewwipednitro.room.historyfrom localStorage on everybeforeunload(every tab close).AvatarInfoPetTrainingPanelViewcrashed ifroomSessionwas null at parser time.useInventoryFurnihad a module-levelfurniMsgFragmentsbuffer that would have collided between two simultaneous client instances (now scoped to auseRefinside the singleton hook).
Dead code removed
src/components/login/components/RegisterDialog.tsx.src/components/login/components/ForgotDialog.tsx.src/components/login/components/shared.ts(consumed only by the two legacy dialogs).useInventoryFurni's emptyFurniturePostItPlacedEventhandler.IRoomSession.sendWhisperGroupMessage+ impl in the renderer (theChatWhisperGroupComposerit referenced never existed; no client call site).
Typecheck baseline
- Repository-wide
tsgo(TS 7 preview) errors driven down to 57 client-side and 0 renderer-side via a series of small targeted sweeps:- Framer-motion
Variantstyping onToolbarView+FriendsBarView(−33). createNitroQueryimport path / generics / Pick subset (−3 + −1 propagation).useFurniChooserStatetyped asIRoomObject+ dead getUserData branch dropped (−10).ColorVariantTypeextended with the 5outline-*bootstrap variants used by the group-forum thread view (−4).- React 19
JSXimport inWiredNeighborhoodSelectorView(−1). showConfirmextra-arg drop inuseOnClickChat(−1).UserContainerViewfriendsCount.toString()(−1).
- Framer-motion
- Renderer-side cluster cleared in a single pass: TS 5.7+
ArrayBufferdrift, Pixi v8Filter[]/WebGLRenderernarrows, missingIGraphicAssetimport, empty-tupleIMessageComposer<[]>,PetBreedingMessageParser.bytesAvailableboolean-vs-number bug,RoomEnterComposerextended with optional spawnX/spawnY to match the Arcturus server (which already reads both ints when present).
Bonus
WidgetErrorBoundary(src/common/error-boundary/) — wraps theRoomWidgetsViewumbrella. A widget crash now degrades gracefully (logged toNitroLogger.error) instead of unmounting the room.CLAUDE.mdat the repo root — onboarding file Claude Code reads at session start. Captures the layout convention, the patterns to use, what's wired up, what isn't, and the open logic bugs.
How to pick the next refactor PR
Foundations are done: React Query enabled with 4 pilot migrations,
Zustand enabled with 1 store, Vitest with 77 cases, error boundary on
the room widgets umbrella, usePollSubscriptions already hoisted to
RoomWidgetsView, WiredCreatorToolsView fully split per tab.
Remaining order of value/risk for the next contributor:
- Migrate
useCatalog's read-only fetches touseNitroQuery. Biggest expected payoff (cache + dedup + loading state for free). The hook is ~1100 lines; start with the page-tree fetch and the handful of fire-and-forget request/response pairs (gift wrapping config, builders-club furni count, sellable pet palettes). The imperative purchase / gift flows stay where they are. Add a Vitest case per migration. - Split
useCatalogalong the doorbell/poll lines (useCatalogData/useCatalogUiState/useCatalogActions, siblings undersrc/hooks/catalog/). Only after step 1 — React Query removes ~60% of the file's responsibility, Zustand can absorb the UI state slice. - Per-widget
WidgetErrorBoundarywrapping insideRoomWidgetsView. The umbrella is in place; granular wrapping means a crash in one widget (e.g.ChatWidgetView) doesn't take down the rest of the room overlay. Mechanical and safe. - Hoist
WiredCreatorToolsView's shared state to a Zustand slice. The 4-tab split is done but the parent still passes ~25 props to each tab. A slice atsrc/components/wired-tools/wiredToolsStore.tswould make each tab subscribe to the keys it needs. - Address the two open logic bugs (see the "Known logic bugs"
section above): the
MainViewCREATED/ENDED race needs a session token; theLayoutFurniImageView/LayoutAvatarImageViewasync fetch race needs a request-id ref (or is solved by migrating the image fetch touseNitroQuerykeyed on props). - Wider Vitest coverage — next worthwhile targets: the
useNitroQueryadapter (timeout + cleanup + accept-filter behavior, needs a stub for@nitrots/nitro-renderer),useDoorbellState/useUserChooserStateevent-reducer logic (needs the same renderer stub).
Skipped intentionally and documented in commit messages:
usePetPackageWidgetanduseWordQuizWidgetgod-hook splits — their "actions" mutate internal state, so a clean data/actions split would need either action arguments or a shared store first.useChatInputWidget/useChatWidget/useAvatarInfoWidget— large state machines, need per-file design before a mechanical split.
Anything else (the LoginView dialog split, the
react-compiler/react-compiler warnings on the remaining big files,
the set-state-in-effect sweep) is a downstream consequence of the
above — easier and safer once the foundations are in place.
Known logic bugs (independent of structural refactor)
These are runtime bugs spotted while doing the structural work. They are not fixed by the patterns above — they need their own PRs with manual QA. Listing them here because there is currently no GitHub Issues board on this repo.
Open
MainView — race between RoomSessionEvent.CREATED and ENDED
src/components/MainView.tsx:47-48 writes the same landingViewVisible
state from two independent listeners with no session-token guard:
useNitroEvent(RoomSessionEvent.CREATED, () => setLandingViewVisible(false));
useNitroEvent(RoomSessionEvent.ENDED, e => setLandingViewVisible(e.openLandingView));
If the events arrive out of order (fast reconnect, network reordering), the final state contradicts the actual session state — landing view stuck open inside a room, or stuck closed at the hotel view. Resolves on next room change.
Fix shape (deferred until useNitroEventReducer companion lands —
see proposal #1):
// One reducer owns both events + the active session token
const { sessionId, landingViewVisible } = useNitroEventReducer<...>(
[RoomSessionEvent.CREATED, RoomSessionEvent.ENDED],
(state, e) => {
if (e.type === RoomSessionEvent.CREATED) {
return { sessionId: e.session.roomId, landingViewVisible: false };
}
if (state.sessionId !== null && e.session.roomId !== state.sessionId) {
return state; // stale ENDED for old session, ignore
}
return { sessionId: null, landingViewVisible: e.openLandingView };
},
{ sessionId: null, landingViewVisible: true }
);
Severity: edge case, observed only after unstable websocket reconnects. UX-degrading, not data-corrupting.
LayoutFurniImageView / LayoutAvatarImageView — async fetch race
In both files an effect kicks off an async processAsImageUrl /
generateImage and writes the result via setImageElement. If props
change twice in quick succession, the first fetch can resolve after
the second one and overwrite the newer image with the older one.
Fix shape: capture a request-id ref at the start of the effect, only write the result if the ref hasn't been bumped meanwhile. Or — better — once React Query (#2) is enabled, model the image fetch as a query keyed on the props tuple; React Query handles cancellation and ordering for free.
Severity: visible only on slow connections / rapid prop changes. Not data-corrupting.
Recently fixed (in this branch)
- Doorbell close button didn't close while users were pending
(
useEffect(() => setIsVisible(!!users.length))overrode the close). Fixed bysrc/components/room/widgets/doorbell/DoorbellWidgetView.tsx(separatedismissedstate, visibility computed in render). - Doorbell optimistic remove without rollback — the original
answer()removed the user from the local list before the server confirmed viaRSDE_ACCEPTED/RSDE_REJECTED, leaving client and server desynced if the network dropped. Fixed by removing the localremoveUsercall: the server-driven events now own the list. Note: a "pending" indicator (so users see their answer is in flight) is desirable — separate small PR. localStorageroom history wiped on every tab close (RoomToolsWidgetView.tsx,useEffectonbeforeunloadremovingnitro.room.history). Fixed by removing thebeforeunloadhandler; history now persists across sessions, which is the only sensible meaning oflocalStorage. If "session-only" was the intent, the right primitive issessionStorage— file an issue if that's actually desired.AvatarInfoPetTrainingPanelViewnull-pointer —roomSession.userDataManager.getPetData(parser.petId)could throw ifroomSessionwas null at the moment the event arrived (between rooms). Fixed with?.chain.