chore(react-router): Pass options from middleware to clerkClient #7292
Conversation
🦋 Changeset detectedLatest commit: 6422dc3 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 |
|
@spegoraro is attempting to deploy a commit to the Clerk Production Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughclerkClient now accepts an optional Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant RouterMiddleware as Router Middleware
participant clerkClient as clerkClient
participant loadOptions as loadOptions
RouterMiddleware->>clerkClient: clerkClient(args, options)
rect rgba(76,175,80,0.08)
clerkClient->>loadOptions: loadOptions(args, overrides=options)
loadOptions-->>clerkClient: resolved config
clerkClient-->>RouterMiddleware: initialized client
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (32)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/react-router/src/server/clerkMiddleware.ts (1)
36-58: Consider refactoring to eliminate duplicateloadOptionscalls.The current implementation calls
loadOptions(args, options)twice:
- Line 36: to get options for
authenticateRequest- Line 58: inside
clerkClient, which callsloadOptionsagain (line 7 of clerkClient.ts)While this achieves the correct behavior, calling
loadOptionstwice with identical arguments is inefficient. Consider refactoring to callloadOptionsonce and reuse the result:const loadedOptions = loadOptions(args, options); // ... extract properties ... const requestState = await createClerkClient({ apiUrl: loadedOptions.apiUrl, secretKey: loadedOptions.secretKey, jwtKey: loadedOptions.jwtKey, proxyUrl: loadedOptions.proxyUrl, isSatellite: loadedOptions.isSatellite, domain: loadedOptions.domain, publishableKey: loadedOptions.publishableKey, machineSecretKey: loadedOptions.machineSecretKey, userAgent: `${PACKAGE_NAME}@${PACKAGE_VERSION}`, }).authenticateRequest(clerkRequest, { // ... authenticateRequest options ... });This would require
clerkClientto either accept pre-loaded options or be restructured, but it would eliminate redundant computation and validation.
📜 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 selected for processing (2)
packages/react-router/src/server/clerkClient.ts(1 hunks)packages/react-router/src/server/clerkMiddleware.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
packages/react-router/src/server/clerkClient.ts (2)
packages/react-router/src/server/loadOptions.ts (2)
DataFunctionArgs(14-14)loadOptions(16-90)packages/react-router/src/server/types.ts (1)
ClerkMiddlewareOptions(20-51)
packages/react-router/src/server/clerkMiddleware.ts (2)
packages/react-router/src/server/clerkClient.ts (1)
clerkClient(6-22)packages/react-router/src/server/index.ts (1)
clerkClient(5-5)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (2)
packages/react-router/src/server/clerkClient.ts (1)
4-7: LGTM! Options are now correctly forwarded.The addition of the
overridesparameter and its forwarding toloadOptionscorrectly implements the fix described in the PR objectives. The parameter is properly typed, has a sensible default, and maintains backward compatibility.packages/react-router/src/server/clerkMiddleware.ts (1)
58-58: Options are now correctly passed to clerkClient.The change fixes the reported bug by forwarding the
optionsparameter toclerkClient, ensuring that manually supplied configuration (e.g., secret keys attached to router context) is honored.
wobsoriano
changed the title
@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: |
4 participants
Description
When using the
clerkMiddlewarefunction with options, the original function correctly extracts them usingloadOptionsbut then callsclerkClientwithargsonly and doesn't pass on the options. In the case where options are passed in manually (e.g. using react-router middleware) this led to incorrect behaviour.For my specific use-case, I was attaching environment variables to the router context then pulling those in the middleware function to pass on to
clerkMiddleware. Despite passing my secret key, this would still fail. Now, by passing the options on toclerkClientit works correctly.Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.