From 9cf413bfac80e8851ce6d5d75501868f6a68314d Mon Sep 17 00:00:00 2001 From: panteliselef Date: Fri, 31 May 2024 16:09:21 +0300 Subject: [PATCH 1/2] fix(nextjs): Use separate buffers for each awaitable navigation type --- .changeset/three-eels-battle.md | 5 +++ integration/tests/navigation.test.ts | 2 +- .../src/app-router/client/useAwaitablePush.ts | 1 + .../app-router/client/useAwaitableReplace.ts | 1 + .../app-router/client/useInternalNavFun.ts | 39 +++++++++++++------ 5 files changed, 36 insertions(+), 12 deletions(-) create mode 100644 .changeset/three-eels-battle.md diff --git a/.changeset/three-eels-battle.md b/.changeset/three-eels-battle.md new file mode 100644 index 00000000000..0c1aecffa7c --- /dev/null +++ b/.changeset/three-eels-battle.md @@ -0,0 +1,5 @@ +--- +'@clerk/nextjs': patch +--- + +Bug fix: Correctly update history state when on internal navigations. diff --git a/integration/tests/navigation.test.ts b/integration/tests/navigation.test.ts index e7332d30281..e5a35115fcc 100644 --- a/integration/tests/navigation.test.ts +++ b/integration/tests/navigation.test.ts @@ -48,7 +48,7 @@ testAgainstRunningApps({ withPattern: ['next.appRouter.withEmailCodes'] })('navi await u.po.expect.toBeSignedIn(); }); - test.skip('sign in with path routing navigates to previous page', async ({ page, context }) => { + test('sign in with path routing navigates to previous page', async ({ page, context }) => { const u = createTestUtils({ app, page, context }); await u.po.signIn.goTo(); await u.po.signIn.waitForMounted(); diff --git a/packages/nextjs/src/app-router/client/useAwaitablePush.ts b/packages/nextjs/src/app-router/client/useAwaitablePush.ts index 3bf53e2310d..131c9cb7055 100644 --- a/packages/nextjs/src/app-router/client/useAwaitablePush.ts +++ b/packages/nextjs/src/app-router/client/useAwaitablePush.ts @@ -15,5 +15,6 @@ export const useAwaitablePush = () => { return useInternalNavFun({ windowNav: typeof window !== 'undefined' ? window.history.pushState.bind(window.history) : undefined, routerNav: router.push.bind(router), + name: 'push', }); }; diff --git a/packages/nextjs/src/app-router/client/useAwaitableReplace.ts b/packages/nextjs/src/app-router/client/useAwaitableReplace.ts index 9107f48f25b..44e7c7eba7e 100644 --- a/packages/nextjs/src/app-router/client/useAwaitableReplace.ts +++ b/packages/nextjs/src/app-router/client/useAwaitableReplace.ts @@ -15,5 +15,6 @@ export const useAwaitableReplace = () => { return useInternalNavFun({ windowNav: typeof window !== 'undefined' ? window.history.replaceState.bind(window.history) : undefined, routerNav: router.replace.bind(router), + name: 'replace', }); }; diff --git a/packages/nextjs/src/app-router/client/useInternalNavFun.ts b/packages/nextjs/src/app-router/client/useInternalNavFun.ts index 38c89e0c518..c9ba8c63f87 100644 --- a/packages/nextjs/src/app-router/client/useInternalNavFun.ts +++ b/packages/nextjs/src/app-router/client/useInternalNavFun.ts @@ -6,31 +6,48 @@ import type { NextClerkProviderProps } from '../../types'; declare global { interface Window { - __clerk_internal_navFun: NonNullable< - NextClerkProviderProps['routerPush'] | NextClerkProviderProps['routerReplace'] + __clerk_internal_navigations: Record< + string, + { + fun: NonNullable; + promisesBuffer: Array<() => void> | undefined; + } >; - __clerk_internal_navPromisesBuffer: Array<() => void> | undefined; } } +const registerNavigationType = (name: string) => { + if (!window.__clerk_internal_navigations) { + window.__clerk_internal_navigations = {}; + } + + if (!(name in window.__clerk_internal_navigations)) { + // @ts-ignore + window.__clerk_internal_navigations[name] = {}; + } + + return window.__clerk_internal_navigations[name]; +}; + export const useInternalNavFun = (props: { windowNav: typeof window.history.pushState | typeof window.history.replaceState | undefined; routerNav: AppRouterInstance['push'] | AppRouterInstance['replace']; + name: string; }) => { - const { windowNav, routerNav } = props; + const { windowNav, routerNav, name } = props; const pathname = usePathname(); const [isPending, startTransition] = useTransition(); if (windowNav) { - window.__clerk_internal_navFun = (to, opts) => { + registerNavigationType(name).fun = (to, opts) => { return new Promise(res => { - if (!window.__clerk_internal_navPromisesBuffer) { + if (!registerNavigationType(name).promisesBuffer) { // We need to use window to store the reference to the buffer, // as ClerkProvider might be unmounted and remounted during navigations // If we use a ref, it will be reset when ClerkProvider is unmounted - window.__clerk_internal_navPromisesBuffer = []; + registerNavigationType(name).promisesBuffer = []; } - window.__clerk_internal_navPromisesBuffer.push(res); + registerNavigationType(name).promisesBuffer!.push(res); startTransition(() => { // If the navigation is internal, we should use the history API to navigate // as this is the way to perform a shallow navigation in Next.js App Router @@ -54,8 +71,8 @@ export const useInternalNavFun = (props: { } const flushPromises = () => { - window.__clerk_internal_navPromisesBuffer?.forEach(resolve => resolve()); - window.__clerk_internal_navPromisesBuffer = []; + registerNavigationType(name).promisesBuffer?.forEach(resolve => resolve()); + registerNavigationType(name).promisesBuffer = []; }; // Flush any pending promises on mount/unmount @@ -72,6 +89,6 @@ export const useInternalNavFun = (props: { }, [pathname, isPending]); return useCallback((to: string) => { - return window.__clerk_internal_navFun(to); + return registerNavigationType(name).fun(to); }, []); }; From df7fea70fde998bd4ca62eaa876481f72be1bd5f Mon Sep 17 00:00:00 2001 From: panteliselef Date: Tue, 4 Jun 2024 11:55:02 +0300 Subject: [PATCH 2/2] chore(nextjs): Resolve PR comments --- .../app-router/client/useInternalNavFun.ts | 36 ++++++++----------- 1 file changed, 15 insertions(+), 21 deletions(-) diff --git a/packages/nextjs/src/app-router/client/useInternalNavFun.ts b/packages/nextjs/src/app-router/client/useInternalNavFun.ts index c9ba8c63f87..90e6ce68066 100644 --- a/packages/nextjs/src/app-router/client/useInternalNavFun.ts +++ b/packages/nextjs/src/app-router/client/useInternalNavFun.ts @@ -16,16 +16,10 @@ declare global { } } -const registerNavigationType = (name: string) => { - if (!window.__clerk_internal_navigations) { - window.__clerk_internal_navigations = {}; - } - - if (!(name in window.__clerk_internal_navigations)) { - // @ts-ignore - window.__clerk_internal_navigations[name] = {}; - } - +const getClerkNavigationObject = (name: string) => { + window.__clerk_internal_navigations ??= {}; + // @ts-ignore + window.__clerk_internal_navigations[name] ??= {}; return window.__clerk_internal_navigations[name]; }; @@ -39,15 +33,13 @@ export const useInternalNavFun = (props: { const [isPending, startTransition] = useTransition(); if (windowNav) { - registerNavigationType(name).fun = (to, opts) => { + getClerkNavigationObject(name).fun = (to, opts) => { return new Promise(res => { - if (!registerNavigationType(name).promisesBuffer) { - // We need to use window to store the reference to the buffer, - // as ClerkProvider might be unmounted and remounted during navigations - // If we use a ref, it will be reset when ClerkProvider is unmounted - registerNavigationType(name).promisesBuffer = []; - } - registerNavigationType(name).promisesBuffer!.push(res); + // We need to use window to store the reference to the buffer, + // as ClerkProvider might be unmounted and remounted during navigations + // If we use a ref, it will be reset when ClerkProvider is unmounted + getClerkNavigationObject(name).promisesBuffer ??= []; + getClerkNavigationObject(name).promisesBuffer?.push(res); startTransition(() => { // If the navigation is internal, we should use the history API to navigate // as this is the way to perform a shallow navigation in Next.js App Router @@ -71,8 +63,8 @@ export const useInternalNavFun = (props: { } const flushPromises = () => { - registerNavigationType(name).promisesBuffer?.forEach(resolve => resolve()); - registerNavigationType(name).promisesBuffer = []; + getClerkNavigationObject(name).promisesBuffer?.forEach(resolve => resolve()); + getClerkNavigationObject(name).promisesBuffer = []; }; // Flush any pending promises on mount/unmount @@ -89,6 +81,8 @@ export const useInternalNavFun = (props: { }, [pathname, isPending]); return useCallback((to: string) => { - return registerNavigationType(name).fun(to); + return getClerkNavigationObject(name).fun(to); + // We are not expecting name to change + // eslint-disable-next-line react-hooks/exhaustive-deps }, []); };