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/defer-refresh-after-set-active.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@clerk/nextjs': patch
---

Fix the App Router hanging on the intermediate route after Clerk's post-authentication navigation lands on a page whose Server Component calls `redirect()`. `ClerkProvider` now waits for in-flight route transitions to settle before dispatching its post-`setActive` `router.refresh()`, so the refresh is no longer lost inside Next.js' router action queue while the server redirect is being followed.
9 changes: 6 additions & 3 deletions packages/nextjs/src/app-router/client/ClerkProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
import { InternalClerkProvider as ReactClerkProvider, type Ui } from '@clerk/react/internal';
import { InitialStateProvider } from '@clerk/shared/react';
import dynamic from 'next/dynamic';
import { useRouter } from 'next/navigation';
import React from 'react';

import { useSafeLayoutEffect } from '../../client-boundary/hooks/useSafeLayoutEffect';
Expand All @@ -15,6 +14,7 @@ import { invalidateCacheAction } from '../server-actions';
import { ClerkScripts } from './ClerkScripts';
import { useAwaitablePush } from './useAwaitablePush';
import { useAwaitableReplace } from './useAwaitableReplace';
import { useDeferredRefresh } from './useDeferredRefresh';

/**
* LazyCreateKeylessApplication should only be loaded if the conditions below are met.
Expand All @@ -26,9 +26,9 @@ const LazyCreateKeylessApplication = dynamic(() =>

const NextClientClerkProvider = <TUi extends Ui = Ui>(props: NextClerkProviderProps<TUi>) => {
const { __internal_invokeMiddlewareOnAuthStateChange = true, __internal_scriptsSlot, children } = props;
const router = useRouter();
const push = useAwaitablePush();
const replace = useAwaitableReplace();
const refresh = useDeferredRefresh();

useSafeLayoutEffect(() => {
window.__internal_onBeforeSetActive = intent => {
Expand Down Expand Up @@ -71,7 +71,10 @@ const NextClientClerkProvider = <TUi extends Ui = Ui>(props: NextClerkProviderPr

window.__internal_onAfterSetActive = () => {
if (__internal_invokeMiddlewareOnAuthStateChange) {
return router.refresh();
// Deferred until in-flight transitions settle, so the refresh is never dispatched while a
// server-redirect follow-up navigation is still pending (which wedges the App Router, #9405).
// Fire-and-forget: setActive must not block on unrelated long-running transitions.
refresh();
}
};
}, []);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
import { act, cleanup, render, waitFor } from '@testing-library/react';
import React from 'react';
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';

import { useDeferredRefresh } from '../useDeferredRefresh';

const mockRefresh = vi.fn();

vi.mock('next/navigation', () => ({
useRouter: () => ({ refresh: mockRefresh }),
}));

let currentRefresh: (() => void) | undefined;

const Harness = () => {
currentRefresh = useDeferredRefresh();
return null;
};

const refresh = () => {
if (!currentRefresh) {
throw new Error('refresh function is not initialized');
}
currentRefresh();
};

describe('useDeferredRefresh', () => {
beforeEach(() => {
currentRefresh = undefined;
window.__clerk_internal_refresh = undefined;
vi.clearAllMocks();
});

afterEach(() => {
cleanup();
});

it('dispatches router.refresh once transitions settle', async () => {
render(<Harness />);

act(() => {
refresh();
});

await waitFor(() => {
expect(mockRefresh).toHaveBeenCalledTimes(1);
});
});
Comment on lines +38 to +48

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Test the pending-transition gate.

The test at Line 38 only proves that a requested refresh eventually runs. It does not keep isPending true and assert that router.refresh() does not run until it becomes false. A direct router.refresh() implementation would pass this test, so the regression condition described in this PR is not covered.

Add a controlled pending-transition test. Assert zero calls while the transition is pending. Then settle the transition and assert one call.

As per coding guidelines, “Unit tests are required for all new functionality” and “Include tests for all new features.”

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/nextjs/src/app-router/client/__tests__/useDeferredRefresh.test.tsx`
around lines 38 - 48, Strengthen the test around the deferred refresh behavior
in the existing useDeferredRefresh test by controlling the transition’s pending
state. Keep isPending true after refresh() is requested and assert mockRefresh
has zero calls, then settle the transition and assert it is called exactly once.

Source: Coding guidelines


it('coalesces concurrent requests into a single router.refresh', async () => {
render(<Harness />);

act(() => {
refresh();
refresh();
});

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

it('does not call router.refresh when nothing was requested', async () => {
render(<Harness />);

// Give the isPending effect a chance to run on mount
await act(async () => {
await Promise.resolve();
});

expect(mockRefresh).not.toHaveBeenCalled();
});

it('dispatches a refresh left pending by a previous instance on mount', async () => {
window.__clerk_internal_refresh = { pending: true };

render(<Harness />);

await waitFor(() => {
expect(mockRefresh).toHaveBeenCalledTimes(1);
});
expect(window.__clerk_internal_refresh?.pending).toBe(false);
});

it('preserves a refresh requested after unmount for the next instance', async () => {
const { unmount } = render(<Harness />);
unmount();

// Request while no instance is mounted (e.g. ClerkProvider remounting during a navigation)
refresh();
expect(window.__clerk_internal_refresh?.pending).toBe(true);
expect(mockRefresh).not.toHaveBeenCalled();

render(<Harness />);

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

it('allows a fresh refresh after a previous dispatch', async () => {
render(<Harness />);

act(() => {
refresh();
});
await waitFor(() => {
expect(mockRefresh).toHaveBeenCalledTimes(1);
});

act(() => {
refresh();
});
await waitFor(() => {
expect(mockRefresh).toHaveBeenCalledTimes(2);
});
});
});
55 changes: 55 additions & 0 deletions packages/nextjs/src/app-router/client/useDeferredRefresh.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
'use client';

import { useRouter } from 'next/navigation';
import { useCallback, useEffect, useTransition } from 'react';

const getClerkRefreshObject = () => {
window.__clerk_internal_refresh ??= {};
return window.__clerk_internal_refresh;
};

/**
* Returns a fire-and-forget `router.refresh()` that waits for React's in-flight transitions to
* settle before dispatching the refresh.
*
* Dispatching a refresh synchronously after an awaitable navigation resolves can permanently wedge
* the App Router: when the pushed route's Server Component calls `redirect()`, Next follows it with
* a second navigation dispatched from its redirect boundary, and a refresh dispatched while that
* follow-up is in flight can end up appended behind a discarded entry in Next's router action
* queue (fixed upstream in next@16.3.0, broken in 15.5.1 through 16.2.x). It then never runs, and
* the unresolved state promise it handed to React suspends the router forever.
*
* An empty transition started here cannot settle while another transition (such as the redirect
* follow-up navigation) is still rendering, so waiting for `isPending` to flip back guarantees the
* refresh is dispatched onto an idle action queue.
*
* The returned function is intentionally not awaitable: a long-running app transition (e.g. a
* suspended `startTransition` held open by userland code) delays the refresh, and callers such as
* `setActive` must not block on it. The pending request lives on `window` so it survives
* `ClerkProvider` remounts; the next mounted instance dispatches it.
*/
export const useDeferredRefresh = (): (() => void) => {
const router = useRouter();
const [isPending, startTransition] = useTransition();

if (typeof window !== 'undefined') {
getClerkRefreshObject().fun = () => {
getClerkRefreshObject().pending = true;
startTransition(() => {
// Intentionally empty: used only to observe when in-flight transitions settle.
});
};
}

useEffect(() => {
if (!isPending && getClerkRefreshObject().pending) {
getClerkRefreshObject().pending = false;
router.refresh();
}
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [isPending]);

return useCallback(() => {
getClerkRefreshObject().fun?.();
}, []);
};
4 changes: 4 additions & 0 deletions packages/nextjs/src/global.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ interface Window {
>;
__clerk_nav_await: Array<(value: void) => void>;
__clerk_nav: (to: string) => Promise<void>;
__clerk_internal_refresh?: {
fun?: () => void;
pending?: boolean;
};

__internal_onBeforeSetActive: (intent?: 'sign-out') => void | Promise<void>;
__internal_onAfterSetActive: () => void | Promise<void>;
Expand Down
Loading