fix(*): Fix cleanup effect is prevent normal navigate (#119)

* Fix cleanup effect is prevent normal navigate * fix with changeset * remove console.log
toss · Feb 24, 2025 · e9d378f · e9d378f
1 parent 0877d04
commit e9d378f
Show file tree

Hide file tree

Showing 16 changed files with 313 additions and 147 deletions.
diff --git a/.changeset/short-news-worry.md b/.changeset/short-news-worry.md
@@ -0,0 +1,9 @@
+---
+'@use-funnel/react-navigation-native': patch
+'@use-funnel/react-router-dom': patch
+'@use-funnel/react-router': patch
+'@use-funnel/browser': patch
+'@use-funnel/next': patch
+---
+
+Fix cleanup effect is prevent normal navigate
diff --git a/examples/nextjs-app-router/app/funnel/header.tsx b/examples/nextjs-app-router/app/funnel/header.tsx
@@ -1,5 +1,11 @@
 'use client';
 
+import Link from 'next/link';
+
 export default function Header() {
-  return <div style={{ border: '1px solid blue', padding: '10px', backgroundColor: 'lightgray' }}>Header</div>;
+  return (
+    <div style={{ border: '1px solid blue', padding: '10px', backgroundColor: 'lightgray' }}>
+      <Link href="/">Home</Link>
+    </div>
+  );
 }
diff --git a/examples/nextjs-app-router/e2e/app-router-funnel.spec.ts b/examples/nextjs-app-router/e2e/app-router-funnel.spec.ts
@@ -1,19 +1,39 @@
-import { test, expect } from '@playwright/test';
+import { expect, test } from '@playwright/test';
 
 const APP_ROUTER_LOCAL_URL = 'http://localhost:3100/';
 
 test('can move the steps of the funnel using history.push.', async ({ page }) => {
   await page.goto(APP_ROUTER_LOCAL_URL);
 
-  await expect(page.getByText('start')).toBeVisible();
+  await page.getByRole('link', { name: 'Go to Funnel' }).click();
+
+  await expect(page.getByText('MAIN - START')).toBeVisible();
+  await expect(page.getByText('Sub Start')).toBeVisible();
 
   await expect(page.getByRole('button', { name: 'next' })).toBeVisible();
 
   await page.getByRole('button', { name: 'next' }).click();
 
-  await expect(page.getByText('end')).toBeVisible();
+  await expect(page.getByText('Sub End')).toBeVisible();
+
+  await page.getByRole('button', { name: 'next' }).click();
+
+  await expect(page.getByText('MAIN - END')).toBeVisible();
 
   await page.goBack();
 
-  await expect(page.getByText('start')).toBeVisible();
+  await expect(page.getByText('Sub End')).toBeVisible();
+});
+
+test('can move the steps of the funnel using Link.', async ({ page }) => {
+  await page.goto(APP_ROUTER_LOCAL_URL);
+
+  await page.getByRole('link', { name: 'Go to Funnel' }).click();
+
+  await expect(page.getByText('MAIN - START')).toBeVisible();
+  await expect(page.getByText('Sub Start')).toBeVisible();
+
+  await page.getByRole('link', { name: 'Home' }).click();
+
+  await expect(page.getByRole('link', { name: 'Go to Funnel' })).toBeVisible();
 });
diff --git a/examples/nextjs-pages-router/e2e/pages-router-funnel.spec.ts b/examples/nextjs-pages-router/e2e/pages-router-funnel.spec.ts
@@ -1,4 +1,4 @@
-import { test, expect } from '@playwright/test';
+import { expect, test } from '@playwright/test';
 
 const PAGES_ROUTER_LOCAL_URL = 'http://localhost:3101/';
 
@@ -17,3 +17,11 @@ test('can move the steps of the funnel using history.push.', async ({ page }) =>
 
   await expect(page.getByText('start')).toBeVisible();
 });
