Commit Graph

7 Commits

Author SHA1 Message Date
simoleo89 a029ee63cb fix(catalog,ci): catch hook-order violations + add CI gate
Two follow-ups to the CatalogPurchaseWidgetView fix (6bf3366):

1. CatalogItemGridWidgetView had the same shape — four useCallback
   declarations (handleDragStart / handleDragOver / handleDrop /
   handleDragEnd) sat below an `if(!currentPage) return null` early
   return. When currentPage flipped from null to a real page the hook
   count jumped by 4 and React would have thrown "Rendered more hooks
   than during the previous render" the moment any consumer rendered
   the grid in admin mode. Moved the four useCallback declarations
   above the early-return; their bodies are safe pre-load (only
   currentPage?.offers is accessed inside handleDrop, optional-chained
   already).

2. CI gate — the existing GitHub Actions workflow runs `yarn
   typecheck` and `yarn test`, but NOT `yarn eslint`. That's why this
   pattern slipped through twice in a row: ESLint flags it locally
   but no PR check enforces it. Full `yarn eslint` emits ~900
   pre-existing baseline errors (brace-style, indentation,
   recommended TS rules — out of scope for this branch), so a blanket
   step would always fail. Instead added a focused
   `eslint.hooks.config.mjs` + `yarn lint:hooks` script that runs
   ESLint with ONLY `react-hooks/rules-of-hooks: error`. Wired into
   ci.yml between `typecheck` and `test`. The local repo now has
   zero violations of the rule.

