From d28819db89395ebaebe5aae033e48cededb0f6d4 Mon Sep 17 00:00:00 2001 From: simoleo89 Date: Tue, 19 May 2026 17:30:03 +0200 Subject: [PATCH] fix(snapshots): re-apply the 3 snapshot-consumer migrations with the use-between/useSyncExternalStore incompatibility resolved MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../menu/AvatarInfoWidgetAvatarView.tsx | 11 +- src/hooks/rooms/widgets/useChatWidget.ts | 8 +- src/hooks/session/useSessionInfo.ts | 68 ++++++------ .../session/useSessionSnapshots.test.tsx | 101 ++++++++++++++++++ 4 files changed, 149 insertions(+), 39 deletions(-) create mode 100644 src/hooks/session/useSessionSnapshots.test.tsx diff --git a/src/components/room/widgets/avatar-info/menu/AvatarInfoWidgetAvatarView.tsx b/src/components/room/widgets/avatar-info/menu/AvatarInfoWidgetAvatarView.tsx index 582b9a1..60345eb 100644 --- a/src/components/room/widgets/avatar-info/menu/AvatarInfoWidgetAvatarView.tsx +++ b/src/components/room/widgets/avatar-info/menu/AvatarInfoWidgetAvatarView.tsx @@ -3,7 +3,7 @@ import { FC, useEffect, useMemo, useState } from 'react'; import { FaChevronLeft, FaChevronRight } from 'react-icons/fa'; import { AvatarInfoUser, DispatchUiEvent, GetOwnRoomObject, GetUserProfile, LocalizeText, MessengerFriend, ReportType, RoomWidgetUpdateChatInputContentEvent, SendMessageComposer } from '../../../../../api'; import { Flex } from '../../../../../common'; -import { useFriends, useHelp, useRoom, useSessionInfo, useWiredTools } from '../../../../../hooks'; +import { useFriends, useHelp, useIsUserIgnored, useRoom, useSessionInfo, useWiredTools } from '../../../../../hooks'; import { ContextMenuHeaderView } from '../../context-menu/ContextMenuHeaderView'; import { ContextMenuListItemView } from '../../context-menu/ContextMenuListItemView'; import { ContextMenuView } from '../../context-menu/ContextMenuView'; @@ -31,6 +31,11 @@ export const AvatarInfoWidgetAvatarView: FC = p const { roomSession = null, isHandItemBlocked = false } = useRoom(); const { userRespectRemaining = 0, respectUser = null } = useSessionInfo(); const { openInspectionForUser, showInspectButton } = useWiredTools(); + // Reactive: the menu auto-flips Ignore <-> Unignore if the state + // changes while the popup is open. Direct hook call (no useBetween + // scope here) so useSyncExternalStore installs against the real + // React dispatcher. + const isIgnored = useIsUserIgnored(avatarInfo.name); const isShowGiveRights = useMemo(() => { @@ -231,11 +236,11 @@ export const AvatarInfoWidgetAvatarView: FC = p { LocalizeText('infostand.link.relationship') } } - { !avatarInfo.isIgnored && + { !isIgnored && processAction('ignore') }> { LocalizeText('infostand.button.ignore') } } - { avatarInfo.isIgnored && + { isIgnored && processAction('unignore') }> { LocalizeText('infostand.button.unignore') } } diff --git a/src/hooks/rooms/widgets/useChatWidget.ts b/src/hooks/rooms/widgets/useChatWidget.ts index f278659..e05fd36 100644 --- a/src/hooks/rooms/widgets/useChatWidget.ts +++ b/src/hooks/rooms/widgets/useChatWidget.ts @@ -2,6 +2,7 @@ import { GetGuestRoomResultEvent, GetRoomEngine, GetSessionDataManager, PetFigur import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; import { ChatBubbleMessage, ChatBubbleUtilities, ChatEntryType, ChatHistoryCurrentDate, GetConfigurationValue, GetRoomObjectScreenLocation, IRoomChatSettings, LocalizeText, PlaySound, RoomChatFormatter } from '../../../api'; import { useMessageEvent, useNitroEvent } from '../../events'; +import { useUserDataSnapshot } from '../../session/useSessionSnapshots'; import { useTranslation } from '../../translation'; import { useRoom } from '../useRoom'; import { useChatHistory } from './../../chat-history'; @@ -22,7 +23,12 @@ const useChatWidgetState = () => const { addChatEntry, updateChatEntry } = useChatHistory(); const { settings, translateIncoming, consumeOutgoingTranslation } = useTranslation(); const isDisposed = useRef(false); - const ownUserId = (GetSessionDataManager()?.userId || -1); + // Reactive: re-renders if the session-data snapshot flips (e.g. + // reconnect under a different user id). Safe to call here — + // useChatWidget is NOT wrapped in useBetween (see export below), + // so the real React dispatcher is in scope and + // useSyncExternalStore installs correctly. + const ownUserId = (useUserDataSnapshot().userId || -1); const applyTranslationToBubble = useCallback((chatMessage: ChatBubbleMessage, originalText: string, translatedText: string, detectedLanguage: string, targetLanguage: string) => { diff --git a/src/hooks/session/useSessionInfo.ts b/src/hooks/session/useSessionInfo.ts index 9959698..df59dc2 100644 --- a/src/hooks/session/useSessionInfo.ts +++ b/src/hooks/session/useSessionInfo.ts @@ -1,16 +1,21 @@ -import { FigureUpdateEvent, GetSessionDataManager, RoomUnitChatStyleComposer, UserInfoDataParser, UserInfoEvent, UserSettingsEvent } from '@nitrots/nitro-renderer'; +import { GetSessionDataManager, RoomUnitChatStyleComposer, UserInfoDataParser, UserInfoEvent, UserSettingsEvent } from '@nitrots/nitro-renderer'; import { useState } from 'react'; import { useBetween } from 'use-between'; import { SendMessageComposer } from '../../api'; import { useMessageEvent } from '../events'; +import { useUserDataSnapshot } from './useSessionSnapshots'; +// State function — ONLY use-between-safe hooks here (useState, +// useMessageEvent, plain actions). Do NOT call snapshot hooks here: +// use-between's dispatcher does not implement useSyncExternalStore, so +// any `useUserDataSnapshot()` / `useExternalSnapshot()` call inside +// this body crashes the React tree on first paint with +// "(intermediate value)() is undefined". See useSessionSnapshots.test.tsx +// for the regression guard. const useSessionInfoState = () => { const [ userInfo, setUserInfo ] = useState(null); - const [ userFigure, setUserFigure ] = useState(null); const [ chatStyleId, setChatStyleId ] = useState(0); - const [ userRespectRemaining, setUserRespectRemaining ] = useState(0); - const [ petRespectRemaining, setPetRespectRemaining ] = useState(0); const updateChatStyleId = (styleId: number) => { @@ -19,45 +24,38 @@ const useSessionInfoState = () => SendMessageComposer(new RoomUnitChatStyleComposer(styleId)); }; - const respectUser = (userId: number) => - { - GetSessionDataManager().giveRespect(userId); - - setUserRespectRemaining(GetSessionDataManager().respectsLeft); - }; - - const respectPet = (petId: number) => - { - GetSessionDataManager().givePetRespect(petId); - - setPetRespectRemaining(GetSessionDataManager().respectsPetLeft); - }; + const respectUser = (userId: number) => GetSessionDataManager().giveRespect(userId); + const respectPet = (petId: number) => GetSessionDataManager().givePetRespect(petId); useMessageEvent(UserInfoEvent, event => { - const parser = event.getParser(); - - setUserInfo(parser.userInfo); - setUserFigure(parser.userInfo.figure); - setUserRespectRemaining(parser.userInfo.respectsRemaining); - setPetRespectRemaining(parser.userInfo.respectsPetRemaining); - }); - - useMessageEvent(FigureUpdateEvent, event => - { - const parser = event.getParser(); - - setUserFigure(parser.figure); + setUserInfo(event.getParser().userInfo); }); useMessageEvent(UserSettingsEvent, event => { - const parser = event.getParser(); - - setChatStyleId(parser.chatType); + setChatStyleId(event.getParser().chatType); }); - return { userInfo, userFigure, chatStyleId, userRespectRemaining, petRespectRemaining, respectUser, respectPet, updateChatStyleId }; + return { userInfo, chatStyleId, respectUser, respectPet, updateChatStyleId }; }; -export const useSessionInfo = () => useBetween(useSessionInfoState); +// Public surface — snapshot reads happen in the OUTER wrapper, in the +// real React dispatcher's scope, so useSyncExternalStore installs +// correctly. useBetween only proxies the non-snapshot slice, where its +// dispatcher works fine. SessionDataManager already invalidates the +// snapshot on UserInfoEvent / FigureUpdateEvent / giveRespect / +// givePetRespect, so userFigure / respectsLeft / respectsPetLeft stay +// in sync without local useState mirrors. +export const useSessionInfo = () => +{ + const shared = useBetween(useSessionInfoState); + const userData = useUserDataSnapshot(); + + return { + ...shared, + userFigure: userData.figure, + userRespectRemaining: userData.respectsLeft, + petRespectRemaining: userData.respectsPetLeft + }; +}; diff --git a/src/hooks/session/useSessionSnapshots.test.tsx b/src/hooks/session/useSessionSnapshots.test.tsx new file mode 100644 index 0000000..c4c0acc --- /dev/null +++ b/src/hooks/session/useSessionSnapshots.test.tsx @@ -0,0 +1,101 @@ +/* @vitest-environment jsdom */ + +import { cleanup, render, renderHook } from '@testing-library/react'; +import { Component, ReactNode, useSyncExternalStore } from 'react'; +import { useBetween } from 'use-between'; +import { afterEach, describe, expect, it, vi } from 'vitest'; + +// Regression guard for the rolled-back snapshot-consumer migration. +// +// `use-between` (v1.x) ships its own dispatcher that proxies a subset of +// React hooks (useState, useReducer, useEffect, useLayoutEffect, +// useCallback, useMemo, useRef, useImperativeHandle). It does NOT +// implement `useSyncExternalStore`. When a state function runs inside +// `useBetween(stateFn)` and that state function calls +// `useSyncExternalStore` (directly or via a wrapper like +// `useExternalSnapshot` / `useUserDataSnapshot`), React resolves the +// dispatcher to use-between's proxy, finds `useSyncExternalStore` +// missing, and throws "(intermediate value)() is undefined" on the +// first render — that's the exact production error reported at +// ToolbarView.tsx:46 last session. +// +// The fix is structural: snapshot hooks must run OUTSIDE the useBetween +// scope (i.e. in the exported wrapper, not in the inner state +// function). These tests pin the constraint so a future migration +// doesn't reintroduce the broken pattern. + +class CaptureBoundary extends Component<{ children: ReactNode }, { error: Error | null }> +{ + state = { error: null as Error | null }; + + static getDerivedStateFromError(error: Error) + { + return { error }; + } + + componentDidCatch() + { + } + + render() + { + return this.state.error ? null : this.props.children; + } +} + +describe('use-between + useSyncExternalStore incompatibility', () => +{ + afterEach(() => + { + cleanup(); + }); + + it('crashes when useSyncExternalStore is called inside a useBetween scope', () => + { + // React 19 logs every render-time error to console.error before + // forwarding to the error boundary. Suppress the noise to keep + // the test output readable, then assert the error fingerprint. + const consoleError = vi.spyOn(console, 'error').mockImplementation(() => undefined); + + const Broken = () => + { + useBetween(() => useSyncExternalStore(() => () => undefined, () => 'v', () => 'v')); + return null; + }; + + let captured: Error | null = null; + const boundaryRef = (instance: CaptureBoundary | null) => + { + if(instance) captured = instance.state.error; + }; + + render( + + + + ); + + expect(captured).not.toBeNull(); + expect(captured!.message).toMatch(/useSyncExternalStore is not a function|intermediate value/); + + consoleError.mockRestore(); + }); + + it('works when useSyncExternalStore is called OUTSIDE the useBetween scope', () => + { + const sharedState = () => ({ count: 0 }); + + const safeHook = () => + { + const shared = useBetween(sharedState); + const external = useSyncExternalStore(() => () => undefined, () => 'value', () => 'value'); + + return { ...shared, external }; + }; + + const { result } = renderHook(() => safeHook()); + + expect(result.current.external).toBe('value'); + expect(result.current.count).toBe(0); + }); +});