From 8f8514c944a90fd266eb819d22de11b1ccaf82f5 Mon Sep 17 00:00:00 2001 From: Charles Rossi Date: Mon, 15 Jun 2026 12:04:55 -0300 Subject: [PATCH] fix(react): keep PathnameProvider mounted when userId changes Previously FlowsProvider returned props.children directly (outside PathnameProvider) when userId was null. When userId later changed to a string, PathnameProvider was inserted into the tree for the first time, causing React to throw "Rendered more hooks than during the previous render" because the component subtree structure changed under the same parent fiber. Fix: always render PathnameProvider so it is stable in the tree; conditionally render FlowsProviderInner vs raw children inside it. Closes #384 --- pnpm-lock.yaml | 6 + workspaces/react/package.json | 2 + workspaces/react/src/flows-provider.test.tsx | 157 +++++++++++++++++++ workspaces/react/src/flows-provider.tsx | 4 +- workspaces/react/tsconfig.json | 3 +- 5 files changed, 168 insertions(+), 4 deletions(-) create mode 100644 workspaces/react/src/flows-provider.test.tsx diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 6b5469d6..e56de619 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -212,12 +212,18 @@ importers: '@types/react': specifier: ^19.2.14 version: 19.2.14 + '@types/react-dom': + specifier: ^19.2.3 + version: 19.2.3(@types/react@19.2.14) jest: specifier: ^30.4.2 version: 30.4.2(@types/node@24.12.0) jest-environment-jsdom: specifier: ^30.4.1 version: 30.4.1 + react-dom: + specifier: 19.2.6 + version: 19.2.6(react@19.2.6) ts-jest: specifier: ^29.4.11 version: 29.4.11(@babel/core@7.28.5)(@jest/transform@30.4.1)(@jest/types@30.4.1)(babel-jest@30.4.1(@babel/core@7.28.5))(esbuild@0.28.1)(jest-util@30.4.1)(jest@30.4.2(@types/node@24.12.0))(typescript@6.0.3) diff --git a/workspaces/react/package.json b/workspaces/react/package.json index 3093665c..2eff9e71 100644 --- a/workspaces/react/package.json +++ b/workspaces/react/package.json @@ -40,8 +40,10 @@ "@types/jest": "^30.0.0", "@types/node": "^24.12.0", "@types/react": "^19.2.14", + "@types/react-dom": "^19.2.3", "jest": "^30.4.2", "jest-environment-jsdom": "^30.4.1", + "react-dom": "19.2.6", "ts-jest": "^29.4.11", "tsup": "^8.5.1", "typescript": "^6.0.3" diff --git a/workspaces/react/src/flows-provider.test.tsx b/workspaces/react/src/flows-provider.test.tsx new file mode 100644 index 00000000..af8c506b --- /dev/null +++ b/workspaces/react/src/flows-provider.test.tsx @@ -0,0 +1,157 @@ +/** + * Regression test for https://github.com/RBND-studio/flows-sdk/issues/384 + * FlowsProvider threw "Rendered more hooks than during the previous render" + * when userId changed from null to a string because PathnameProvider was + * conditionally included in the tree. + */ + +import React, { act, type FC, type ReactNode, useState } from "react"; +import { createRoot, type Root } from "react-dom/client"; +import { FlowsProvider } from "./flows-provider"; + +// Required for React 19 concurrent act() in non-browser environments (jsdom). +// See https://react.dev/blog/2022/03/08/react-18-upgrade-guide#configuring-your-testing-environment +(globalThis as Record).IS_REACT_ACT_ENVIRONMENT = true; + +// --------------------------------------------------------------------------- +// Minimal stubs so the provider can mount in jsdom without real network calls +// --------------------------------------------------------------------------- + +// Stub WebSocket so useWebsocket does not attempt real connections. +class StubWebSocket { + static CONNECTING = 0; + static OPEN = 1; + static CLOSING = 2; + static CLOSED = 3; + readyState = StubWebSocket.CONNECTING; + addEventListener() {} + removeEventListener() {} + close() {} +} + +// Stub getApi so useBlocks never fires real HTTP requests. +jest.mock("@flows/shared", () => { + const actual = jest.requireActual>("@flows/shared"); + return { + ...actual, + getApi: () => ({ + getBlocks: () => new Promise(() => {}), // never resolves + sendEvent: () => Promise.resolve(), + }), + getSessionStorageRunningSurveys: () => [], + saveSessionStorageRunningSurveys: () => {}, + }; +}); + +// --------------------------------------------------------------------------- +// Minimal component props for FlowsProvider +// --------------------------------------------------------------------------- +const COMMON_PROPS = { + organizationId: "org-test", + environment: "test", + components: {}, + tourComponents: {}, + surveyComponents: {}, +} as const; + +// --------------------------------------------------------------------------- +// Helper: a wrapper component that can toggle userId between null and string +// --------------------------------------------------------------------------- +let setUserIdExternal: ((id: string | null) => void) | null = null; + +const Wrapper: FC<{ children?: ReactNode }> = ({ children }) => { + const [userId, setUserId] = useState(null); + setUserIdExternal = setUserId; + + return ( + + {children ??
child
} + + ); +}; + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- +describe("FlowsProvider – userId change", () => { + let container: HTMLDivElement; + let root: Root; + const originalWebSocket = (globalThis as Record).WebSocket; + + beforeAll(() => { + (globalThis as Record).WebSocket = StubWebSocket; + }); + + afterAll(() => { + (globalThis as Record).WebSocket = originalWebSocket; + }); + + beforeEach(() => { + container = document.createElement("div"); + document.body.appendChild(container); + root = createRoot(container); + }); + + afterEach(() => { + act(() => { + root.unmount(); + }); + container.remove(); + setUserIdExternal = null; + }); + + it("does not throw when userId changes from null to a string", () => { + // Mount with userId=null (SDK disabled state) + act(() => { + root.render(); + }); + + // Confirm the child is rendered even while userId is null + expect(container.querySelector("[data-testid=child]")).not.toBeNull(); + + // Change userId from null → "user-1" — this previously threw: + // "Rendered more hooks than during the previous render." + expect(() => { + act(() => { + setUserIdExternal?.("user-1"); + }); + }).not.toThrow(); + + // Child should still be rendered after the transition + expect(container.querySelector("[data-testid=child]")).not.toBeNull(); + }); + + it("does not throw when userId changes from a string back to null", () => { + act(() => { + root.render(); + }); + + act(() => { + setUserIdExternal?.("user-1"); + }); + + expect(() => { + act(() => { + setUserIdExternal?.(null); + }); + }).not.toThrow(); + + expect(container.querySelector("[data-testid=child]")).not.toBeNull(); + }); + + it("does not throw on rapid null → string → null → string transitions", () => { + act(() => { + root.render(); + }); + + expect(() => { + act(() => { + setUserIdExternal?.("user-1"); + setUserIdExternal?.(null); + setUserIdExternal?.("user-2"); + }); + }).not.toThrow(); + + expect(container.querySelector("[data-testid=child]")).not.toBeNull(); + }); +}); diff --git a/workspaces/react/src/flows-provider.tsx b/workspaces/react/src/flows-provider.tsx index da193c39..932f0388 100644 --- a/workspaces/react/src/flows-provider.tsx +++ b/workspaces/react/src/flows-provider.tsx @@ -127,11 +127,9 @@ export interface FlowsProviderProps { } export const FlowsProvider: FC = (props) => { - if (!isProps(props)) return props.children; - return ( - + {isProps(props) ? : props.children} ); }; diff --git a/workspaces/react/tsconfig.json b/workspaces/react/tsconfig.json index 675639b1..d0720ea7 100644 --- a/workspaces/react/tsconfig.json +++ b/workspaces/react/tsconfig.json @@ -1,7 +1,8 @@ { "extends": "../../tsconfig.base.json", "compilerOptions": { - "jsx": "react-jsx" + "jsx": "react-jsx", + "types": ["jest"] }, "include": ["src/**/*"] }