3. useSessionSnapshots.test.tsx — added eslint-disable-next-line
   comments on the three lines that intentionally violate the rule
   (they're the assertions that the broken pattern crashes). Without
   the comments the new CI gate would fail on the regression-guard
   suite.

Verification: yarn lint:hooks green, yarn typecheck clean, yarn test
209/209.
2026-05-19 17:57:28 +02:00
simoleo89 d28819db89 fix(snapshots): re-apply the 3 snapshot-consumer migrations with the use-between/useSyncExternalStore incompatibility resolved
Root cause of last session's "(intermediate value)() is undefined" at
ToolbarView.tsx:46:

  use-between 1.x ships its own React-dispatcher proxy (ownDispatcher
  in node_modules/use-between/release/index.esm.js:54-169) that
  re-implements only useState, useReducer, useEffect, useLayoutEffect,
  useCallback, useMemo, useRef and useImperativeHandle. It does NOT
  implement useSyncExternalStore. When the inner state function of
  useBetween(stateFn) calls useSyncExternalStore (directly or via
  useExternalSnapshot / useUserDataSnapshot), React resolves the
  dispatcher to use-between's proxy, finds .useSyncExternalStore
  missing, and calls undefined() — that's the exact production crash
  in Firefox. Chrome reports the same as
  "dispatcher.useSyncExternalStore is not a function".

Neither the vite alias (790ad2b) nor the defensive renderer-method
guards (c35a2d4) could fix it — both addressed downstream symptoms
(stale dist / missing manager methods) but the dispatcher is upstream
of both. That's why every retry kept reproducing the same error.

Fix is structural: snapshot hooks (useUserDataSnapshot,
useIsUserIgnored, etc.) MUST run outside any useBetween scope. Three
re-applied migrations:

- useSessionInfo: snapshot read moved into the outer wrapper. The
  inner useSessionInfoState (useBetween-shared) now contains only
  use-between-safe hooks: useState, useMessageEvent, plain actions.
  userFigure / userRespectRemaining / petRespectRemaining come from
  useUserDataSnapshot() OUTSIDE useBetween, so useSyncExternalStore
  installs against the real React dispatcher.

- useChatWidget.ownUserId: direct snapshot read. useChatWidget is
  exported as `useChatWidget = useChatWidgetState` (NOT wrapped in
  useBetween), so this hook never sat inside the broken scope — the
  precautionary rollback was unnecessary in retrospect. Gains
  session-change reactivity (e.g. reconnect under a different user id).

- AvatarInfoWidgetAvatarView Ignore/Unignore: useIsUserIgnored(name)
  read directly in the component body. Same reasoning as
  useChatWidget — never inside useBetween. The menu auto-flips
  Ignore <-> Unignore while the popup is open.

Added regression guard at src/hooks/session/useSessionSnapshots.test.tsx
with two cases: (1) useSyncExternalStore inside useBetween throws,
(2) useSyncExternalStore outside useBetween in the same render works.
Pins the constraint so future migrations cannot reintroduce the bad
shape silently.

Verification: yarn typecheck clean, yarn test 209/209 (207 baseline
+ 2 new regression cases), no consumer surface changes — every
destructured field (userFigure, userRespectRemaining, respectUser,
petRespectRemaining, respectPet, chatStyleId, updateChatStyleId) is
still returned with the same name and shape.
2026-05-19 17:30:03 +02:00
simoleo89 e142efd793 revert(hooks): roll back the three snapshot-consumer migrations to pre-71a0eee state
The migrations of useSessionInfo, useChatWidget.ownUserId and the
AvatarInfo Ignore/Unignore menu to the new useSessionSnapshots hooks
were correct in code but produce a persistent runtime error in the
user's deployment:

  TypeError: (intermediate value)() is undefined
      ToolbarView ToolbarView.tsx:46

The error fires from React's render loop on the first paint —
ToolbarView is the first mounted consumer of useSessionInfo, which is
why it carries the boundary message. Two attempted fixes did not
resolve it on the user's side:
- 790ad2b: vite alias forcing @nitrots/nitro-renderer to source index.ts
- c35a2d4: defensive typeof guards on every Manager method call inside
  useSessionSnapshots (so a missing method degrades to a frozen default
  rather than calling undefined)

Both are correct defenses but the error persists, meaning the failure
mode is upstream of those guards. Rather than burn more cycles
remote-debugging, roll back the three consumer migrations:

- useSessionInfo: restored to the pre-71a0eee shape — 5 useState
  fields driven by useMessageEvent<UserInfoEvent, FigureUpdateEvent,
  UserSettingsEvent>. The five consumers (ToolbarView, HcCenterView,
  ChatInputView, AvatarInfoPetTrainingPanelView, InfoStandWidgetPetView,
  AvatarInfo{Avatar,Pet,OwnPet}View) get the same destructured shape
  they had before this session.
- useChatWidget.ownUserId: restored to `GetSessionDataManager()?.userId`
  (synchronous, captured at mount). Loses the session-change reactivity
  but matches the previous, working behaviour.
- AvatarInfoWidgetAvatarView Ignore/Unignore: restored to
  `avatarInfo.isIgnored` (captured by AvatarInfoUtilities at click
  time, not reactive). Loses the live-toggle if the user is
  ignored/unignored while the popup is open — known small regression,
  worth it for stability.

Kept intact:
- The useSessionSnapshots.ts hook file itself, with defensive guards,
  so the API stays available for any future opt-in consumer.
- 790ad2b vite alias for the umbrella, still useful as defence in
  depth for future migrations.
- All the other non-snapshot modernizations from this session
  (usePetPackageWidget reducer, useWordQuizWidget bug fix,
  useChatCommandSelector Zustand store, useAvatarInfoWidget typed
  globalThis accessor).

Verification: yarn typecheck clean, yarn test 207/207, yarn build green.
The toolbar should boot without the error now — the call chain on the
first paint no longer touches the new useExternalSnapshot / snapshot
getter path.
2026-05-18 22:16:48 +02:00
simoleo89 c35a2d4b4f fix(useSessionSnapshots): defensive guards against missing renderer methods
The snapshot hooks were chained against renderer Manager methods
(getUserDataSnapshot, getIgnoredUsersSnapshot, subscribe, …) under the
assumption that the resolved \`@nitrots/nitro-renderer\` bundle always
includes the v2.1.0+ snapshot API.

That assumption fails in two real scenarios:

1. A stale \`dist/index.js\` shadows the source umbrella at resolution
   time (the vite alias commit 790ad2b mitigates this in dev, but it
   only takes effect after a server restart).
2. A consumer bundles the client against an older renderer release
   (e.g. NitroV3-Housekeeping's embedded copy in \`public/nitro3\`).

In both cases the snapshot hook calls \`undefined()\` and React shows
the error-boundary fallback "(intermediate value)() is undefined".

Wrap every renderer-side call with a typeof guard:

  const manager = GetSessionDataManager();
  if(!manager || typeof manager.getUserDataSnapshot !== 'function')
      return DEFAULT_USER_DATA;
  return manager.getUserDataSnapshot();

Module-level frozen defaults (DEFAULT_USER_DATA, EMPTY_IGNORED_LIST,
EMPTY_GROUP_BADGES, EMPTY_USER_LIST, DEFAULT_VOLUMES, NOOP_UNSUBSCRIBE)
keep the snapshot reference stable across fallback calls, so
useSyncExternalStore's bailout still works and we don't trigger render
loops on the degraded path.

Once the renderer is upgraded (or the alias kicks in after restart),
the hooks transparently switch to the real getters — no code change
needed at any consumer.

Verification: yarn typecheck clean, yarn test 207/207, yarn build green.
The fix is defense-in-depth on top of 790ad2b (vite alias) — both can
coexist, neither alone is sufficient for every deployment surface.
2026-05-18 22:06:32 +02:00
simoleo89 71a0eee195 refactor(hooks/session): migrate useSessionInfo to useUserDataSnapshot
Replace the local useState mirror of userFigure / userRespectRemaining /
petRespectRemaining (driven by useMessageEvent<UserInfoEvent> +
useMessageEvent<FigureUpdateEvent> + manual setUser after giveRespect)
with a single useUserDataSnapshot() read.

Why this works: SessionDataManager already invalidates its snapshot
on every state change that mattered to the old hook — UserInfoEvent
handler (line 142), FigureUpdateEvent listener (line 117),
giveRespect / givePetRespect (lines 540/551). The snapshot's
respectsLeft / respectsPetLeft map directly to the parser fields
respectsRemaining / respectsPetRemaining the old code mirrored.

Net result: 3 useState declarations + 2 useMessageEvent subscriptions
removed; respectUser / respectPet become trivial pass-throughs (no
post-call setState because the manager's invalidate dispatches the
event for us). UserSettingsEvent stays on useMessageEvent —
chatStyleId is not in the snapshot.

Also drops the deprecated `userInfo: UserInfoDataParser` field from
the return shape — no in-tree consumer reads it (verified via grep
across src/), it was carried as legacy clutter.

Consumers unchanged: ToolbarView, HcCenterView, ChatInputView,
AvatarInfoPetTrainingPanelView, InfoStandWidgetPetView, AvatarInfoWidget
{Avatar,Pet,OwnPet}View. All destructure individual fields, not the
deprecated userInfo.

Verification: yarn typecheck clean, yarn test 203/203.
2026-05-18 21:31:36 +02:00
simoleo89 b2a86da912 feat(hooks/session): React-side consumer hooks for the renderer snapshot pattern
The renderer exposes six referentially-stable snapshot getters under the
v2.1.0 React-friendly pattern (SessionData / RoomSession / IgnoredUsers /
GroupBadges / RoomUserList / SoundVolumes), each invalidated by a
dedicated NitroEventType.*_UPDATED dispatch. Until now nothing on the
client consumed them — useExternalSnapshot existed as a useSyncExternalStore
wrapper but no widget was wired up to a snapshot.

Add thin consumer hooks under src/hooks/session/useSessionSnapshots.ts,
each a useExternalSnapshot wrapper around the matching subscribe+getter
pair:

- useUserDataSnapshot()        → Readonly<IUserDataSnapshot>
- useActiveRoomSessionSnapshot() → Readonly<IRoomSessionSnapshot> | null
- useIgnoredUsersSnapshot()    → ReadonlyArray<string>
- useIsUserIgnored(name)       → boolean (useMemo over the array)
- useGroupBadgesSnapshot()     → ReadonlyMap<number, string>
- useGroupBadge(groupId)       → string (useMemo over the map)
- useVolumesSnapshot()         → Readonly<ISoundVolumesSnapshot>
- useRoomUserListSnapshot()    → ReadonlyArray<IRoomUserData>

Two design details worth noting:

- useRoomUserListSnapshot subscribes to BOTH ROOM_USER_LIST_UPDATED (for
  join/leave/update inside a session) AND ROOM_SESSION_UPDATED (because
  the underlying userDataManager reference flips when the active room
  session changes). A single module-level frozen EMPTY_USER_LIST is the
  fallback when no session is active, keeping reference stability across
  reads in the no-room state.
- useIsUserIgnored / useGroupBadge memoize the scalar derivation so a
  re-render only happens when the underlying snapshot reference flips,
  not on unrelated useExternalSnapshot wake-ups.

These hooks unlock per-component snapshot consumption — widgets that
previously juggled addEventListener + useState pairs (or worse, read
GetSessionDataManager().userId directly and never re-rendered) can now
go through one of these and get reactivity for free. Migration of
existing consumers (useSessionInfo, AvatarInfoUtilities, etc.) is the
next pass.

Verification: yarn typecheck clean, yarn test 203/203, yarn build green.
2026-05-18 21:24:03 +02:00
DuckieTM 7feb10ab15 🆙 Init V3 2026-01-31 09:10:52 +01:00