chore(nuxt): Change default routing strategy to path-based #7260
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 4cc101c The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis PR introduces path-based routing as the default strategy for major UI components in Changes
Sequence DiagramsequenceDiagram
participant App as Nuxt App
participant Wrapper as Wrapped Component<br/>(SignIn/SignUp/etc)
participant Composable as useRoutingProps
participant BaseComponent as Base Clerk<br/>Vue Component
App->>Wrapper: Mount (with optional path prop)
activate Wrapper
Wrapper->>Composable: useRoutingProps(componentName, props, routingOptions)
activate Composable
Composable->>Composable: Validate path with routing mode
alt routing === 'path'
Composable->>Composable: Require path prop (error if missing)
else routing !== 'path' (hash/virtual)
Composable->>Composable: Reject path prop if provided (error)
Composable->>Composable: Set routing mode explicitly
end
Composable-->>Wrapper: Return computed routing props
deactivate Composable
Wrapper->>Wrapper: usePathnameWithoutSplatRouteParams()<br/>to derive current path
Wrapper->>BaseComponent: Render with merged routing props
activate BaseComponent
BaseComponent-->>Wrapper: Rendered component
deactivate BaseComponent
Wrapper-->>App: Rendered UI with path-aware routing
deactivate Wrapper
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Comment |
wobsoriano
changed the title
wobsoriano
changed the title
wobsoriano
changed the title
@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/themes
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![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
🧹 Nitpick comments (9)
.changeset/thirty-cherries-pull.md (1)
1-12: Breaking change properly documented, consider adding migration guidance.The major version bump is appropriate for this breaking routing strategy change. The changeset clearly lists affected components.
Consider whether a migration guide is needed. Users upgrading will need to:
- Update page routes from hash-based to path-based (e.g.,
/sign-in#/→/sign-in/)- Adjust any custom routing configuration
- Update middleware that relies on hash routing
Would you like me to help create migration documentation or open an issue to track this?
integration/templates/nuxt-node/app/middleware/auth.global.js (1)
4-5: Middleware correctly targets/user-profile; consider whether/hash/sign-inshould also be treated as publicThe updated matchers and redirect to
/user-profilelook consistent with the new routing structure and tests.If you expect
/hash/sign-into behave like the regular sign-in route for already-authenticated users (i.e., redirect them to/user-profileas well), you might want to include it in the public matcher:- const isPublicPage = createRouteMatcher(['/sign-in(.*)', '/sign-up(.*)']); + const isPublicPage = createRouteMatcher(['/sign-in(.*)', '/sign-up(.*)', '/hash/sign-in(.*)']);If the hash-based route is meant as an isolated example/demo, the current behavior is fine.
Also applies to: 9-15
packages/vue/src/errors/messages.ts (1)
41-45: Routing error messages are clear; consider framework-neutral examplesThe new
noPathProvidedErrorandincompatibleRoutingWithPathProvidedErrorstrings line up with theuseRoutingPropsbehavior and give actionable guidance.Minor nit: the examples (
path={'/my-path'},routing='path') read like JSX. If you want the messages to feel more framework-neutral (and match Vue templates a bit better), you could switch to something like:`Example: <${componentName} path="/my-path" />` `To resolve this error, set routing="path" on <${componentName} /> ...`Purely cosmetic; current wording is still understandable.
integration/tests/nuxt/navigation.test.ts (2)
7-20: Make teardown resilient tobeforeAllfailures
fakeUseris assumed to be initialized inafterAll; ifcreateFakeUserorcreateBapiUserthrows,afterAllwill error when accessingfakeUser.deleteIfExists(), masking the original failure.Consider making
fakeUseroptional and guarding in teardown, e.g. initializing it tonulland checking before callingdeleteIfExists.
66-75: Assert the URL when checking that user profile uses path routingThe test name implies verifying path routing, but it currently only checks for the
Securityheader. To fully exercise the routing behavior, it would be stronger to also assert the URL, for example:await u.page.goToRelative('/user-profile/security'); await u.page.waitForURL(`${app.serverUrl}/user-profile/security`); await expect(page.url()).not.toContain('#'); await expect(u.page.locator('.cl-headerTitle').filter({ hasText: 'Security' })).toBeVisible();This ensures the component is actually mounted via a path-based route with no hash fragment.
packages/vue/src/composables/useRoutingProps.ts (2)
13-18: TightenpropsValueinitialization for clearer typing and semantics
const propsValue = toValue(props) || {};can both (a) widen the inferred type ofpropsValue(toT | {}) and (b) treat falsy-but-valid values as “missing”. While callers are unlikely to pass such values, you can make this safer and more explicit by using nullish coalescing and preserving the generic type:const propsValue = (toValue(props) ?? {}) as T;This keeps
propsValuestrongly typed asTand only falls back when the source isnull/undefined, matching the intended usage.
17-40: Clarify behavior when non‑path routing is combined with apathfromroutingOptionsFor non‑
'path'routing modes, the check:if (propsValue.path) { return errorThrower.throw(incompatibleRoutingWithPathProvidedError(componentName)); }only treats a
pathcoming frompropsas incompatible; apathcoming fromroutingOptionsValueis silently dropped by the final spread +path: undefined.If the intent is that any
pathprovided alongside non‑path routing (hash,virtual, etc.) should be considered a configuration error, you might want to base the incompatibility check on the combinedpath(from either source) instead ofpropsValue.pathalone. If the current behavior is intentional (wrappers providing a defaultpaththat can be overridden byrouting: 'hash'), consider adding a short comment to make that contract explicit.packages/nuxt/src/runtime/components/uiComponents.ts (2)
21-48: Clarify assumptions and edge cases inusePathnameWithoutSplatRouteParamsThe helper is a nice way to normalize the mount path, but a few implicit assumptions are baked in:
- It treats any array-valued
route.paramsentry as a catch‑all and flattens all such arrays into a singlesplatRouteParam.- It then removes the first occurrence of that joined segment from
route.pathvia.replace(splatRouteParam, ...).If you ever end up with multiple array params or the same segment value repeated in the path, this could remove an earlier occurrence than intended. Also, the logic assumes Nuxt always represents catch‑all params as arrays, including the single‑segment case.
This is probably fine for the current
/user-profile/[...rest]style routes, but it would be good to either (a) document these assumptions in a brief comment, or (b) tighten the behavior (e.g., only stripping a trailingsplatRouteParamwith an end‑anchored replace) to make it robust to future route shape changes.
81-119: Consider forwarding slots for non‑profile wrappers as well
UserProfileandOrganizationProfileforwardslotsto their base components:return () => h(BaseUserProfile, routingProps.value, slots);while
CreateOrganization,OrganizationList,SignIn, andSignUponly passroutingProps.value:return () => h(BaseSignIn, routingProps.value);If any of these base components support slot‑based customization in
@clerk/vue, existing Nuxt users relying on those slots could lose that behavior now that the wrapped versions are exported by default.It may be worth aligning them with the profile wrappers by also forwarding
slots(and adding any static properties that need to be preserved), or explicitly confirming that these components are not expected to use slots in Nuxt.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
packages/vue/src/composables/__tests__/__snapshots__/useRoutingProps.test.ts.snapis excluded by!**/*.snappnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (21)
.changeset/thirty-cherries-pull.md(1 hunks).changeset/wild-bees-explode.md(1 hunks)integration/templates/nuxt-node/app/middleware/auth.global.js(1 hunks)integration/templates/nuxt-node/app/pages/hash/sign-in/[...slug].vue(1 hunks)integration/templates/nuxt-node/app/pages/sign-in.vue(0 hunks)integration/templates/nuxt-node/app/pages/sign-in/[...slug].vue(1 hunks)integration/templates/nuxt-node/app/pages/sign-up/[...slug].vue(1 hunks)integration/tests/nuxt/basic.test.ts(2 hunks)integration/tests/nuxt/navigation.test.ts(1 hunks)packages/nuxt/package.json(1 hunks)packages/nuxt/src/module.ts(2 hunks)packages/nuxt/src/runtime/components/index.ts(1 hunks)packages/nuxt/src/runtime/components/uiComponents.ts(1 hunks)packages/nuxt/tsup.config.ts(2 hunks)packages/vue/package.json(1 hunks)packages/vue/src/composables/__tests__/useRoutingProps.test.ts(1 hunks)packages/vue/src/composables/useRoutingProps.ts(1 hunks)packages/vue/src/errors/messages.ts(1 hunks)packages/vue/src/internal.ts(1 hunks)packages/vue/tsup.config.ts(1 hunks)pnpm-workspace.yaml(1 hunks)
💤 Files with no reviewable changes (1)
- integration/templates/nuxt-node/app/pages/sign-in.vue
🔇 Additional comments (14)
packages/vue/package.json (1)
73-73: LGTM! Centralized version management.The change to use
catalog:repoaligns with the workspace catalog entry (Vue 3.5.21) and ensures consistent Vue versions across packages..changeset/wild-bees-explode.md (1)
1-5: LGTM! Changeset properly documents the minor version bump.The changeset correctly specifies a minor version bump for the internal composable addition, which is appropriate for internal API changes.
packages/nuxt/tsup.config.ts (2)
29-29: LGTM! Correctly externalizing Vue.Adding
vueto the externals array ensures Vue is not bundled and will be resolved at runtime, which is the correct approach for this integration.
11-11: No issues found with the glob pattern.The directory contains only 2 TypeScript files (
index.tsanduiComponents.ts), both of which are legitimate component files. There are no test files (.test.ts,.spec.ts) or unintended utility files present, so the glob pattern./src/runtime/components/*.tsappropriately captures only the intended component files.packages/nuxt/package.json (1)
81-82: LGTM! Consistent dependency management.Adding Vue as a dev dependency with
catalog:repoaligns with the workspace catalog strategy and supports the new Vue-based component wrappers.packages/vue/tsup.config.ts (1)
2-2: LGTM! Clean type import simplification.Removing the unused
Optionstype import simplifies the code while maintaining functionality, asdefineConfigprovides the necessary typing.integration/templates/nuxt-node/app/pages/sign-in/[...slug].vue (1)
1-3: No changes needed — SignIn component is properly auto-imported.Verification confirms that
SignInis explicitly registered for auto-import inpackages/nuxt/src/module.ts(line 142-154). The module uses Nuxt'saddComponent()function to register the component with the correct export name and file path, enabling it to be used without explicit imports in templates.integration/templates/nuxt-node/app/pages/sign-up/[...slug].vue (1)
1-3: Sign-up catch-all page wiring looks correctTemplate-only page using
SignUpwithsignInUrl="/sign-in"is consistent with the new path-based routing defaults and the slug-based sign-up route structure.packages/vue/src/internal.ts (1)
1-2: Internal export foruseRoutingPropsis appropriateExposing
useRoutingPropsfrom the internal entrypoint aligns with how Nuxt/runtime wrappers consume Vue internals and keeps the public API clean.integration/templates/nuxt-node/app/pages/hash/sign-in/[...slug].vue (1)
1-3: Hash-based sign-in page matches the routing strategyUsing
SignInwithrouting="hash"under a dedicated/hash/sign-in/[...slug]route cleanly preserves hash-routing support alongside the new path-based default.integration/tests/nuxt/basic.test.ts (1)
33-49: Test route updates align with new/user-profileredirect targetSwitching the navigation target from
/userto/user-profilein both the SSR data and redirect tests matches the updated middleware behavior and the new profile route. The flows still look coherent end-to-end.Also applies to: 51-57
packages/vue/src/composables/__tests__/useRoutingProps.test.ts (1)
1-117:useRoutingPropstest coverage looks solidThe suite exercises the key behaviors (default path routing, hash/virtual interactions, error paths, and precedence rules) and ties directly to the new error messages. Using JSON snapshots for the computed options keeps the assertions focused and maintainable.
packages/nuxt/src/module.ts (1)
140-188: The review comment's concern aboutAPIKeysremoval appears to be based on incorrect assumptions.The code review assumes that
APIKeyswas previously auto-registered in the Nuxt module and has been removed in this PR. However, the evidence contradicts this:
CHANGELOG says "Introduce": The Nuxt CHANGELOG entry states "Introduce
<APIKeys />component ([#7225])" — not "Remove" — suggesting this is a new addition to Nuxt, not a removal.No prior registration found: A complete scan of
packages/nuxt/src/module.tsshowsAPIKeyshas never appeared in the auto-registration lists, and no evidence exists of it being in a prior version.Intentional exclusion is reasonable: The
APIKeyscomponent is in "early access and not yet recommended for production use" (per the clerk-js codebase). Excluding experimental features from auto-registration is a valid design decision to avoid promoting unstable APIs to new users.The concern about breaking existing apps that rely on
<APIKeys>being auto-registered is unfounded if it was never auto-registered in Nuxt to begin with.Likely an incorrect or invalid review comment.
packages/nuxt/src/runtime/components/index.ts (1)
1-1: Re-export change looks correct and preserves the public surfaceRouting the six key components through
./uiComponentswhile leaving the rest sourced from@clerk/vuekeeps the existing import API intact (import { SignIn } from '@clerk/nuxt') and cleanly swaps in the Nuxt-aware wrappers.No issues spotted here.
pnpm-workspace.yaml
Outdated
| typescript: 5.8.3 | ||
| zx: 8.8.5 | ||
| rolldown: 1.0.0-beta.47 | ||
| vue: 3.5.21 |
There was a problem hiding this comment.
🧩 Analysis chain
Verify the Vue version is current and secure.
Since my knowledge is from March 2025 and this is November 2025, please verify that Vue 3.5.21 is still an appropriate version choice.
🌐 Web query:
What is the latest stable version of Vue 3 as of November 2025?
💡 Result:
The latest stable Vue 3 release is v3.5.24 (released November 7, 2025). [1]
Sources:
[1] GitHub — vuejs/core releases (v3.5.24).
Update Vue to the latest stable version (3.5.24).
The latest stable Vue 3 release is v3.5.24 (released November 7, 2025), but pnpm-workspace.yaml specifies 3.5.21. Update to 3.5.24 to receive recent bug fixes and security improvements.
🤖 Prompt for AI Agents
In pnpm-workspace.yaml around line 20, the Vue version is pinned to 3.5.21;
update that entry to 3.5.24. Edit the file to replace "vue: 3.5.21" with "vue:
3.5.24", then run your package manager (e.g., pnpm install or pnpm up vue
--latest) to update lockfiles and run the test/build suite to verify nothing
broke.
Description
Changes the default routing strategy for Clerk UI components in the Nuxt SDK from hash-based to path-based routing, matching the behavior of other fullstack SDKs like Next.js, React Router, and TanStack Start.
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.