Skip to content

Commit 33d1ff5

Browse files
manovotnyclaude
andcommitted
fix(nextjs): make the deferred post-setActive refresh fire-and-forget
Awaiting the deferred refresh made setActive block on unrelated long-running transitions: the empty transition used to detect settling cannot finish while an app-held transition is pending, which broke the pinned behavior that auth state changes apply immediately mid-transition (transitions.test.ts). The refresh is now requested via a window-stored pending flag and dispatched by whichever hook instance observes transitions settling, so nothing awaits it (restoring onAfterSetActive's original void contract) and a request survives ClerkProvider remounts instead of resolving without ever running. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 3611e9c commit 33d1ff5

5 files changed

Lines changed: 141 additions & 164 deletions

File tree

packages/nextjs/src/app-router/client/ClerkProvider.tsx

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@ import { RouterTelemetry } from '../../utils/router-telemetry';
1313
import { invalidateCacheAction } from '../server-actions';
1414
import { ClerkScripts } from './ClerkScripts';
1515
import { useAwaitablePush } from './useAwaitablePush';
16-
import { useAwaitableRefresh } from './useAwaitableRefresh';
1716
import { useAwaitableReplace } from './useAwaitableReplace';
17+
import { useDeferredRefresh } from './useDeferredRefresh';
1818

1919
/**
2020
* LazyCreateKeylessApplication should only be loaded if the conditions below are met.
@@ -28,7 +28,7 @@ const NextClientClerkProvider = <TUi extends Ui = Ui>(props: NextClerkProviderPr
2828
const { __internal_invokeMiddlewareOnAuthStateChange = true, __internal_scriptsSlot, children } = props;
2929
const push = useAwaitablePush();
3030
const replace = useAwaitableReplace();
31-
const refresh = useAwaitableRefresh();
31+
const refresh = useDeferredRefresh();
3232

3333
useSafeLayoutEffect(() => {
3434
window.__internal_onBeforeSetActive = intent => {
@@ -72,10 +72,10 @@ const NextClientClerkProvider = <TUi extends Ui = Ui>(props: NextClerkProviderPr
7272
window.__internal_onAfterSetActive = () => {
7373
if (__internal_invokeMiddlewareOnAuthStateChange) {
7474
// Deferred until in-flight transitions settle, so the refresh is never dispatched while a
75-
// server-redirect follow-up navigation is still pending (which wedges the App Router, #9405)
76-
return refresh();
75+
// server-redirect follow-up navigation is still pending (which wedges the App Router, #9405).
76+
// Fire-and-forget: setActive must not block on unrelated long-running transitions.
77+
refresh();
7778
}
78-
return undefined;
7979
};
8080
}, []);
8181

packages/nextjs/src/app-router/client/__tests__/useAwaitableRefresh.test.tsx

Lines changed: 0 additions & 129 deletions
This file was deleted.
Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
import { act, cleanup, render, waitFor } from '@testing-library/react';
2+
import React from 'react';
3+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
4+
5+
import { useDeferredRefresh } from '../useDeferredRefresh';
6+
7+
const mockRefresh = vi.fn();
8+
9+
vi.mock('next/navigation', () => ({
10+
useRouter: () => ({ refresh: mockRefresh }),
11+
}));
12+
13+
let currentRefresh: (() => void) | undefined;
14+
15+
const Harness = () => {
16+
currentRefresh = useDeferredRefresh();
17+
return null;
18+
};
19+
20+
const refresh = () => {
21+
if (!currentRefresh) {
22+
throw new Error('refresh function is not initialized');
23+
}
24+
currentRefresh();
25+
};
26+
27+
describe('useDeferredRefresh', () => {
28+
beforeEach(() => {
29+
currentRefresh = undefined;
30+
window.__clerk_internal_refresh = undefined;
31+
vi.clearAllMocks();
32+
});
33+
34+
afterEach(() => {
35+
cleanup();
36+
});
37+
38+
it('dispatches router.refresh once transitions settle', async () => {
39+
render(<Harness />);
40+
41+
act(() => {
42+
refresh();
43+
});
44+
45+
await waitFor(() => {
46+
expect(mockRefresh).toHaveBeenCalledTimes(1);
47+
});
48+
});
49+
50+
it('coalesces concurrent requests into a single router.refresh', async () => {
51+
render(<Harness />);
52+
53+
act(() => {
54+
refresh();
55+
refresh();
56+
});
57+
58+
await waitFor(() => {
59+
expect(mockRefresh).toHaveBeenCalledTimes(1);
60+
});
61+
});
62+
63+
it('does not call router.refresh when nothing was requested', async () => {
64+
render(<Harness />);
65+
66+
// Give the isPending effect a chance to run on mount
67+
await act(async () => {
68+
await Promise.resolve();
69+
});
70+
71+
expect(mockRefresh).not.toHaveBeenCalled();
72+
});
73+
74+
it('dispatches a refresh left pending by a previous instance on mount', async () => {
75+
window.__clerk_internal_refresh = { pending: true };
76+
77+
render(<Harness />);
78+
79+
await waitFor(() => {
80+
expect(mockRefresh).toHaveBeenCalledTimes(1);
81+
});
82+
expect(window.__clerk_internal_refresh?.pending).toBe(false);
83+
});
84+
85+
it('preserves a refresh requested after unmount for the next instance', async () => {
86+
const { unmount } = render(<Harness />);
87+
unmount();
88+
89+
// Request while no instance is mounted (e.g. ClerkProvider remounting during a navigation)
90+
refresh();
91+
expect(window.__clerk_internal_refresh?.pending).toBe(true);
92+
expect(mockRefresh).not.toHaveBeenCalled();
93+
94+
render(<Harness />);
95+
96+
await waitFor(() => {
97+
expect(mockRefresh).toHaveBeenCalledTimes(1);
98+
});
99+
});
100+
101+
it('allows a fresh refresh after a previous dispatch', async () => {
102+
render(<Harness />);
103+
104+
act(() => {
105+
refresh();
106+
});
107+
await waitFor(() => {
108+
expect(mockRefresh).toHaveBeenCalledTimes(1);
109+
});
110+
111+
act(() => {
112+
refresh();
113+
});
114+
await waitFor(() => {
115+
expect(mockRefresh).toHaveBeenCalledTimes(2);
116+
});
117+
});
118+
});

packages/nextjs/src/app-router/client/useAwaitableRefresh.ts renamed to packages/nextjs/src/app-router/client/useDeferredRefresh.ts

Lines changed: 16 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -9,59 +9,47 @@ const getClerkRefreshObject = () => {
99
};
1010

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

3035
if (typeof window !== 'undefined') {
3136
getClerkRefreshObject().fun = () => {
32-
return new Promise<void>(res => {
33-
// The buffer lives on window so a pending refresh survives ClerkProvider
34-
// being unmounted and remounted during navigations.
35-
const refresh = getClerkRefreshObject();
36-
refresh.promisesBuffer ??= [];
37-
refresh.promisesBuffer.push(res);
38-
startTransition(() => {
39-
// Intentionally empty: used only to observe when in-flight transitions settle.
40-
});
37+
getClerkRefreshObject().pending = true;
38+
startTransition(() => {
39+
// Intentionally empty: used only to observe when in-flight transitions settle.
4140
});
4241
};
4342
}
4443

45-
const flushPromises = () => {
46-
const refresh = getClerkRefreshObject();
47-
refresh.promisesBuffer?.forEach(resolve => resolve());
48-
refresh.promisesBuffer = [];
49-
};
50-
51-
// Resolve any pending promises on unmount so callers awaiting a refresh are never left hanging
52-
useEffect(() => {
53-
return flushPromises;
54-
}, []);
55-
5644
useEffect(() => {
57-
if (!isPending && getClerkRefreshObject().promisesBuffer?.length) {
45+
if (!isPending && getClerkRefreshObject().pending) {
46+
getClerkRefreshObject().pending = false;
5847
router.refresh();
59-
flushPromises();
6048
}
6149
// eslint-disable-next-line react-hooks/exhaustive-deps
6250
}, [isPending]);
6351

6452
return useCallback(() => {
65-
return getClerkRefreshObject().fun?.() ?? Promise.resolve();
53+
getClerkRefreshObject().fun?.();
6654
}, []);
6755
};

packages/nextjs/src/global.d.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,8 @@ interface Window {
3939
__clerk_nav_await: Array<(value: void) => void>;
4040
__clerk_nav: (to: string) => Promise<void>;
4141
__clerk_internal_refresh?: {
42-
fun?: () => Promise<void>;
43-
promisesBuffer?: Array<() => void>;
42+
fun?: () => void;
43+
pending?: boolean;
4444
};
4545

4646
__internal_onBeforeSetActive: (intent?: 'sign-out') => void | Promise<void>;

0 commit comments

Comments
 (0)