diff --git a/.changeset/nextjs-hidden-provider-navigation.md b/.changeset/nextjs-hidden-provider-navigation.md new file mode 100644 index 00000000000..3479ade01ff --- /dev/null +++ b/.changeset/nextjs-hidden-provider-navigation.md @@ -0,0 +1,5 @@ +--- +'@clerk/nextjs': patch +--- + +Fix sign-in and sign-out getting stuck in apps with `cacheComponents`. This happened after a client-side navigation that changed a root param, for example from `/en` to `/sk`. diff --git a/packages/nextjs/src/app-router/client/__tests__/useInternalNavFun.test.tsx b/packages/nextjs/src/app-router/client/__tests__/useInternalNavFun.test.tsx index b1312550b7d..05c2f63722e 100644 --- a/packages/nextjs/src/app-router/client/__tests__/useInternalNavFun.test.tsx +++ b/packages/nextjs/src/app-router/client/__tests__/useInternalNavFun.test.tsx @@ -194,4 +194,32 @@ describe('useInternalNavFun', () => { expect(routerNav).not.toHaveBeenCalled(); }); + + it('navigates with the mounted instance when another instance renders but never commits', async () => { + const mountedRouterNav = vi.fn(); + const uncommittedRouterNav = vi.fn(); + const NeverResolves = React.lazy(() => new Promise(() => {})); + + render( + <> + + + + + + , + ); + + let promise!: Promise; + act(() => { + promise = navigate('/dashboard'); + }); + + await waitFor(() => { + expect(mountedRouterNav).toHaveBeenCalledWith('/dashboard'); + }); + + expect(uncommittedRouterNav).not.toHaveBeenCalled(); + await expect(promise).resolves.toBeUndefined(); + }); }); diff --git a/packages/nextjs/src/app-router/client/useInternalNavFun.ts b/packages/nextjs/src/app-router/client/useInternalNavFun.ts index 4787a9d2379..cb2a66f2041 100644 --- a/packages/nextjs/src/app-router/client/useInternalNavFun.ts +++ b/packages/nextjs/src/app-router/client/useInternalNavFun.ts @@ -1,6 +1,7 @@ import type { AppRouterInstance } from 'next/dist/shared/lib/app-router-context.shared-runtime'; import { useCallback, useEffect, useTransition } from 'react'; +import { useSafeLayoutEffect } from '../../client-boundary/hooks/useSafeLayoutEffect'; import { removeBasePath } from '../../utils/removeBasePath'; const getClerkNavigationObject = (name: string) => { @@ -18,48 +19,50 @@ export const useInternalNavFun = (props: { const { windowNav, routerNav, name } = props; const [isPending, startTransition] = useTransition(); - if (windowNav) { - getClerkNavigationObject(name).fun = (to, opts) => { - const nav = getClerkNavigationObject(name); + useSafeLayoutEffect(() => { + 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; - // 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 => { + // 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 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 + // without unmounting/remounting the page or fetching data from the server. + if (opts?.__internal_metadata?.navigationType === 'internal') { + // Passing `null` ensures App Router shallow navigations keep search params reactive. + windowNav(null, '', to); + } else { + // If the navigation is external (usually when navigating away from the component but still within the app), + // we should use the Next.js router to navigate as it will handle updating the URL and also + // fetching the new page if necessary. + routerNav(removeBasePath(to)); + } + }); }); - } - - 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 - 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 - // without unmounting/remounting the page or fetching data from the server. - if (opts?.__internal_metadata?.navigationType === 'internal') { - // Passing `null` ensures App Router shallow navigations keep search params reactive. - windowNav(null, '', to); - } else { - // If the navigation is external (usually when navigating away from the component but still within the app), - // we should use the Next.js router to navigate as it will handle updating the URL and also - // fetching the new page if necessary. - routerNav(removeBasePath(to)); - } - }); - }); - }; - } + }; + } + }); const flushPromises = () => { const nav = getClerkNavigationObject(name);