fix(nextjs): make @clerk/nextjs ESM-safe for non-Node.js runtimes#7954
fix(nextjs): make @clerk/nextjs ESM-safe for non-Node.js runtimes#7954nikosdouvlis wants to merge 5 commits intomainfrom
Conversation
auth(), auth.protect(), and currentUser() call require('server-only')
at runtime. Cloudflare Workers is pure ESM with no require(), so this
crashes with "require is not defined" when running @clerk/nextjs on
vinext (Cloudflare's Vite-based Next.js reimplementation).
Replace the three bare require() calls with an assertServerOnly()
helper that checks typeof require before calling. On Next.js (where
require exists) behavior is identical. On pure ESM runtimes the guard
is skipped, deferring to the bundler's own RSC/client environment
separation.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: ff5650a 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 |
📝 WalkthroughWalkthroughAdds an 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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/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: |
safe-node-apis.js (node + browser) used require/module.exports which
crashes in Vite's ESM module runner. usePathnameWithoutCatchAll used
require('next/navigation') to lazily load hooks, which also fails in
pure ESM runtimes. Since @clerk/nextjs only supports Next.js 15.2.8+,
next/navigation is always available as a static import.
nikosdouvlis
changed the title
![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 the current code and only fix it if needed.
Inline comments:
In `@packages/nextjs/src/client-boundary/hooks/usePathnameWithoutCatchAll.tsx`:
- Around line 27-35: The hook calls usePathname() and useParams() are currently
invoked only when pagesRouter is falsy, violating the Rules of Hooks; move both
usePathname() and useParams() to be called unconditionally at the top of
usePathnameWithoutCatchAll (so always call usePathname() and useParams() to get
pathname and params) and then keep the existing pagesRouter conditional logic to
decide which values to use or to short-circuit, using the unconditional values
rather than calling hooks inside branches; update any references to pathname,
pathParts, and catchAllParams accordingly (functions/identifiers:
usePathnameWithoutCatchAll, usePathname, useParams, pagesRouter, pathname,
params, catchAllParams).
ℹ️ Review info
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
.changeset/quiet-waves-guard.mdpackages/nextjs/src/client-boundary/hooks/usePathnameWithoutCatchAll.tsxpackages/nextjs/src/runtime/browser/safe-node-apis.jspackages/nextjs/src/runtime/node/safe-node-apis.js
🚧 Files skipped from review as they are similar to previous changes (1)
- .changeset/quiet-waves-guard.md
| const pathname = usePathname() ?? ''; | ||
| const pathParts = pathname.split('/').filter(Boolean); | ||
| // the useParams hook returns an object with all named and catch all params | ||
| // for named params, the key in the returned object always contains a single value | ||
| // for catch all params, the key in the returned object contains an array of values | ||
| // we find the catch all params by checking if the value is an array | ||
| // and then we remove one path part for each catch all param | ||
| const catchAllParams = Object.values(useParams() || {}) | ||
| const params = useParams(); | ||
| const catchAllParams = Object.values(params ?? {}) |
There was a problem hiding this comment.
Rules of Hooks violation: usePathname() and useParams() are called conditionally.
The early return on lines 11-20 when pagesRouter exists means these hooks are only called when in App Router mode. This violates React's Rules of Hooks and is flagged by Biome.
While this may work in practice because an app is typically either Pages Router or App Router (not both), this pattern can cause React warnings in development and potential issues if the router mode detection ever changes between renders.
Consider restructuring to call hooks unconditionally at the top of the function, then conditionally use their values.
Proposed restructure
export const usePathnameWithoutCatchAll = () => {
const pathRef = React.useRef<string>();
-
const { pagesRouter } = usePagesRouter();
+ const pathname = usePathname() ?? '';
+ const params = useParams();
if (pagesRouter) {
if (pathRef.current) {
return pathRef.current;
} else {
pathRef.current = pagesRouter.pathname.replace(/\/\[\[\.\.\..*/, '');
return pathRef.current;
}
}
- const pathname = usePathname() ?? '';
const pathParts = pathname.split('/').filter(Boolean);
- const params = useParams();
const catchAllParams = Object.values(params ?? {})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const pathname = usePathname() ?? ''; | |
| const pathParts = pathname.split('/').filter(Boolean); | |
| // the useParams hook returns an object with all named and catch all params | |
| // for named params, the key in the returned object always contains a single value | |
| // for catch all params, the key in the returned object contains an array of values | |
| // we find the catch all params by checking if the value is an array | |
| // and then we remove one path part for each catch all param | |
| const catchAllParams = Object.values(useParams() || {}) | |
| const params = useParams(); | |
| const catchAllParams = Object.values(params ?? {}) | |
| export const usePathnameWithoutCatchAll = () => { | |
| const pathRef = React.useRef<string>(); | |
| const { pagesRouter } = usePagesRouter(); | |
| const pathname = usePathname() ?? ''; | |
| const params = useParams(); | |
| if (pagesRouter) { | |
| if (pathRef.current) { | |
| return pathRef.current; | |
| } else { | |
| pathRef.current = pagesRouter.pathname.replace(/\/\[\[\.\.\..*/, ''); | |
| return pathRef.current; | |
| } | |
| } | |
| const pathParts = pathname.split('/').filter(Boolean); | |
| const catchAllParams = Object.values(params ?? {}) |
🧰 Tools
🪛 Biome (2.4.4)
[error] 27-27: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
(lint/correctness/useHookAtTopLevel)
[error] 34-34: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
(lint/correctness/useHookAtTopLevel)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/nextjs/src/client-boundary/hooks/usePathnameWithoutCatchAll.tsx`
around lines 27 - 35, The hook calls usePathname() and useParams() are currently
invoked only when pagesRouter is falsy, violating the Rules of Hooks; move both
usePathname() and useParams() to be called unconditionally at the top of
usePathnameWithoutCatchAll (so always call usePathname() and useParams() to get
pathname and params) and then keep the existing pagesRouter conditional logic to
decide which values to use or to short-circuit, using the unconditional values
rather than calling hooks inside branches; update any references to pathname,
pathParts, and catchAllParams accordingly (functions/identifiers:
usePathnameWithoutCatchAll, usePathname, useParams, pagesRouter, pathname,
params, catchAllParams).
Add integration test infrastructure and 3 test suites (12 tests) to validate @clerk/nextjs works on vinext (Cloudflare's Vite-based Next.js reimplementation for Workers). Template: integration/templates/vinext-app/ with minimal Clerk app Preset: integration/presets/vinext.ts wired into test framework Tests: quickstart (sign-in/out), protect (auth gating), auth-state (cookie persistence, cross-tab, server/client consistency) All tests tagged @vinext for filtering.
|
@nikosdouvlis good work, would like to get this merged to deploy on vinext if I can. Wondering if this merits changes in: TBH mostly going blind here and letting codex guide me as I'm not an expert on nodejs internals |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@integration/templates/vinext-app/app/api/me/route.ts`:
- Line 3: Exported template functions are missing explicit return type
annotations; add them to the three functions: annotate GET in
integration/templates/vinext-app/app/api/me/route.ts as async GET():
Promise<Response> (or Promise<NextResponse> if you’re using NextResponse), and
annotate the React page components Page in
integration/templates/vinext-app/app/sign-in/[[...sign-in]]/page.tsx and
integration/templates/vinext-app/app/sign-up/[[...sign-up]]/page.tsx as function
Page(): JSX.Element (or async Page(): Promise<JSX.Element> if they are async),
updating the function signatures only to include these explicit return types.
ℹ️ Review info
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (18)
integration/presets/index.tsintegration/presets/vinext.tsintegration/templates/index.tsintegration/templates/vinext-app/.gitignoreintegration/templates/vinext-app/app/api/me/route.tsintegration/templates/vinext-app/app/auth-display.tsxintegration/templates/vinext-app/app/layout.tsxintegration/templates/vinext-app/app/page.tsxintegration/templates/vinext-app/app/protected/page.tsxintegration/templates/vinext-app/app/sign-in/[[...sign-in]]/page.tsxintegration/templates/vinext-app/app/sign-up/[[...sign-up]]/page.tsxintegration/templates/vinext-app/middleware.tsintegration/templates/vinext-app/package.jsonintegration/templates/vinext-app/tsconfig.jsonintegration/templates/vinext-app/vite.config.tsintegration/tests/vinext-auth-state.test.tsintegration/tests/vinext-protect.test.tsintegration/tests/vinext-quickstart.test.ts
✅ Files skipped from review due to trivial changes (1)
- integration/templates/vinext-app/.gitignore
| @@ -0,0 +1,14 @@ | |||
| import { auth } from '@clerk/nextjs/server'; | |||
|
|
|||
| export async function GET() { | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if the mentioned files exist and inspect their function signatures
echo "=== Checking integration/templates/vinext-app/app/api/me/route.ts ==="
if [ -f "integration/templates/vinext-app/app/api/me/route.ts" ]; then
cat -n "integration/templates/vinext-app/app/api/me/route.ts"
else
echo "File not found"
fi
echo ""
echo "=== Checking integration/templates/vinext-app/app/sign-in/[[...sign-in]]/page.tsx ==="
if [ -f "integration/templates/vinext-app/app/sign-in/[[...sign-in]]/page.tsx" ]; then
cat -n "integration/templates/vinext-app/app/sign-in/[[...sign-in]]/page.tsx"
else
echo "File not found"
fi
echo ""
echo "=== Checking integration/templates/vinext-app/app/sign-up/[[...sign-up]]/page.tsx ==="
if [ -f "integration/templates/vinext-app/app/sign-up/[[...sign-up]]/page.tsx" ]; then
cat -n "integration/templates/vinext-app/app/sign-up/[[...sign-up]]/page.tsx"
else
echo "File not found"
fiRepository: clerk/javascript
Length of output: 950
Add explicit return types on exported template functions.
Exported functions in these files lack explicit return type annotations:
integration/templates/vinext-app/app/api/me/route.tsline 3:GET()integration/templates/vinext-app/app/sign-in/[[...sign-in]]/page.tsxline 2:Page()integration/templates/vinext-app/app/sign-up/[[...sign-up]]/page.tsxline 2:Page()
Per TypeScript guidelines, always define explicit return types for public APIs.
Proposed fixes
-export async function GET() {
+export async function GET(): Promise<Response> {
const authObj = await auth();
return new Response(-export default function Page() {
+export default function Page(): JSX.Element {
return <SignIn />;
}-export default function Page() {
+export default function Page(): JSX.Element {
return <SignUp />;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export async function GET() { | |
| export async function GET(): Promise<Response> { | |
| const authObj = await auth(); | |
| return new Response( |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@integration/templates/vinext-app/app/api/me/route.ts` at line 3, Exported
template functions are missing explicit return type annotations; add them to the
three functions: annotate GET in
integration/templates/vinext-app/app/api/me/route.ts as async GET():
Promise<Response> (or Promise<NextResponse> if you’re using NextResponse), and
annotate the React page components Page in
integration/templates/vinext-app/app/sign-in/[[...sign-in]]/page.tsx and
integration/templates/vinext-app/app/sign-up/[[...sign-up]]/page.tsx as function
Page(): JSX.Element (or async Page(): Promise<JSX.Element> if they are async),
updating the function signatures only to include these explicit return types.
Add test:integration:vinext script, turbo task, and CI matrix entry so the @vinext tagged integration tests run on every PR.
3 participants
Why
@clerk/nextjscrashes on import in pure ESM runtimes like Cloudflare Workers (via vinext). Three separaterequire()patterns fail becauserequireis not defined in ESM-only environments.Ref: cloudflare/vinext#73
What changed
auth(),auth.protect(), andcurrentUser()usedrequire('server-only')which crashes in ESM runtimes. Replaced withassertServerOnly()that checkstypeof requirebefore calling it. Theserver-onlypackage uses thereact-serverexport condition, which vinext/Workers already enforce at the bundler level, so the guard is redundant there.safe-node-apis.js(both node and browser variants) usedrequire('node:fs')/module.exports, which crashes in Vite's ESM module runner. Converted to ESMimport/export default.usePathnameWithoutCatchAllusedrequire('next/navigation')to lazily load hooks. Since@clerk/nextjsonly supports Next.js 15.2.8+,next/navigationis always available as a static import.Testing
Verified with a vinext smoke test app deployed to Cloudflare Workers (vinext 0.0.18 + Next.js 16.1.6):
/(home withauth()) - 200, renders auth state/api/me(API route withauth()) - 200, returns userId/sessionId/sign-in(<SignIn />component) - 200, renders sign-in UI/protected(auth.protect()) - correctly redirects unauthenticated usersAlso added 12 Playwright e2e tests across 3 suites (quickstart, protect, auth-state) running against a local vinext dev server in CI.
Note on vinext CJS support
vinext 0.0.18 added
vite-plugin-commonjs(cloudflare/vinext#198) to handlerequire()at build time. We tested: the build passes with canary@clerk/nextjs(without our fixes), but Cloudflare Workers deploy still fails because Workers statically rejectsrequire()in the output bundle. The plugin also doesn't handle mixed ESM+CJS files (likeusePathnameWithoutCatchAll.tsxwhich has bothimportandrequire()). Our ESM fixes remain necessary.