From 7a3ed5eaf7b439818b7bb5023e06a243c3b4b77d Mon Sep 17 00:00:00 2001 From: Katherine Jensen Date: Fri, 24 Jul 2026 10:28:13 -0700 Subject: [PATCH 1/3] PT-4179: stub renderer PAPI hooks + RPC handler in Storybook Renderer app components (the startup-wizard shell/steps, plus dialogs and overlays) import `useLocalizedStrings` from `@renderer/hooks/papi-hooks` and otherwise reach the network service. Storybook has no PAPI backend, so the renderer `RpcClient`'s connection attempt to `ws://localhost:` never settles and its `AsyncVariable('websocket connected')` rejects unhandled after ~10s ("Timeout reached when waiting for websocket connected to settle"), which the dev overlay surfaces as a crash on every startup-wizard story. Storybook previously only stubbed `@papi/*` imports, never the `@renderer`/`@shared` paths these renderer components actually use. Add two Storybook-only module replacements: - `papi-stubs/renderer-papi-hooks.ts`: re-exports the real hooks but overrides `useLocalizedStrings` to resolve real English strings synchronously (no connection). - `papi-stubs/rpc-handler.factory.ts`: inert RPC handler so `networkService.initialize()` succeeds with no socket and no timer, so no connection is ever attempted and nothing rejects. Both are wired via `NormalModuleReplacementPlugin`, not `resolve.alias`: the base renderer webpack config resolves `@renderer`/`@shared` via `TsconfigPathsPlugin`, which wins over `resolve.alias` (that is why the existing `@papi/*` aliases work but a `@renderer/...` alias would be ignored). Verified with `npm run storybook:build` and by rendering the first-run stories (real English strings, zero unhandled rejections). Co-Authored-By: Claude Opus 4.8 (1M context) --- .storybook/main.ts | 34 ++++++++++++- .storybook/papi-stubs/renderer-papi-hooks.ts | 51 ++++++++++++++++++++ .storybook/papi-stubs/rpc-handler.factory.ts | 46 ++++++++++++++++++ 3 files changed, 130 insertions(+), 1 deletion(-) create mode 100644 .storybook/papi-stubs/renderer-papi-hooks.ts create mode 100644 .storybook/papi-stubs/rpc-handler.factory.ts diff --git a/.storybook/main.ts b/.storybook/main.ts index 480536e7438..84aaaba71fe 100644 --- a/.storybook/main.ts +++ b/.storybook/main.ts @@ -1,7 +1,7 @@ import { dirname, join } from 'path'; import type { StorybookConfig } from '@storybook/react-webpack5'; import { mergeWithCustomize } from 'webpack-merge'; -import { RuleSetRule } from 'webpack'; +import { NormalModuleReplacementPlugin, RuleSetRule } from 'webpack'; const config: StorybookConfig = { stories: [ @@ -165,6 +165,38 @@ const config: StorybookConfig = { }; } + // Renderer app components (startup-wizard shell/steps, dialogs, overlays) import + // `useLocalizedStrings` from the `@renderer/hooks/papi-hooks` barrel. The real hook opens a PAPI + // WebSocket that has no backend in Storybook, rejecting unhandled after ~10s ("Timeout reached + // when waiting for websocket connected to settle") and crashing every startup-wizard story. The + // stub re-exports the real hooks and overrides only `useLocalizedStrings` to resolve strings + // without a connection. + // + // This MUST use NormalModuleReplacementPlugin, not `resolve.alias`: the base renderer webpack + // config resolves `@renderer/*` via `TsconfigPathsPlugin` (webpack.config.base.ts), which wins + // over `resolve.alias`, so an alias entry is silently ignored (that is why the `@papi/*` aliases + // above work — `@papi` is not a tsconfig path — but a `@renderer/...` alias would not). The + // replacement rewrites the request in `beforeResolve`, before TsconfigPaths runs. The `$`-anchored + // regex matches only the exact barrel, so deep-path `@renderer/hooks/papi-hooks/*` imports (used + // by the stub itself to re-export the real hooks) still resolve normally. + webpackConfig.plugins = webpackConfig.plugins ?? []; + webpackConfig.plugins.push( + new NormalModuleReplacementPlugin( + /^@renderer\/hooks\/papi-hooks$/, + join(__dirname, 'papi-stubs/renderer-papi-hooks.ts'), + ), + // Stop `networkService.initialize()` from constructing a real renderer RpcClient, which tries + // to open a PAPI WebSocket that has no backend in Storybook and rejects unhandled after ~10s + // ("Timeout reached when waiting for websocket connected to settle"), crashing renderer + // stories via the dev overlay. The inert handler makes initialize() succeed with no socket, so + // no connection is attempted. Same reasoning as above re: replacement plugin vs `resolve.alias` + // (`@shared/*` is a TsconfigPathsPlugin path). + new NormalModuleReplacementPlugin( + /^@shared\/services\/rpc-handler\.factory$/, + join(__dirname, 'papi-stubs/rpc-handler.factory.ts'), + ), + ); + // Remove the Storybook Webpack rules that we already have our own rules for return mergeWithCustomize({ customizeObject(wpConfig: object, rConfig: object, key: string) { diff --git a/.storybook/papi-stubs/renderer-papi-hooks.ts b/.storybook/papi-stubs/renderer-papi-hooks.ts new file mode 100644 index 00000000000..77eef449b18 --- /dev/null +++ b/.storybook/papi-stubs/renderer-papi-hooks.ts @@ -0,0 +1,51 @@ +/** + * Storybook stub for the renderer PAPI-hooks barrel (`@renderer/hooks/papi-hooks`). + * + * Renderer app components (e.g. the first-run / startup-wizard shell and steps) import + * `useLocalizedStrings` from this barrel. The real hook subscribes to the localization data + * provider, which calls `networkService.initialize()` → opens a PAPI WebSocket. There is no PAPI + * backend in Storybook, so that connection never settles and rejects unhandled after ~10 s with + * "Timeout reached when waiting for websocket connected to settle", surfacing as the webpack error + * overlay on every startup-wizard story. + * + * This stub is aliased in over the barrel (see `.storybook/main.ts`). It re-exports every real hook + * unchanged and overrides ONLY `useLocalizedStrings` to resolve strings synchronously from the + * Storybook localization helper (real English text, no network). Other hooks stay real: they are + * inert unless actually called, so aliasing the barrel does not change any story that does not use + * them. This mirrors the `useLocalizedStrings` override already in `papi-stubs/frontend-react.ts` + * for the `@papi/frontend/react` web-view barrel. + */ + +import type { LocalizeKey } from 'platform-bible-utils'; +import type { DataProviderSubscriberOptions } from '@shared/models/data-provider.model'; +import type { LocalizationData } from '@shared/services/localization.service-model'; +import { getLocalizedStrings } from '../localization.utils'; + +// Re-export every real hook except useLocalizedStrings (overridden below). These remain the genuine +// implementations; they only touch the network when invoked, which startup-wizard stories do not. +export { default as useDataProvider } from '@renderer/hooks/papi-hooks/use-data-provider.hook'; +export { default as useData } from '@renderer/hooks/papi-hooks/use-data.hook'; +export { default as useScrollGroupScrRef } from '@renderer/hooks/papi-hooks/use-scroll-group-scr-ref.hook'; +export { default as useSetting } from '@renderer/hooks/papi-hooks/use-setting.hook'; +export { default as useProjectData } from '@renderer/hooks/papi-hooks/use-project-data.hook'; +export { default as useProjectDataProvider } from '@renderer/hooks/papi-hooks/use-project-data-provider.hook'; +export { default as useProjectSetting } from '@renderer/hooks/papi-hooks/use-project-setting.hook'; +export { default as useDialogCallback } from '@renderer/hooks/papi-hooks/use-dialog-callback.hook'; +export { default as useDataProviderMulti } from '@renderer/hooks/papi-hooks/use-data-provider-multi.hook'; +export { default as useWebViewController } from '@renderer/hooks/papi-hooks/use-web-view-controller.hook'; +export { default as useRecentScriptureRefs } from '@renderer/hooks/papi-hooks/use-recent-scripture-refs.hook'; + +/** + * Storybook override of the real `useLocalizedStrings`. Resolves the requested keys to English + * strings synchronously (never loading, never networked). Signature matches the real hook so + * consumers destructure `[localizedStrings, isLoading]` exactly as they do at runtime. + */ +export function useLocalizedStrings( + localizationKeys: LocalizeKey[], + // eslint-disable-next-line @typescript-eslint/no-unused-vars -- part of the real hook signature + localizationLocales?: string[], + // eslint-disable-next-line @typescript-eslint/no-unused-vars -- part of the real hook signature + subscriberOptions?: DataProviderSubscriberOptions, +): [localizedStrings: LocalizationData, isLoading: boolean] { + return [getLocalizedStrings(localizationKeys ?? []), false]; +} diff --git a/.storybook/papi-stubs/rpc-handler.factory.ts b/.storybook/papi-stubs/rpc-handler.factory.ts new file mode 100644 index 00000000000..55a6e4ffe8b --- /dev/null +++ b/.storybook/papi-stubs/rpc-handler.factory.ts @@ -0,0 +1,46 @@ +/** + * Storybook stub for `@shared/services/rpc-handler.factory`. + * + * In the app, `networkService.initialize()` calls `createRpcHandler()`, which (in the renderer) + * constructs an `RpcClient`. That client immediately arms a 10s `AsyncVariable('websocket + * connected')` and tries to open `ws://localhost:`. Storybook has no PAPI backend, so the + * socket never opens and the AsyncVariable rejects unhandled with "Timeout reached when waiting for + * websocket connected to settle" — which the webpack/react-refresh dev overlay surfaces as a crash + * on any story whose component tree touches the network service (the whole startup-wizard set, plus + * dialogs/overlays). + * + * This inert handler makes `initialize()` succeed instantly with no socket and no timer, so no + * connection is ever attempted and nothing rejects. Requests resolve to a JSON-RPC error (there is + * no backend to answer them); event registration/emission are no-ops. Data hooks that reach the + * network therefore render their loading/offline state instead of hanging — which is the correct + * Storybook behavior. Wired via NormalModuleReplacementPlugin in `.storybook/main.ts` (not + * `resolve.alias`, which `TsconfigPathsPlugin` overrides for `@shared/*`). + */ + +import { ConnectionStatus } from '@shared/data/rpc.model'; +import type { EventHandler, RequestParams } from '@shared/data/rpc.model'; +import type { IRpcMethodRegistrar } from '@shared/models/rpc.interface'; +import type { SerializedRequestType } from '@shared/utils/util'; +import type { JSONRPCResponse } from 'json-rpc-2.0'; + +export const createRpcHandler = async (): Promise => ({ + connectionStatus: ConnectionStatus.Connected, + connect: async (_localEventHandler: EventHandler) => true, + disconnect: async () => {}, + request: async ( + requestType: SerializedRequestType, + _requestParams: RequestParams, + ): Promise => ({ + jsonrpc: '2.0', + // json-rpc requires an id; 0 is fine for these unanswerable Storybook requests. + id: 0, + error: { code: -32601, message: `Storybook: no PAPI backend to handle "${requestType}"` }, + }), + emitEventOnNetwork: () => {}, + registerMethod: async () => true, + unregisterMethod: async () => true, + registerEvent: async () => true, + unregisterEvent: async () => true, +}); + +export default createRpcHandler; From ea1a4f30a2ee72c964a64bcf0f78675c062e8e58 Mon Sep 17 00:00:00 2001 From: Katherine Jensen Date: Fri, 24 Jul 2026 11:41:20 -0700 Subject: [PATCH 2/3] PT-4179: fix leaked Sonner timer in notification-display tests Sonner's auto-dismiss timer fires after jsdom tears down, causing a "window is not defined" unhandled error that fails the test run even though all 1191 tests pass. Add an afterEach that calls toast.dismiss() (wrapped in act) to cancel all pending Sonner timers before the test environment is torn down. Co-Authored-By: Claude Opus 4.8 --- .../components/notification-display.test.tsx | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/renderer/components/notification-display.test.tsx b/src/renderer/components/notification-display.test.tsx index 8bcf852a832..eab4a45ae69 100644 --- a/src/renderer/components/notification-display.test.tsx +++ b/src/renderer/components/notification-display.test.tsx @@ -1,5 +1,6 @@ -import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { render, screen, fireEvent, act } from '@testing-library/react'; +import { toast } from 'sonner'; import '@testing-library/jest-dom'; import type { CommandHandlers } from 'papi-shared-types'; import type { ThemeDefinitionExpanded } from 'platform-bible-utils'; @@ -95,6 +96,15 @@ describe('NotificationDisplay with real Sonner', () => { await startNotificationService(); }); + afterEach(() => { + // Sonner auto-dismiss timers fire after jsdom tears down if toasts are still alive, + // causing a "window is not defined" unhandled error. Dismissing all toasts here + // triggers Sonner's internal clearTimeout calls before the environment is torn down. + act(() => { + toast.dismiss(); + }); + }); + it('sends the secondary command when the user clicks the rendered cancel-slot button', async () => { render(); From 43a5fc5ded062a6232fb3a871507c05565e3a38c Mon Sep 17 00:00:00 2001 From: Katherine Jensen Date: Sat, 25 Jul 2026 15:53:10 -0700 Subject: [PATCH 3/3] PT-4179: fix lint errors in papi-stubs (require-disable-comment, no-unused-vars) Co-Authored-By: Claude Sonnet 4.6 --- .storybook/papi-stubs/renderer-papi-hooks.ts | 6 ++++-- .storybook/papi-stubs/rpc-handler.factory.ts | 8 ++------ 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/.storybook/papi-stubs/renderer-papi-hooks.ts b/.storybook/papi-stubs/renderer-papi-hooks.ts index 77eef449b18..456ca1d0572 100644 --- a/.storybook/papi-stubs/renderer-papi-hooks.ts +++ b/.storybook/papi-stubs/renderer-papi-hooks.ts @@ -42,9 +42,11 @@ export { default as useRecentScriptureRefs } from '@renderer/hooks/papi-hooks/us */ export function useLocalizedStrings( localizationKeys: LocalizeKey[], - // eslint-disable-next-line @typescript-eslint/no-unused-vars -- part of the real hook signature + // Unused in this stub; required to match the real hook's signature for drop-in compatibility. + // eslint-disable-next-line @typescript-eslint/no-unused-vars localizationLocales?: string[], - // eslint-disable-next-line @typescript-eslint/no-unused-vars -- part of the real hook signature + // Unused in this stub; required to match the real hook's signature for drop-in compatibility. + // eslint-disable-next-line @typescript-eslint/no-unused-vars subscriberOptions?: DataProviderSubscriberOptions, ): [localizedStrings: LocalizationData, isLoading: boolean] { return [getLocalizedStrings(localizationKeys ?? []), false]; diff --git a/.storybook/papi-stubs/rpc-handler.factory.ts b/.storybook/papi-stubs/rpc-handler.factory.ts index 55a6e4ffe8b..20ba4c92bf9 100644 --- a/.storybook/papi-stubs/rpc-handler.factory.ts +++ b/.storybook/papi-stubs/rpc-handler.factory.ts @@ -18,19 +18,15 @@ */ import { ConnectionStatus } from '@shared/data/rpc.model'; -import type { EventHandler, RequestParams } from '@shared/data/rpc.model'; import type { IRpcMethodRegistrar } from '@shared/models/rpc.interface'; import type { SerializedRequestType } from '@shared/utils/util'; import type { JSONRPCResponse } from 'json-rpc-2.0'; export const createRpcHandler = async (): Promise => ({ connectionStatus: ConnectionStatus.Connected, - connect: async (_localEventHandler: EventHandler) => true, + connect: async () => true, disconnect: async () => {}, - request: async ( - requestType: SerializedRequestType, - _requestParams: RequestParams, - ): Promise => ({ + request: async (requestType: SerializedRequestType): Promise => ({ jsonrpc: '2.0', // json-rpc requires an id; 0 is fine for these unanswerable Storybook requests. id: 0,