Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/cozy-lemons-crash.md
Original file line number Diff line number Diff line change
@@ -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. We now coalesce duplicate same-destination pushes into a single in-flight transition, reset pending destination state after flush, and add regression tests for duplicate and post-flush navigation behavior.
Original file line number Diff line number Diff line change
@@ -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<void>;
};

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(<Harness routerNav={routerNav} />);

let firstPromise!: Promise<void>;
let secondPromise!: Promise<void>;

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(<Harness routerNav={routerNav} />);

await expect(Promise.all([firstPromise, secondPromise])).resolves.toEqual([undefined, undefined]);
});

it('does not dedupe when destination differs', async () => {
const routerNav = vi.fn();
render(<Harness routerNav={routerNav} />);

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(<Harness routerNav={routerNav} />);

let firstPromise!: Promise<void>;
let secondPromise!: Promise<void>;

act(() => {
firstPromise = navigate('/sign-in');
secondPromise = navigate('/sign-in');
});

await waitFor(() => {
expect(routerNav).toHaveBeenCalledTimes(1);
});

mockedPathname = '/sign-in';
view.rerender(<Harness routerNav={routerNav} />);

await expect(Promise.all([firstPromise, secondPromise])).resolves.toEqual([undefined, undefined]);

act(() => {
void navigate('/sign-in');
});

await waitFor(() => {
expect(routerNav).toHaveBeenCalledTimes(2);
});
});
});
27 changes: 23 additions & 4 deletions packages/nextjs/src/app-router/client/useInternalNavFun.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<void>(res => {
nav.promisesBuffer ??= [];
nav.promisesBuffer.push(res);
});
}

nav.pendingDestination = to;

return new Promise<void>(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
Expand All @@ -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
Expand Down
1 change: 1 addition & 0 deletions packages/nextjs/src/global.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ interface Window {
{
fun: NavigationFunction;
promisesBuffer: Array<() => void> | undefined;
pendingDestination: string | undefined;
}
>;
__clerk_nav_await: Array<(value: void) => void>;
Expand Down
Loading