Uh oh!
There was an error while loading. Please reload this page.
fix(init): scaffold ssr.noExternal for React Router v8 projects - #374
Conversation
React Router 8 ships development/production conditional exports. `react-router dev` externalizes @clerk/react-router for SSR, so it resolves react-router's production build while app code gets the development build — two module instances, two Router contexts — and every SSR render throws "useNavigate() may be used only in the context of a <Router>" (remix-run/react-router#15232). A clerk init'd RR8 app was broken in dev out of the box. Scaffold ssr: { noExternal: ["@clerk/react-router"] } into the vite config for react-router >= 8 (v7 has no conditional exports and needs nothing). Emits a manual post-instruction when the config is missing or uses a function form magicast can't safely modify. Verified against a live repro: create-react-router@latest (RR 8.0.0) + @clerk/react-router@3.5.5 fails in dev SSR without the entry and renders cleanly with it; production build unaffected. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 4d247bc 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 |
Companion PRs, now all open:
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR adds React Router v8 scaffold logic that updates Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant reactRouter.scaffold
participant scaffoldViteConfig
participant vite.config.* file
participant postInstructions
reactRouter.scaffold->>scaffoldViteConfig: scaffoldViteConfig(ctx)
scaffoldViteConfig->>vite.config.* file: locate and read config
alt config contains `@clerk/react-router`
scaffoldViteConfig-->>reactRouter.scaffold: no change
else config is writable
scaffoldViteConfig->>vite.config.* file: add `@clerk/react-router` to ssr.noExternal
scaffoldViteConfig-->>reactRouter.scaffold: FileAction modify
else manual wiring needed
scaffoldViteConfig-->>reactRouter.scaffold: needsManualViteWire
reactRouter.scaffold->>postInstructions: append v8 SSR note
end
Related PRs None Suggested labels None Suggested reviewers None 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Warning Review ran into problems🔥 ProblemsLinked repositories: Your configuration references 7 linked repositories, but your current plan allows 1. Analyzed Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/cli-core/src/commands/init/frameworks/react-router.test.ts (1)
461-483: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winStrengthen assertions to catch duplicate
ssrkeys, not just substring presence.This test only checks that the output contains
"some-other-pkg"and"@clerk/react-router"as substrings. It wouldn't catch a regression where the fix ends up in a duplicatessr:block that gets shadowed by the original one at runtime (see the related comment onaddClerkNoExternalinreact-router.ts). Consider asserting the actual parsed array (e.g., via a secondparseModuleonviteAction.contentand checkingssr.noExternalcontains exactly one@clerk/react-routeralongsidesome-other-pkg), or at least asserting there's only a singlessr:occurrence in the output.🤖 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/cli-core/src/commands/init/frameworks/react-router.test.ts` around lines 461 - 483, Strengthen the React Router 8 scaffold test so it verifies the final Vite config structure, not just raw substrings. In react-router.test.ts, update the assertion around reactRouter.scaffold and viteAction.content to parse the generated config (or otherwise inspect structure) and confirm ssr.noExternal contains both some-other-pkg and exactly one `@clerk/react-router` entry, with only a single ssr block present. Use the existing addClerkNoExternal / reactRouter.scaffold / viteAction checks to locate the test and make the assertion resistant to duplicate ssr keys.
🤖 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/cli-core/src/commands/init/frameworks/react-router.ts`:
- Around line 434-444: The early skip in react-router init is using a naive
content.includes("`@clerk/react-router`") check that can falsely treat unrelated
mentions as already configured. Update the logic in the config inspection path
around the existing skip branch so it only skips after confirming
`@clerk/react-router` is actually present in the ssr.noExternal configuration
(preferably via the AST parse/inspection already used in this flow, or a much
narrower pattern tied to noExternal). If the match is only incidental, continue
with the normal wiring path and emit the manual Vite instruction instead of
returning skip from the current check.
- Around line 399-420: The addClerkNoExternal helper is mutating
mod.exports.default too early and the defineConfig fallback can create a
duplicate ssr block. Unwrap the defineConfig(...) argument first, then update
the existing ssr.noExternal array on that config object before generating code;
if ssr already exists, merge into it instead of injecting a second ssr key. Use
addClerkNoExternal, parseModule, and mod.exports.default to locate the fix.
---
Nitpick comments:
In `@packages/cli-core/src/commands/init/frameworks/react-router.test.ts`:
- Around line 461-483: Strengthen the React Router 8 scaffold test so it
verifies the final Vite config structure, not just raw substrings. In
react-router.test.ts, update the assertion around reactRouter.scaffold and
viteAction.content to parse the generated config (or otherwise inspect
structure) and confirm ssr.noExternal contains both some-other-pkg and exactly
one `@clerk/react-router` entry, with only a single ssr block present. Use the
existing addClerkNoExternal / reactRouter.scaffold / viteAction checks to locate
the test and make the assertion resistant to duplicate ssr keys.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5107ccbd-34ba-4b06-99d2-2a373394222c
📒 Files selected for processing (3)
.changeset/react-router-v8-noexternal.mdpackages/cli-core/src/commands/init/frameworks/react-router.test.tspackages/cli-core/src/commands/init/frameworks/react-router.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
clerk/clerk_go(manual)
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
…icating it
magicast proxies `export default defineConfig({...})` as a function-call
expression, and property writes on that proxy throw — so the AST branch
never ran and every defineConfig config went through the regex fallback.
With a pre-existing ssr block the fallback injected a second ssr key,
and since the later key wins, the @clerk/react-router entry was
silently dropped.
Operate on the call's first argument instead and drop the string
fallback entirely: unparseable or non-array shapes now surface the
manual post-instruction rather than a maybe-broken patch. Tests now
assert a single ssr block so a duplicate-key regression fails.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Summary
clerk initon a React Router 8 project scaffolds an app that is broken in dev out of the box: every request fails during SSR withuseNavigate() may be used only in the context of a <Router> component.This came out of root-causing a public field report where an agent couldn't one-shot a Clerk + React Router app.RR8 ships development/production conditional exports.
react-router devexternalizes@clerk/react-routerfor SSR, so Node resolves react-router's production build for Clerk while the app code gets the development build through Vite — two module instances, two Router contexts, and Clerk'suseNavigate()call insideClerkProviderthrows. Upstream: remix-run/react-router#15232. The scaffolder already handles the v7/v8 split for thev8_middlewareflag but never touched the vite config.Changes
ssr: { noExternal: ["@clerk/react-router"] }intovite.config.{ts,js,mts,mjs}whenreact-routeris v8+ (or unparseable, matching how thev8_middlewaregate already treats unknown versions). v7 has no conditional exports and is left alone.ssr.noExternalarray instead of clobbering it; skips when the config already references@clerk/react-router.Verification
create-react-router@latest→ RR 8.0.0 +@clerk/react-router@3.5.5): dev SSR 500s without the entry, HTTP 200 with it;react-router buildunaffected.bun run format,lint,typecheckclean;react-router.test.ts21/21 pass. (Fullbun run testhas 363 pre-existing failures in credential-store/host-execution on cleanmainin my sandbox — identical count with and without this change.)Not planned (follow-up candidates)
test/e2e/fixtures/react-router) pinsreact-router@7.15.0, so CI never exercises v8 — worth adding an RR8 fixture so the next regression of this class is caught.References
🤖 Generated with Claude Code