diff --git a/.changeset/cozy-lemons-crash.md b/.changeset/cozy-lemons-crash.md new file mode 100644 index 00000000000..c9fea8bf9e9 --- /dev/null +++ b/.changeset/cozy-lemons-crash.md @@ -0,0 +1,5 @@ +--- +'@clerk/nextjs': patch +--- + +Fix an App Router navigation edge case where duplicate in-flight redirects to the same destination could leave Clerk's awaitable navigation pending indefinitely. diff --git a/packages/nextjs/src/app-router/client/__tests__/useInternalNavFun.test.tsx b/packages/nextjs/src/app-router/client/__tests__/useInternalNavFun.test.tsx new file mode 100644 index 00000000000..113f8de777f --- /dev/null +++ b/packages/nextjs/src/app-router/client/__tests__/useInternalNavFun.test.tsx @@ -0,0 +1,116 @@ +import { act, cleanup, render, waitFor } from '@testing-library/react'; +import React from 'react'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { useInternalNavFun } from '../useInternalNavFun'; + +let mockedPathname = '/'; + +vi.mock('next/navigation', () => ({ + usePathname: () => mockedPathname, +})); + +let currentNavigate: NavigationFunction | undefined; + +const windowNav = window.history.pushState.bind(window.history); + +const Harness = ({ routerNav }: { routerNav: (...args: any[]) => unknown }) => { + currentNavigate = useInternalNavFun({ + windowNav, + routerNav: routerNav as any, + name: 'push', + }); + + return null; +}; + +const navigate = (to: string) => { + if (!currentNavigate) { + throw new Error('navigate function is not initialized'); + } + + return currentNavigate(to, { windowNavigate: vi.fn() } as any) as Promise; +}; + +describe('useInternalNavFun', () => { + beforeEach(() => { + mockedPathname = '/protected'; + currentNavigate = undefined; + window.__clerk_internal_navigations = {}; + vi.clearAllMocks(); + }); + + afterEach(() => { + cleanup(); + }); + + it('dedupes duplicate in-flight pushes to the same destination', async () => { + const routerNav = vi.fn(); + const view = render(); + + let firstPromise!: Promise; + let secondPromise!: Promise; + + act(() => { + firstPromise = navigate('/sign-in'); + secondPromise = navigate('/sign-in'); + }); + + await waitFor(() => { + expect(routerNav).toHaveBeenCalledTimes(1); + }); + + // Simulate route change completion so buffered resolvers flush. + mockedPathname = '/sign-in'; + view.rerender(); + + await expect(Promise.all([firstPromise, secondPromise])).resolves.toEqual([undefined, undefined]); + }); + + it('does not dedupe when destination differs', async () => { + const routerNav = vi.fn(); + render(); + + act(() => { + void navigate('/sign-in'); + void navigate('/sign-up'); + }); + + await waitFor(() => { + expect(routerNav).toHaveBeenCalledTimes(2); + }); + + expect(routerNav).toHaveBeenNthCalledWith(1, '/sign-in'); + expect(routerNav).toHaveBeenNthCalledWith(2, '/sign-up'); + }); + + it('allows a fresh same-destination push after flush', async () => { + const routerNav = vi.fn(); + const view = render(); + + let firstPromise!: Promise; + let secondPromise!: Promise; + + act(() => { + firstPromise = navigate('/sign-in'); + secondPromise = navigate('/sign-in'); + }); + + await waitFor(() => { + expect(routerNav).toHaveBeenCalledTimes(1); + }); + + mockedPathname = '/sign-in'; + view.rerender(); + + await expect(Promise.all([firstPromise, secondPromise])).resolves.toEqual([undefined, undefined]); + + act(() => { + void navigate('/sign-in'); + }); + + await waitFor(() => { + expect(routerNav).toHaveBeenCalledTimes(2); + }); + }); +}); diff --git a/packages/nextjs/src/app-router/client/useInternalNavFun.ts b/packages/nextjs/src/app-router/client/useInternalNavFun.ts index af7b7e11866..8964ad1a734 100644 --- a/packages/nextjs/src/app-router/client/useInternalNavFun.ts +++ b/packages/nextjs/src/app-router/client/useInternalNavFun.ts @@ -22,12 +22,29 @@ export const useInternalNavFun = (props: { if (windowNav) { getClerkNavigationObject(name).fun = (to, opts) => { + const nav = getClerkNavigationObject(name); + + // If a transition to this exact URL is already in-flight, avoid starting + // a second one. Calling startTransition with a duplicate router.push() + // can permanently stick useTransition's isPending at `true` when Next.js + // deduplicates/cancels the redundant router action. Instead, add the + // resolver to the shared buffer so it resolves alongside the existing + // transition. + if ((nav.promisesBuffer?.length ?? 0) > 0 && nav.pendingDestination === to) { + return new Promise(res => { + nav.promisesBuffer ??= []; + nav.promisesBuffer.push(res); + }); + } + + nav.pendingDestination = to; + return new Promise(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); + nav.promisesBuffer ??= []; + nav.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 @@ -47,8 +64,10 @@ export const useInternalNavFun = (props: { } const flushPromises = () => { - getClerkNavigationObject(name).promisesBuffer?.forEach(resolve => resolve()); - getClerkNavigationObject(name).promisesBuffer = []; + const nav = getClerkNavigationObject(name); + nav.promisesBuffer?.forEach(resolve => resolve()); + nav.promisesBuffer = []; + nav.pendingDestination = undefined; }; // Flush any pending promises on mount/unmount diff --git a/packages/nextjs/src/global.d.ts b/packages/nextjs/src/global.d.ts index efefffd1ed0..fa739c3edcf 100644 --- a/packages/nextjs/src/global.d.ts +++ b/packages/nextjs/src/global.d.ts @@ -33,6 +33,7 @@ interface Window { { fun: NavigationFunction; promisesBuffer: Array<() => void> | undefined; + pendingDestination: string | undefined; } >; __clerk_nav_await: Array<(value: void) => void>;