fix(nextjs): defer the post-setActive refresh until router transitions settle - #9406
fix(nextjs): defer the post-setActive refresh until router transitions settle#9406manovotny wants to merge 2 commits into
Conversation
…s settle Clerk's post-auth navigation into a route whose Server Component calls redirect() wedged the App Router on the intermediate route. The refresh dispatched from __internal_onAfterSetActive landed while Next was still following the redirect, got appended behind a discarded entry in the router action queue, and never ran — leaving React suspended on its unresolved state promise. ClerkProvider now routes that refresh through useAwaitableRefresh, which starts an empty transition and dispatches router.refresh() only once isPending settles, so the refresh always reaches an idle action queue. Fixes #9405 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
🦋 Changeset detectedLatest commit: 33d1ff5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/electron
@clerk/electron-passkeys
@clerk/eslint-plugin
@clerk/expo
@clerk/expo-google-signin
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/hono
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
API Changes Report
Summary
No API Changes DetectedAll packages have stable APIs with no detected changes. Report generated by Break Check Last ran on |
📝 WalkthroughWalkthroughThe change adds Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with 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.
Inline comments:
In `@packages/nextjs/src/app-router/client/useAwaitableRefresh.ts`:
- Around line 51-54: Update useAwaitableRefresh cleanup so unmounting a
ClerkProvider instance preserves queued refresh callbacks for a successor hook
and does not flush the window buffer prematurely; only resolve callbacks without
dispatching router.refresh() after permanent teardown is established. In
packages/nextjs/src/app-router/client/__tests__/useAwaitableRefresh.test.tsx
lines 97-109, add the remount scenario verifying one refresh dispatch occurs
before promise resolution and update the immediate-unmount assertion for the
final teardown behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: a562e132-30c0-47d1-96f0-0959a9bfd118
📒 Files selected for processing (5)
.changeset/defer-refresh-after-set-active.mdpackages/nextjs/src/app-router/client/ClerkProvider.tsxpackages/nextjs/src/app-router/client/__tests__/useAwaitableRefresh.test.tsxpackages/nextjs/src/app-router/client/useAwaitableRefresh.tspackages/nextjs/src/global.d.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
clerk/clerk_go(manual)clerk/dashboard(manual)clerk/accounts(manual)clerk/backoffice(manual)clerk/clerk(manual)clerk/clerk-docs(manual)clerk/cloudflare-workers(manual)clerk/clerk-ios(auto-detected)clerk/cli(auto-detected)clerk/clerk-android(auto-detected)
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>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with 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.
Inline comments:
In `@packages/nextjs/src/app-router/client/__tests__/useDeferredRefresh.test.tsx`:
- Around line 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.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 88439700-5b18-4657-b806-ba535f113566
📒 Files selected for processing (4)
packages/nextjs/src/app-router/client/ClerkProvider.tsxpackages/nextjs/src/app-router/client/__tests__/useDeferredRefresh.test.tsxpackages/nextjs/src/app-router/client/useDeferredRefresh.tspackages/nextjs/src/global.d.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
clerk/clerk_go(manual)clerk/dashboard(manual)clerk/accounts(manual)clerk/backoffice(manual)clerk/clerk(manual)clerk/clerk-docs(manual)clerk/cloudflare-workers(manual)clerk/clerk-ios(auto-detected)clerk/cli(auto-detected)clerk/clerk-android(auto-detected)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/nextjs/src/app-router/client/ClerkProvider.tsx
| it('dispatches router.refresh once transitions settle', async () => { | ||
| render(<Harness />); | ||
|
|
||
| act(() => { | ||
| refresh(); | ||
| }); | ||
|
|
||
| await waitFor(() => { | ||
| expect(mockRefresh).toHaveBeenCalledTimes(1); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🎯 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
1 participant
Description
Clerk's post-auth navigation into a route whose Server Component calls
redirect()hung the App Router on the intermediate route:Rendering…forever, URL parked, signed-out UI. Therouter.refresh()we dispatch from__internal_onAfterSetActivelands while Next is still following the server redirect. In dev, Next's redirect boundary dispatches the follow-up navigation twice (Strict Mode double effects), the duplicate discards the first queue entry, and the action queue'slastpointer is left on the discarded node. Our refresh gets appended behind that dead node, never runs, and the unresolved state promise it handed to React suspends the router forever. Everything on Clerk's side completes; the wedge is the refresh timing.ClerkProvidernow routes the post-setActiverefresh through a newuseDeferredRefreshhook. A request sets a pending flag onwindowand starts an empty transition; the refresh dispatches onceisPendingsettles. An empty transition can't settle while the redirect follow-up is still rendering, so the refresh always reaches an idle action queue. The hook is fire-and-forget —setActivemust not block on unrelated app-held transitions (transitions.test.tspins that auth state changes apply immediately mid-transition), soonAfterSetActivekeeps its original void contract. Concurrent requests coalesce into one refresh, and the flag survivesClerkProviderremounts, so a request made mid-remount is dispatched by the successor instance.To test: run https://github.com/manovotny/clerk-nextjs-post-auth-redirect-hang against this build and click "1. Sign in → /setup". Before, it hung on
/setup; now it lands on/landedwith the signed-in header.setActiveduring a heldstartTransitionresolves in ~57ms with the refresh parked until the transition finishes.The queue bookkeeping is a known Next.js bug, fixed upstream in
next@16.3.0via vercel/next.js#95391 (verified: the repro on stock 16.3.0 lands even with unfixed@clerk/nextjs). It remains broken on every 15.5.1–15.5.23 and 16.0–16.2.12 release, and 16.2.x/15.5.x patch releases cut after the fix did not backport it — so this change is what protects the Next versions most apps are actually on.Fixes #9405
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change