From a20c78744ac772f91beaae04471086695dbc8deb Mon Sep 17 00:00:00 2001 From: chungyo Date: Tue, 1 Sep 2026 14:45:24 +0900 Subject: [PATCH] fix(browser): preserve funnel history in StrictMode React StrictMode replays effects in development, causing cleanup to remove the persisted step, context, and histories after a reload. Restore the snapshot only when the same mounted funnel reconnects, while keeping cleanup synchronous for real unmounts. Refs #170 --- .changeset/quiet-funnels-wait.md | 5 + .../e2e/app-router-funnel.spec.ts | 18 +++ packages/browser/src/index.ts | 62 ++++++++-- packages/browser/test/index.test.tsx | 112 ++++++++++++++++++ 4 files changed, 185 insertions(+), 12 deletions(-) create mode 100644 .changeset/quiet-funnels-wait.md diff --git a/.changeset/quiet-funnels-wait.md b/.changeset/quiet-funnels-wait.md new file mode 100644 index 0000000..80aaeca --- /dev/null +++ b/.changeset/quiet-funnels-wait.md @@ -0,0 +1,5 @@ +--- +'@use-funnel/browser': patch +--- + +Preserve persisted funnel history when React Strict Mode replays effects. diff --git a/examples/nextjs-app-router/e2e/app-router-funnel.spec.ts b/examples/nextjs-app-router/e2e/app-router-funnel.spec.ts index b75670d..2f30022 100644 --- a/examples/nextjs-app-router/e2e/app-router-funnel.spec.ts +++ b/examples/nextjs-app-router/e2e/app-router-funnel.spec.ts @@ -25,6 +25,24 @@ test('can move the steps of the funnel using history.push.', async ({ page }) => await expect(page.getByText('Sub End')).toBeVisible(); }); +test('preserves the current step across reloads in StrictMode.', async ({ page }) => { + await page.goto(`${APP_ROUTER_LOCAL_URL}funnel`); + + await page.getByRole('button', { name: 'next', exact: true }).click(); + await page.getByRole('button', { name: 'next', exact: true }).click(); + + await expect(page.getByText('MAIN - END')).toBeVisible(); + + await page.reload(); + + await expect(page.getByText('MAIN - END')).toBeVisible(); + await expect(page).toHaveURL(/test-app-router-funnel\.step=end/); + + await page.reload(); + + await expect(page.getByText('MAIN - END')).toBeVisible(); +}); + test('can move the steps of the funnel using Link.', async ({ page }) => { await page.goto(APP_ROUTER_LOCAL_URL); diff --git a/packages/browser/src/index.ts b/packages/browser/src/index.ts index 0f896ca..c57895d 100644 --- a/packages/browser/src/index.ts +++ b/packages/browser/src/index.ts @@ -1,11 +1,12 @@ 'use client'; import { AnyFunnelState, createUseFunnel } from '@use-funnel/core'; -import { useCallback, useEffect, useMemo, useState } from 'react'; +import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; export * from '@use-funnel/core'; export const useFunnel = createUseFunnel(({ id, initialState }) => { + const restoreCleanupRef = useRef<(() => void) | undefined>(undefined); const [location, setLocation] = useState(() => ({ search: typeof window !== 'undefined' ? window.location.search : '', })); @@ -14,6 +15,11 @@ export const useFunnel = createUseFunnel(({ id, initialState }) => { })); useEffect(() => { + // Restore a cleanup from React Strict Mode's setup-cleanup-setup replay. + const restoreCleanup = restoreCleanupRef.current; + restoreCleanupRef.current = undefined; + restoreCleanup?.(); + function handlePopState(event: PopStateEvent) { setLocation(window.location); setState(event.state); @@ -80,25 +86,57 @@ export const useFunnel = createUseFunnel(({ id, initialState }) => { window.history.go(index); }, cleanup() { + restoreCleanupRef.current = undefined; + const newHistoryState = { ...window.history.state, }; - const searchParams = new URLSearchParams(window.location.search); + const originalHref = window.location.href; + const cleanupUrl = new URL(originalHref); + const contextName = `${id}.context`; + const historiesName = `${id}.histories`; + const stepName = `${id}.step`; + const currentContext = newHistoryState[contextName]; + const currentHistories = newHistoryState[historiesName]; + const currentStep = cleanupUrl.searchParams.get(stepName); - if ( - newHistoryState[`${id}.context`] == null || - newHistoryState[`${id}.histories`] == null || - searchParams.get(`${id}.step`) == null - ) { + if (currentContext == null || currentHistories == null || currentStep == null) { return; } - delete newHistoryState[`${id}.context`]; - delete newHistoryState[`${id}.histories`]; - - searchParams.delete(`${id}.step`); - window.history.replaceState(newHistoryState, '', `?${searchParams.toString()}`); + delete newHistoryState[contextName]; + delete newHistoryState[historiesName]; + + cleanupUrl.searchParams.delete(stepName); + window.history.replaceState(newHistoryState, '', cleanupUrl); + + const cleanedHref = window.location.href; + const cleanedHistoryState = window.history.state; + + restoreCleanupRef.current = () => { + // Do not restore stale funnel state over another navigation. + if (window.location.href !== cleanedHref || window.history.state !== cleanedHistoryState) { + return; + } + + const currentHistoryState = { + ...window.history.state, + }; + const currentSearchParams = new URLSearchParams(window.location.search); + + if ( + contextName in currentHistoryState || + historiesName in currentHistoryState || + currentSearchParams.has(stepName) + ) { + return; + } + + currentHistoryState[contextName] = currentContext; + currentHistoryState[historiesName] = currentHistories; + window.history.replaceState(currentHistoryState, '', originalHref); + }; }, }), [id, history, currentIndex, currentState, changeState], diff --git a/packages/browser/test/index.test.tsx b/packages/browser/test/index.test.tsx index 3800d97..25d7fc6 100644 --- a/packages/browser/test/index.test.tsx +++ b/packages/browser/test/index.test.tsx @@ -1,8 +1,45 @@ import { cleanup, render, screen, waitForElementToBeRemoved } from '@testing-library/react'; import { userEvent } from '@testing-library/user-event'; +import { StrictMode, useEffect } from 'react'; import { afterEach, describe, expect, test } from 'vitest'; import { useFunnel } from '../src/index.js'; +function setPersistedHistory(id: string) { + const context = { id: 'persisted' }; + const histories = [ + { step: 'A', context: {} }, + { step: 'B', context }, + ]; + + window.history.replaceState( + { + ...window.history.state, + unrelated: 'preserved', + [`${id}.context`]: context, + [`${id}.histories`]: histories, + }, + '', + `?${id}.step=B&unrelated=preserved#summary`, + ); + + return { context, histories }; +} + +function PersistedFunnel({ id }: { id: string }) { + const funnel = useFunnel<{ + A: { id?: string }; + B: { id: string }; + }>({ + id, + initial: { + step: 'A', + context: {}, + }, + }); + + return
{funnel.step}
; +} + describe('Test useFunnel browser router', () => { afterEach(cleanup); @@ -51,6 +88,81 @@ describe('Test useFunnel browser router', () => { expect(screen.queryByText('Go B')).not.toBeNull(); }); + test('should preserve history during StrictMode effect replay and clean it up on unmount', () => { + const id = 'strict-mode'; + const { context, histories } = setPersistedHistory(id); + const originalHref = window.location.href; + + const { unmount } = render( + + + , + ); + + expect(screen.queryByText('B')).not.toBeNull(); + expect(window.location.href).toBe(originalHref); + expect(new URLSearchParams(window.location.search).get(`${id}.step`)).toBe('B'); + expect(new URLSearchParams(window.location.search).get('unrelated')).toBe('preserved'); + expect(window.location.hash).toBe('#summary'); + expect(window.history.state[`${id}.context`]).toEqual(context); + expect(window.history.state[`${id}.histories`]).toEqual(histories); + expect(window.history.state.unrelated).toBe('preserved'); + + unmount(); + + expect(new URLSearchParams(window.location.search).get(`${id}.step`)).toBeNull(); + expect(window.history.state[`${id}.context`]).toBeUndefined(); + expect(window.history.state[`${id}.histories`]).toBeUndefined(); + expect(new URLSearchParams(window.location.search).get('unrelated')).toBe('preserved'); + expect(window.location.hash).toBe('#summary'); + expect(window.history.state.unrelated).toBe('preserved'); + }); + + test('should not restore history over a same-path navigation', () => { + const id = 'navigation'; + setPersistedHistory(id); + + function NavigateDuringReplay() { + useEffect(() => { + return () => { + window.history.replaceState({ replacement: 'new' }, '', '?view=other#details'); + }; + }, []); + + return null; + } + + render( + + + + , + ); + + expect(new URLSearchParams(window.location.search).get(`${id}.step`)).toBeNull(); + expect(window.history.state[`${id}.context`]).toBeUndefined(); + expect(window.history.state[`${id}.histories`]).toBeUndefined(); + expect(new URLSearchParams(window.location.search).get('view')).toBe('other'); + expect(window.location.hash).toBe('#details'); + expect(window.history.state.replacement).toBe('new'); + }); + + test('should clean up history before the same funnel id remounts', () => { + const id = 'remount'; + setPersistedHistory(id); + + const firstRender = render(); + firstRender.unmount(); + + expect(new URLSearchParams(window.location.search).get(`${id}.step`)).toBeNull(); + expect(window.history.state[`${id}.context`]).toBeUndefined(); + expect(window.history.state[`${id}.histories`]).toBeUndefined(); + + render(); + + expect(screen.queryByText('A')).not.toBeNull(); + }); + test('should work funnel.Render.overlay', async () => { function FunnelRenderTest() { const funnel = useFunnel<{