Commit Graph

5 Commits

Author SHA1 Message Date
simoleo89 7b062299de useClubGifts + useNitroEventInvalidator: close the catalogOptions bag
This commit drains the last field out of ICatalogOptions (clubGifts)
and deletes the interface — useCatalog no longer owns a catch-all
mutable object that downstream components stuff data into.

Two pieces:

1) New useNitroEventInvalidator(eventType, queryKey, accept?) — a
   small companion to useNitroQuery for the case where the server
   pushes the same event unprompted (e.g. ClubGiftInfoEvent fires
   both as the response to GetClubGiftInfo and again after the user
   claims a gift via SelectClubGiftComposer). It calls
   queryClient.invalidateQueries() on each matching push so the
   next render of any subscriber triggers a fresh queryFn.

2) New useClubGifts() — useNitroQuery on the ClubGiftInfoEvent
   pair, paired with useNitroEventInvalidator so server-driven
   pushes refresh the cache automatically. CatalogLayoutVipGiftsView
   now consumes the query directly. The local optimistic
   'giftsAvailable--' mutation (which side-effected the parser
   object passed back to the catalog state!) is dropped — the
   server's authoritative ClubGiftInfoEvent push is the single
   source of truth via the invalidator.

useCatalog drops the matching listener + the GetClubGiftInfo dispatch
from the catalog-open effect. ICatalogOptions is now empty and
deleted; the catalogOptions / setCatalogOptions state + return-shape
field are removed from useCatalog along with the import.
2026-05-11 22:38:32 +02:00
simoleo89 96b61ff67b Fix 4 typecheck errors in createNitroQuery
- Import path for SendMessageComposer pointed at ../SendMessageComposer
  (non-existent); the actual module lives at ../nitro/SendMessageComposer.
  Worked at runtime via Vite alias, broke at tsgo.
- request factory was typed as () => unknown so passing the return into
  SendMessageComposer (which expects IMessageComposer<unknown[]>)
  failed the cast.
- The Pick<NitroQueryConfig, ...> bundle handed to awaitNitroResponse
  included 'key', which isn't part of that subset.
- When no select is provided, resolve(event) leaked TParser through the
  TData channel; cast to TData (the default TData=TParser fallback is
  fine for typed callers, but the explicit-generic case needed it).

Net tsgo error count: 100 -> 97.
2026-05-11 21:12:34 +02:00
simoleo89 bf84a0c2a6 useNitroQuery: add accept() predicate; migrate two mod-tools chatlog views
Many composer/parser pairs on the Nitro wire are correlation-key based:
the request carries a key (roomId, issueId, etc.) and the response shows
up on the globally-shared event bus, where other components may be
listening for the same parser type with a different key. The previous
useNitroQuery resolved on the FIRST matching parser event regardless of
key — useless for that pattern, which is why two obvious migration
targets (ModToolsChatlogView, CfhChatlogView) were skipped earlier.

Adapter change
- New optional `accept?: (event) => boolean` on NitroQueryConfig.
- In awaitNitroResponse, events for which accept returns false are
  IGNORED rather than resolving the promise. The listener stays
  registered, the timeout still applies. This lets callers do:
    accept: e => e.getParser()?.data.roomId === roomId

Migrations
- src/components/mod-tools/views/room/ModToolsChatlogView.tsx
  - Was: useState<ChatRecordData>(null) + useMessageEvent with
    `if (parser.data.roomId !== roomId) return; setRoomChatlog(...)` +
    a mount-only useEffect dispatching the composer.
  - Now: a single useNitroQuery call keyed on roomId; accept filters
    by roomId; the query is enabled only when roomId is set.
    The composer is no longer re-dispatched on remount within
    staleTime; switching to a different room still triggers a fresh
    fetch because the queryKey changes.
- src/components/mod-tools/views/tickets/CfhChatlogView.tsx
  - Same pattern, keyed on issueId.

Both migrations drop ~15 lines per file (no more local state + manual
listener + manual send) while gaining cache/dedup/loading/error
handling from TanStack Query.

Verification
- yarn eslint on the four files: 1 pre-existing error (the
  IMessageEvent "redundant union" false positive in createNitroQuery
  that we already documented — local sandbox doesn't have the
  renderer SDK installed, so its types resolve as `any`).
- yarn test: 49/49 passing.
- yarn tsc on the four files: clean.
2026-05-11 17:00:06 +00:00
simoleo89 34b1b56788 Enable React Query (proposal #2) + first real-data pilot on OfferView
Phase 1 of the refactor plan in docs/ARCHITECTURE.md.

Install
- yarn add @tanstack/react-query@5 @tanstack/react-query-devtools@5
- Both pinned to ^5 (matches React 19 peer requirement).

Wiring
- src/index.tsx: mounts QueryClientProvider above ErrorBoundary +
  Suspense. Default config: staleTime=30s, retry=1,
  refetchOnWindowFocus=false (chat client, not a data dashboard).

Adapter
- src/api/nitro-query/createNitroQuery.ts: replaces the previous
  prototype that just threw. Exposes:
    * useNitroQuery({ key, request, parser, select, timeoutMs })
      — wraps TanStack's useQuery; queryFn awaits the parser response.
    * awaitNitroResponse(...) — lower-level helper for imperative use
      via queryClient.fetchQuery.
  The Promise:
    1. registers the parser via GetCommunication().registerMessageEvent
    2. dispatches the composer via SendMessageComposer
    3. resolves with select(event) on the first matching parser
    4. rejects after timeoutMs (default 15s)
    5. always cleans up the listener + timeout (cancel-safe).

Pilot
- src/components/catalog/views/targeted-offer/OfferView.tsx:
  the previous useMessageEventState + manual useEffect-send pattern
  becomes a single useNitroQuery call. staleTime:Infinity because the
  targeted offer doesn't change during a session. Subsequent OfferView
  remounts (e.g. opening/closing the dialog) now reuse the cached
  payload — the GetTargetedOfferComposer is no longer re-sent each
  time.

Verification
- yarn eslint on the four touched files: 1 pre-existing
  no-redundant-type-constituents error (IMessageEvent resolves as `any`
  in the local sandbox without the renderer SDK installed; matches the
  12 other pre-existing instances of the same false positive).
- yarn tsc on the four touched files: clean (modulo the
  project-wide TS2307 about @nitrots/nitro-renderer).
- The original prototype's "throw" guard is gone — useNitroQuery is now
  callable.

Migration path (per docs/ARCHITECTURE.md)
- Next adoption targets (read-only fetches first): useCatalog's page
  data, useInventoryFurni's bot listing, Navigator search results,
  Marketplace listings.
- Push messages (server-pushed events the client doesn't request)
  keep using useNitroEventState / useMessageEventState — they're
  subscriptions, not requests.

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