+
+test('can move the steps of the funnel using Link.', async ({ page }) => {
+  await page.goto(PAGES_ROUTER_LOCAL_URL);
+
+  await page.getByRole('link', { name: 'Go Home' }).click();
+
+  await expect(page.getByText('Home')).toBeVisible();
+});
diff --git a/examples/nextjs-pages-router/pages/home.tsx b/examples/nextjs-pages-router/pages/home.tsx
@@ -0,0 +1,3 @@
+export default function Home() {
+  return <h1>Home</h1>;
+}
diff --git a/examples/nextjs-pages-router/src/funnel.tsx b/examples/nextjs-pages-router/src/funnel.tsx
@@ -1,18 +1,22 @@
 import { useFunnel } from '@use-funnel/next';
+import Link from 'next/link';
 
 export const TestPagesRouterFunnel = () => {
   const funnel = useFunnel<FunnelState>({ id: FUNNEL_ID, initial: { step: 'start', context: {} } });
   return (
-    <funnel.Render
-      start={({ history }) => (
-        <div>
-          <p>start</p>
+    <>
+      <Link href="/home">Go Home</Link>
+      <funnel.Render
+        start={({ history }) => (
+          <div>
+            <p>start</p>
 
-          <button onClick={() => history.push('end')}>next</button>
-        </div>
-      )}
-      end={() => <div>end</div>}
-    />
+            <button onClick={() => history.push('end')}>next</button>
+          </div>
+        )}
+        end={() => <div>end</div>}
+      />
+    </>
   );
 };
 

diff --git a/packages/browser/src/index.ts b/packages/browser/src/index.ts
@@ -44,7 +44,7 @@ export const useFunnel = createUseFunnel(({ id, initialState }) => {
 
   const changeState = useCallback(
     (method: 'pushState' | 'replaceState', newState: AnyFunnelState) => {
-      const searchParams = new URLSearchParams(location.search);
+      const searchParams = new URLSearchParams(window.location.search);
       searchParams.set(`${id}.step`, newState.step);
 
       const newHistoryState = {
@@ -83,12 +83,20 @@ export const useFunnel = createUseFunnel(({ id, initialState }) => {
           ...window.history.state,
         };
 
+        const searchParams = new URLSearchParams(window.location.search);
+
+        if (
+          newHistoryState[`${id}.context`] == null ||
+          newHistoryState[`${id}.histories`] == null ||
+          searchParams.get(`${id}.step`) == null
+        ) {
+          return;
+        }
+
         delete newHistoryState[`${id}.context`];
         delete newHistoryState[`${id}.histories`];
 
-        const searchParams = new URLSearchParams(location.search);
         searchParams.delete(`${id}.step`);
-
         window.history.replaceState(newHistoryState, '', `?${searchParams.toString()}`);
       },
     }),

diff --git a/packages/browser/test/index.test.tsx b/packages/browser/test/index.test.tsx
@@ -129,24 +129,26 @@ describe('Test useFunnel browser router', () => {
           context: {},
         },
       });
-      return <funnel.Render
-        sub1={({ history }) => <button onClick={() => history.push('sub2', { id: 'SubId' })}>Go to sub2</button>}
-        sub2={({ context }) => {
-          return (
-            <div>
-              <h1>Your id is {context.id}?</h1>
-              <button onClick={() => props.onNext(context.id)}>OK</button>
-            </div>
-          );
-        }}
-      />
+      return (
+        <funnel.Render
+          sub1={({ history }) => <button onClick={() => history.push('sub2', { id: 'SubId' })}>Go to sub2</button>}
+          sub2={({ context }) => {
+            return (
+              <div>
+                <h1>Your id is {context.id}?</h1>
+                <button onClick={() => props.onNext(context.id)}>OK</button>
+              </div>
+            );
+          }}
+        />
+      );
     }
 
     function MainFunnel() {
       const funnel = useFunnel<{
         main1: { id1?: string };
         main2: { id1: string; id2?: string };
-        main3: { id1: string; id2?: string }
+        main3: { id1: string; id2?: string };
       }>({
         id: 'main',
         initial: {
@@ -155,20 +157,22 @@ describe('Test useFunnel browser router', () => {
         },
       });
 
-      return <funnel.Render
-        main1={({ history }) => <SubFunnel onNext={(id) => history.push('main2', { id1: id })} />}
-        main2={({ context, history }) => (
-          <div>
-            <h1>id1 is {context.id1}!</h1>
-            {context.id2 == null ? (
-              <button onClick={() => history.push('main3')}>set id2</button>
-            ) : (
-              <h1>id2 is {context.id2}</h1>
-            )}
-          </div>
-        )}
-        main3={({ history }) => <SubFunnel onNext={(id) => history.push('main2', { id2: id })} />}
-      />
+      return (
+        <funnel.Render
+          main1={({ history }) => <SubFunnel onNext={(id) => history.push('main2', { id1: id })} />}
+          main2={({ context, history }) => (
+            <div>
+              <h1>id1 is {context.id1}!</h1>
+              {context.id2 == null ? (
+                <button onClick={() => history.push('main3')}>set id2</button>
+              ) : (
+                <h1>id2 is {context.id2}</h1>
+              )}
+            </div>
+          )}
+          main3={({ history }) => <SubFunnel onNext={(id) => history.push('main2', { id2: id })} />}
+        />
+      );
     }
 
     render(<MainFunnel />);
@@ -200,5 +204,5 @@ describe('Test useFunnel browser router', () => {
     // Main2
     expect(screen.queryByText('id1 is SubId!')).not.toBeNull();
     expect(screen.queryByText('id2 is SubId')).not.toBeNull();
-  })
+  });
 });
