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 3 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: f166d40 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 a new 🚥 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).
1 participant
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 (Vite 7.3.1 + vinext 0.0.15 + 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 usersSummary by CodeRabbit