-
Notifications
You must be signed in to change notification settings - Fork 436
feat(nextjs): Isolate nonce fetch in Suspense boundary for PPR support #7773
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat(nextjs): Isolate nonce fetch in Suspense boundary for PPR support #7773
Conversation
Move nonce fetching from the server ClerkProvider's main body into a separate DynamicClerkScripts server component wrapped in Suspense. This allows pages using dynamic=true to remain statically renderable and compatible with PPR/cacheComponents. - Create DynamicClerkScripts async server component - Add getNonce cached function to utils - Skip client ClerkScripts when server scripts are used - Pass __internal_skipScripts through KeylessProvider
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
🦋 Changeset detectedLatest commit: 31da6d0 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 |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds an internal boolean prop 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@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: |
…ndling Extract duplicated script rendering into a shared ClerkScriptTags component used by both ClerkScripts (client) and DynamicClerkScripts (server). Add try/catch to getNonce() so errors in prerendering or "use cache" contexts degrade gracefully instead of propagating unhandled.
…n RSC Import clerkJSScriptUrl, buildClerkJSScriptAttributes, clerkUIScriptUrl from @clerk/shared/loadClerkJsScript instead of @clerk/react/internal in the shared ClerkScriptTags component. The @clerk/react/internal barrel re-exports modules that use React.createContext, which breaks when the RSC bundler evaluates the barrel in server component context.
…t-await-nonce-in-the-clerkprovider
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/nextjs/src/app-router/server/utils.ts`:
- Around line 156-180: Add automated tests for getNonce covering three
scenarios: (1) when the request provides X-Nonce header ensure getNonce returns
that value and caching via reactCache deduplicates repeated calls; (2) when
X-Nonce is absent but Content-Security-Policy contains a script-nonce ensure
getNonce falls back to getScriptNonceFromHeader and returns the extracted nonce;
and (3) when the dynamic import throws a prerendering bailout
(isPrerenderingBailout returns true) ensure the bailout is rethrown. In tests,
mock the dynamic import of 'next/headers' to return a headers() object with
get() behavior, spy/override getScriptNonceFromHeader and isPrerenderingBailout
to simulate CSP and bailout cases, and verify caching semantics by calling
getNonce multiple times in the same test to confirm reactCache deduplication.
|
!snapshot |
|
Hey @jacekradko - the snapshot version command generated the following package versions:
Tip: Use the snippet copy button below to quickly install the required packages. npm i @clerk/agent-toolkit@0.3.0-snapshot.v20260206212043 --save-exact
npm i @clerk/astro@3.0.0-snapshot.v20260206212043 --save-exact
npm i @clerk/backend@3.0.0-snapshot.v20260206212043 --save-exact
npm i @clerk/chrome-extension@3.0.0-snapshot.v20260206212043 --save-exact
npm i @clerk/clerk-js@6.0.0-snapshot.v20260206212043 --save-exact
npm i @clerk/dev-cli@1.0.0-snapshot.v20260206212043 --save-exact
npm i @clerk/expo@3.0.0-snapshot.v20260206212043 --save-exact
npm i @clerk/expo-passkeys@1.0.0-snapshot.v20260206212043 --save-exact
npm i @clerk/express@2.0.0-snapshot.v20260206212043 --save-exact
npm i @clerk/fastify@2.7.0-snapshot.v20260206212043 --save-exact
npm i @clerk/localizations@4.0.0-snapshot.v20260206212043 --save-exact
npm i @clerk/msw@0.0.1-snapshot.v20260206212043 --save-exact
npm i @clerk/nextjs@7.0.0-snapshot.v20260206212043 --save-exact
npm i @clerk/nuxt@2.0.0-snapshot.v20260206212043 --save-exact
npm i @clerk/react@6.0.0-snapshot.v20260206212043 --save-exact
npm i @clerk/react-router@3.0.0-snapshot.v20260206212043 --save-exact
npm i @clerk/shared@4.0.0-snapshot.v20260206212043 --save-exact
npm i @clerk/tanstack-react-start@1.0.0-snapshot.v20260206212043 --save-exact
npm i @clerk/testing@2.0.0-snapshot.v20260206212043 --save-exact
npm i @clerk/ui@1.0.0-snapshot.v20260206212043 --save-exact
npm i @clerk/upgrade@2.0.0-snapshot.v20260206212043 --save-exact
npm i @clerk/vue@2.0.0-snapshot.v20260206212043 --save-exact |
Ephem
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! This looks very similar to what I was envisioning, but a bit better. 😁 Left a few NIT-comments, I'll go ahead and take this for a spin in the dashboard.
| @@ -61,14 +68,7 @@ export function ClerkScripts({ router }: { router: ClerkScriptProps['router'] }) | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find the naming here confusing.
ClerkScripts -> Used for both app and pages
ClerkScriptTags -> The app router "static" script tags
ClerkScript -> The pages router "static" script tags (only kind there is)
?
Maybe we can finish the swing you started here and get rid of ClerkScripts entirely now, and just render ClerkScriptTags directly from the app router, and ClerkScript directly from the pages one, seeing how ClerkScripts doesn't really have any valuable shared logic?
And also rename ClerkScriptTags and ClerkScript (or just let them live in the app and pages router folders instead of at the shared level)?
…t-await-nonce-in-the-clerkprovider # Conflicts: # packages/nextjs/src/utils/clerk-script.tsx
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
3 similar comments
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
…erkScripts per router - Move getNonce from shared utils into DynamicClerkScripts (its only consumer), replacing the dynamic import with a static import of next/headers and removing the reactCache fallback since this is an RSC-only component. - Split the shared ClerkScripts component into router-specific versions in app-router/client/ and pages/, eliminating the router prop dispatch. - Delete utils/clerk-script.tsx.
Cover X-Nonce header, CSP fallback, missing nonce, prerendering bailout rethrow, and graceful degradation on non-bailout errors.
…t-await-nonce-in-the-clerkprovider
Ephem
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the updates!
|
|
||
| return ( | ||
| <> | ||
| <script |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like we've always used a <Script> tag here before, was there a specific reason to change that?
| const dynamicScripts = dynamic ? ( | ||
| <Suspense> | ||
| <DynamicClerkScripts | ||
| publishableKey={propsWithEnvs.publishableKey} | ||
| clerkJSUrl={propsWithEnvs.clerkJSUrl} | ||
| clerkJSVersion={propsWithEnvs.clerkJSVersion} | ||
| clerkUIUrl={propsWithEnvs.clerkUIUrl} | ||
| domain={propsWithEnvs.domain} | ||
| proxyUrl={propsWithEnvs.proxyUrl} | ||
| prefetchUI={propsWithEnvs.prefetchUI} | ||
| /> | ||
| </Suspense> | ||
| ) : null; | ||
|
|
||
| if (shouldRunAsKeyless) { | ||
| return ( | ||
| <KeylessProvider | ||
| rest={propsWithEnvs} | ||
| runningWithClaimedKeys={runningWithClaimedKeys} | ||
| __internal_skipScripts={dynamic} | ||
| > | ||
| {dynamicScripts} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of introducing a new boolean prop, maybe we can add 'use client' at the top of ClerkScripts and leverage composition?
Also, I think there's an issue with the scripts being included twice if we do this:
<ClerkProvider dynamic>
<ClerkProvider dynamic>Which might seem like an invalid pattern today, but we don't know how this part of the code will evolve so I think it might be nice to tighten it up.
So perhaps we introduce a "scriptsSlot" or similar, that does not get rendered for nested boundaries (that is, when the InitialStateProvider renders alone)? Something like:
| const dynamicScripts = dynamic ? ( | |
| <Suspense> | |
| <DynamicClerkScripts | |
| publishableKey={propsWithEnvs.publishableKey} | |
| clerkJSUrl={propsWithEnvs.clerkJSUrl} | |
| clerkJSVersion={propsWithEnvs.clerkJSVersion} | |
| clerkUIUrl={propsWithEnvs.clerkUIUrl} | |
| domain={propsWithEnvs.domain} | |
| proxyUrl={propsWithEnvs.proxyUrl} | |
| prefetchUI={propsWithEnvs.prefetchUI} | |
| /> | |
| </Suspense> | |
| ) : null; | |
| if (shouldRunAsKeyless) { | |
| return ( | |
| <KeylessProvider | |
| rest={propsWithEnvs} | |
| runningWithClaimedKeys={runningWithClaimedKeys} | |
| __internal_skipScripts={dynamic} | |
| > | |
| {dynamicScripts} | |
| const scripts = dynamic ? ( | |
| <Suspense> | |
| <DynamicClerkScripts | |
| publishableKey={propsWithEnvs.publishableKey} | |
| clerkJSUrl={propsWithEnvs.clerkJSUrl} | |
| clerkJSVersion={propsWithEnvs.clerkJSVersion} | |
| clerkUIUrl={propsWithEnvs.clerkUIUrl} | |
| domain={propsWithEnvs.domain} | |
| proxyUrl={propsWithEnvs.proxyUrl} | |
| prefetchUI={propsWithEnvs.prefetchUI} | |
| /> | |
| </Suspense> | |
| ) : <ClerkScripts />; | |
| if (shouldRunAsKeyless) { | |
| return ( | |
| <KeylessProvider | |
| rest={propsWithEnvs} | |
| runningWithClaimedKeys={runningWithClaimedKeys} | |
| scriptsSlot={scripts} | |
| > |
Co-authored-by: Fredrik Höglund <fredrik@clerk.dev>
…ion pattern Replace the boolean `__internal_skipScripts` prop with a `__internal_scriptsSlot` React node prop. The server provider now passes dynamic scripts as a slot prop instead of rendering them as sibling children, preventing duplicate script rendering in nested `<ClerkProvider dynamic>` scenarios.
|
!snapshot |
|
Hey @jacekradko - the snapshot version command generated the following package versions:
Tip: Use the snippet copy button below to quickly install the required packages. npm i @clerk/agent-toolkit@0.3.0-snapshot.v20260210163448 --save-exact
npm i @clerk/astro@3.0.0-snapshot.v20260210163448 --save-exact
npm i @clerk/backend@3.0.0-snapshot.v20260210163448 --save-exact
npm i @clerk/chrome-extension@3.0.0-snapshot.v20260210163448 --save-exact
npm i @clerk/clerk-js@6.0.0-snapshot.v20260210163448 --save-exact
npm i @clerk/dev-cli@1.0.0-snapshot.v20260210163448 --save-exact
npm i @clerk/expo@3.0.0-snapshot.v20260210163448 --save-exact
npm i @clerk/expo-passkeys@1.0.0-snapshot.v20260210163448 --save-exact
npm i @clerk/express@2.0.0-snapshot.v20260210163448 --save-exact
npm i @clerk/fastify@2.7.0-snapshot.v20260210163448 --save-exact
npm i @clerk/localizations@4.0.0-snapshot.v20260210163448 --save-exact
npm i @clerk/msw@0.0.1-snapshot.v20260210163448 --save-exact
npm i @clerk/nextjs@7.0.0-snapshot.v20260210163448 --save-exact
npm i @clerk/nuxt@2.0.0-snapshot.v20260210163448 --save-exact
npm i @clerk/react@6.0.0-snapshot.v20260210163448 --save-exact
npm i @clerk/react-router@3.0.0-snapshot.v20260210163448 --save-exact
npm i @clerk/shared@4.0.0-snapshot.v20260210163448 --save-exact
npm i @clerk/tanstack-react-start@1.0.0-snapshot.v20260210163448 --save-exact
npm i @clerk/testing@2.0.0-snapshot.v20260210163448 --save-exact
npm i @clerk/ui@1.0.0-snapshot.v20260210163448 --save-exact
npm i @clerk/upgrade@2.0.0-snapshot.v20260210163448 --save-exact
npm i @clerk/vue@2.0.0-snapshot.v20260210163448 --save-exact |
Summary
DynamicClerkScriptsserver component wrapped inSuspensedynamic=trueto remain statically renderable and compatible with PPR/cacheComponentsProblem
In the Next.js App Router server
ClerkProvider, awaiting the nonce fromheaders()opts the entire page out of static rendering, breaking PPR/cacheComponents.Solution
Isolate the nonce fetch in a Suspense boundary:
DynamicClerkScriptsasync server component that fetches the nonce and renders script tagsdynamic=true, render<Suspense><DynamicClerkScripts/></Suspense>and skip client-sideClerkScriptsvia__internal_skipScriptsgetNonceuses a staticimport { headers } from 'next/headers'(no dynamic import needed since this is an App Router-only component)ClerkScriptTagsas a shared pure presentational component for script + preload tagsClerkScriptswrapper into router-specific components:app-router/client/ClerkScripts.tsxandpages/ClerkScripts.tsxTest plan
DynamicClerkScriptscovering X-Nonce header, CSP fallback, missing nonce, prerendering bailout rethrow, and graceful degradationdynamic=true— scripts render correctlyCloses USER-4607