-
Notifications
You must be signed in to change notification settings - Fork 419
chore(vue,nuxt): Make initialState prop public and bump dependencies
#6132
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
Conversation
🦋 Changeset detectedLatest commit: a0fd8fc 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 |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| getContents: () => `import type { SessionAuthObject } from '@clerk/backend'; | ||
| declare module 'h3' { | ||
| type AuthObjectHandler = (SignedInAuthObject | SignedOutAuthObject) & { | ||
| (): SignedInAuthObject | SignedOutAuthObject; | ||
| type AuthObjectHandler = SessionAuthObject & { | ||
| (): SessionAuthObject; |
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.
SessionAuthObject is a new type introduced in machine auth
see token types
📝 WalkthroughWalkthroughThis set of changes updates dependency versions in both the Nuxt and Vue package manifests, including Nuxt-related packages and several Vue development dependencies. TypeScript type definitions are refined across Nuxt and Vue source files: in Nuxt, a new optional property Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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
🧹 Nitpick comments (3)
packages/nuxt/src/global.d.ts (1)
11-15: Add brief JSDoc for the new runtime-config key
webhookSigningSecretlooks perfectly fine here, but since this file already serves as the single source of truth for downstream consumers, consider adding a one-liner JSDoc comment so IDEs show a short description / guidance on expected value (e.g. “HMAC secret for verifying Clerk webhook payloads”).packages/nuxt/package.json (1)
74-80: Validate patch bumps & semver rangesAll bumps stay within the same major versions – good.
Nit: we usually lock Nuxt ecosystem deps with~(patch-only) to avoid accidental minor jumps that may introduce new hooks / breaking changes. Switching^3.17.5→~3.17.5keeps reproducibility tighter:-"@nuxt/kit": "^3.17.5", -"@nuxt/schema": "^3.17.5", -"nuxt": "^3.17.5", +"@nuxt/kit": "~3.17.5", +"@nuxt/schema": "~3.17.5", +"nuxt": "~3.17.5",packages/nuxt/src/module.ts (1)
15-18:initialStateexclusion is 👍, but a defensive strip at runtime would be safer
initialStatecan potentially be a large, SSR-only payload. Even though the type system now prevents the option from being set, a consumer can still sneak it in with an explicit cast, and the unconditional spread at line 58 would push that data intopublic.runtimeConfig, bloating the client bundle.A tiny guard avoids the risk altogether:
- void updateRuntimeConfig({ + const { initialState: _ignored, ...safeOptions } = options; + void updateRuntimeConfig({ ... - ...options, + ...safeOptions,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
packages/nuxt/package.json(1 hunks)packages/nuxt/src/global.d.ts(1 hunks)packages/nuxt/src/module.ts(2 hunks)packages/nuxt/src/runtime/plugin.ts(2 hunks)packages/vue/package.json(1 hunks)packages/vue/src/plugin.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
packages/vue/src/plugin.ts (3)
packages/types/src/utils.ts (1)
Without(105-107)packages/types/src/multiDomain.ts (1)
MultiDomainAndOrProxy(11-35)packages/types/src/ssr.ts (1)
InitialState(11-25)
packages/nuxt/src/runtime/plugin.ts (1)
packages/types/src/ssr.ts (1)
InitialState(11-25)
packages/nuxt/src/module.ts (3)
packages/types/src/utils.ts (1)
Without(105-107)packages/vue/src/plugin.ts (1)
PluginOptions(10-13)packages/vue/src/index.ts (1)
PluginOptions(8-8)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Build Packages
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: semgrep/ci
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
packages/vue/package.json (1)
64-69: Major jump forunplugin-vue– double-check build pipeline
unplugin-vuemoved from5.x→6.xwhich contains a few breaking changes around transform hooks. Although it sits indevDependencies, it will affect TS/rollup/vite builds. Please make suretsup+vue-tscstill succeed and treeshaking results are identical.packages/nuxt/src/runtime/plugin.ts (1)
10-13: Great: explicit typing for shared SSR stateExplicitly annotating
useStatewithInitialState | undefinedremoves a whole class of “any” escapes in user land. Nice touch.packages/nuxt/src/module.ts (1)
112-116: Migration toSessionAuthObjectlooks correctThe H3 context now exposes a single, unified
SessionAuthObject—this simplifies downstream type juggling and matches the current@clerk/backendsurface. No issues spotted.
| export type PluginOptions = Without<LoadClerkJsScriptOptions, 'domain' | 'proxyUrl'> & | ||
| MultiDomainAndOrProxy & { | ||
| initialState?: InitialState; | ||
| }; |
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.
🛠️ Refactor suggestion
initialState leaks into loadClerkJsScript options
pluginOptions now carries initialState, but a few lines below we cast the full object to LoadClerkJsScriptOptions and forward it to loadClerkJsScript:
const options = {
...pluginOptions,
sdkMetadata: pluginOptions.sdkMetadata || SDK_METADATA,
} as LoadClerkJsScriptOptions;loadClerkJsScript has no concept of initialState; the extra key is silently ignored today but future strict‐mode checks (or external libs doing Object.keys) might fail.
A minimal, type-safe guard:
-const options = {
- ...pluginOptions,
- sdkMetadata: pluginOptions.sdkMetadata || SDK_METADATA,
-} as LoadClerkJsScriptOptions;
+const { initialState, ...loadOptions } = pluginOptions;
+const options: LoadClerkJsScriptOptions = {
+ ...loadOptions,
+ sdkMetadata: loadOptions.sdkMetadata || SDK_METADATA,
+};Keeps runtime payload clean and removes the need for the as cast.
🤖 Prompt for AI Agents
In packages/vue/src/plugin.ts around lines 10 to 13, the PluginOptions type
includes an initialState property that is not part of LoadClerkJsScriptOptions
but is being cast and passed to loadClerkJsScript, causing a type leak. To fix
this, explicitly separate initialState from pluginOptions before passing options
to loadClerkJsScript, ensuring only valid LoadClerkJsScriptOptions keys are
forwarded. Remove the type assertion cast and construct the options object
without initialState to keep the runtime payload clean and maintain type safety.
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/elements
@clerk/clerk-expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/clerk-react
@clerk/react-router
@clerk/remix
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/themes
@clerk/types
@clerk/upgrade
@clerk/vue
commit: |
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: 0
🧹 Nitpick comments (3)
packages/nextjs/src/__tests__/webhooks.test.ts (3)
16-24: Normalize header keys to avoid case-sensitivity pitfalls
IncomingHttpHeadersin Node/Next normalizes all keys to lowercase, whileRequest.headers.getis case-insensitive.
Your helper currently looks up the exact casing that was passed in; a mismatch (e.g.'Svix-ID') will silently fail and the test will throw for “missing headers” even though they exist.-const getHeader = (key: string) => { - if (request instanceof Request) { - return request.headers.get(key); - } - return request.headers?.[key]; -}; +const getHeader = (key: string) => { + const k = key.toLowerCase(); + if (request instanceof Request) { + return request.headers.get(k); + } + // handle both exact and already-lower-cased keys + return request.headers?.[k] ?? request.headers?.[key]; +};
27-29: Prefer a domain-specific error type overErrorThrowing a bare
Errorwith a hard-coded string makes the mock brittle and loses semantic meaning.
Consider re-using the real library’s error class (e.g.WebhookError) or defining a custom one so callers can assert viainstanceofinstead of string matching.
97-99: Relax exact string assertion to reduce brittlenessThe test couples tightly to the error message; any punctuation or wording change will break it while the behaviour remains correct. Matching a substring or regex is usually sufficient and future-proof:
-await expect(verifyWebhook(mockNextApiRequest)).rejects.toThrow( - 'Missing required Svix headers: svix-id, svix-timestamp, svix-signature', -); +await expect(verifyWebhook(mockNextApiRequest)).rejects.toThrow( + /Missing required Svix headers/, +);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (1)
packages/nextjs/src/__tests__/webhooks.test.ts(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/nextjs/src/__tests__/webhooks.test.ts (1)
packages/nextjs/src/webhooks.ts (1)
verifyWebhook(50-57)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: Build Packages
- GitHub Check: semgrep/ci
- GitHub Check: Analyze (javascript-typescript)
Description
initialStateprop publicChecklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
New Features
webhookSigningSecretproperty in runtime configuration for enhanced webhook security.Improvements
initialStateproperty.initialStateproperty publicly accessible for broader usage.