diff --git a/packages/core/src/useFunnel.tsx b/packages/core/src/useFunnel.tsx
@@ -34,8 +34,8 @@ export interface UseFunnel<TRouteOption extends RouteOption> {
     TStepKeys extends keyof _TStepContextMap = keyof _TStepContextMap,
     TStepContext extends _TStepContextMap[TStepKeys] = _TStepContextMap[TStepKeys],
     TStepContextMap extends string extends keyof _TStepContextMap
-    ? Record<TStepKeys, TStepContext>
-    : _TStepContextMap = string extends keyof _TStepContextMap ? Record<TStepKeys, TStepContext> : _TStepContextMap,
+      ? Record<TStepKeys, TStepContext>
+      : _TStepContextMap = string extends keyof _TStepContextMap ? Record<TStepKeys, TStepContext> : _TStepContextMap,
   >(
     options: UseFunnelOptions<TStepContextMap>,
   ): UseFunnelResults<TStepContextMap, TRouteOption>;
@@ -49,8 +49,8 @@ export function createUseFunnel<TRouteOption extends RouteOption>(
     TStepKeys extends keyof _TStepContextMap = keyof _TStepContextMap,
     TStepContext extends _TStepContextMap[TStepKeys] = _TStepContextMap[TStepKeys],
     TStepContextMap extends string extends keyof _TStepContextMap
-    ? Record<TStepKeys, TStepContext>
-    : _TStepContextMap = string extends keyof _TStepContextMap ? Record<TStepKeys, TStepContext> : _TStepContextMap,
+      ? Record<TStepKeys, TStepContext>
+      : _TStepContextMap = string extends keyof _TStepContextMap ? Record<TStepKeys, TStepContext> : _TStepContextMap,
   >(options: UseFunnelOptions<TStepContextMap>): UseFunnelResults<TStepContextMap, TRouteOption> {
     const optionsRef = useUpdatableRef(options);
     const router = useFunnelRouter({
@@ -66,8 +66,8 @@ export function createUseFunnel<TRouteOption extends RouteOption>(
     useEffect(() => {
       return () => {
         cleanUpRef.current();
-      }
-    }, [])
+      };
+    }, []);
 
     const parseStepContext = useCallback(
       <TStep extends keyof TStepContextMap>(step: TStep, context: unknown): TStepContextMap[TStep] | null => {
@@ -94,16 +94,16 @@ export function createUseFunnel<TRouteOption extends RouteOption>(
           typeof assignContext === 'function'
             ? assignContext(currentStateRef.current.context)
             : {
-              ...currentStateRef.current.context,
-              ...assignContext,
-            };
+                ...currentStateRef.current.context,
+                ...assignContext,
+              };
         const context = parseStepContext(step, newContext);
         return context == null
           ? optionsRef.current.initial
           : ({
-            step,
-            context,
-          } as FunnelStateByContextMap<TStepContextMap>);
+              step,
+              context,
+            } as FunnelStateByContextMap<TStepContextMap>);
       };
       return {
         push: async (...args) => {
@@ -129,9 +129,9 @@ export function createUseFunnel<TRouteOption extends RouteOption>(
         ...(validContext == null
           ? optionsRef.current.initial
           : {
-            step: currentState.step,
-            context: validContext,
-          }),
+              step: currentState.step,
+              context: validContext,
+            }),
         history,
         index: router.currentIndex,
         historySteps: router.history as FunnelStateByContextMap<TStepContextMap>[],

diff --git a/packages/next/src/index.ts b/packages/next/src/index.ts
@@ -105,6 +105,11 @@ export const useFunnel = createUseFunnel<NextPageRouteOption>(({ id, initialStat
       go: (index) => window.history.go(index),
       async cleanup() {
         const { pathname, query } = makePath(router);
+
+        if (query[`${QS_KEY}${id}${HISTORY_KEY}`] == null || query[`${QS_KEY}${id}${CONTEXT_KEY}`] == null) {
+          return;
+        }
+
         const queryContext = {
           [`${QS_KEY}${id}${STEP_KEY}`]: undefined,
           [`${QS_KEY}${id}${CONTEXT_KEY}`]: undefined,

diff --git a/packages/next/test/index.test.tsx b/packages/next/test/index.test.tsx
@@ -91,24 +91,26 @@ describe('Test useFunnel next router', () => {
           context: {},
         },
       });
-      return <funnel.Render
-        sub1={({ history }) => <button onClick={() => history.push('sub2', { id: 'SubId' })}>Go to sub2</button>}
-        sub2={({ context }) => {
-          return (
-            <div>
-              <h1>Your id is {context.id}?</h1>
-              <button onClick={() => props.onNext(context.id)}>OK</button>
-            </div>
-          );
-        }}
-      />
+      return (
+        <funnel.Render
+          sub1={({ history }) => <button onClick={() => history.push('sub2', { id: 'SubId' })}>Go to sub2</button>}
+          sub2={({ context }) => {
+            return (
+              <div>
+                <h1>Your id is {context.id}?</h1>
+                <button onClick={() => props.onNext(context.id)}>OK</button>
+              </div>
+            );
+          }}
+        />
+      );
     }
 
     function MainFunnel() {
       const funnel = useFunnel<{
         main1: { id1?: string };
         main2: { id1: string; id2?: string };
-        main3: { id1: string; id2?: string }
+        main3: { id1: string; id2?: string };
       }>({
         id: 'main',
         initial: {
@@ -117,20 +119,22 @@ describe('Test useFunnel next router', () => {
         },
       });
 
-      return <funnel.Render
-        main1={({ history }) => <SubFunnel onNext={(id) => history.push('main2', { id1: id })} />}
-        main2={({ context, history }) => (
-          <div>
-            <h1>id1 is {context.id1}!</h1>
-            {context.id2 == null ? (
-              <button onClick={() => history.push('main3')}>set id2</button>
-            ) : (
-              <h1>id2 is {context.id2}</h1>
-            )}
-          </div>
-        )}
-        main3={({ history }) => <SubFunnel onNext={(id) => history.push('main2', { id2: id })} />}
-      />
+      return (
+        <funnel.Render
+          main1={({ history }) => <SubFunnel onNext={(id) => history.push('main2', { id1: id })} />}
+          main2={({ context, history }) => (
+            <div>
+              <h1>id1 is {context.id1}!</h1>
+              {context.id2 == null ? (
+                <button onClick={() => history.push('main3')}>set id2</button>
+              ) : (
+                <h1>id2 is {context.id2}</h1>
+              )}
+            </div>
+          )}
+          main3={({ history }) => <SubFunnel onNext={(id) => history.push('main2', { id2: id })} />}
+        />
+      );
     }
 
     render(<MainFunnel />);
@@ -162,5 +166,5 @@ describe('Test useFunnel next router', () => {
     // Main2
     expect(screen.queryByText('id1 is SubId!')).not.toBeNull();
     expect(screen.queryByText('id2 is SubId')).not.toBeNull();
-  })
+  });
 });