test(react-router): cover auth context isolation under concurrency#8889
test(react-router): cover auth context isolation under concurrency#8889nikosdouvlis wants to merge 1 commit into
Conversation
clerkMiddleware stores auth in the React Router request context and getAuth reads it back. Pin that a per-request context keeps auth isolated and that a shared context leaks one request's auth into another under concurrency.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@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: |
API Changes Report
Summary
No API Changes DetectedAll packages have stable APIs with no detected changes. Report generated by Break Check Last ran on |
📝 WalkthroughWalkthroughA new Vitest test file is added that verifies concurrency isolation for ChangesConcurrency Isolation Tests
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Warning Review ran into problems🔥 ProblemsStopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/react-router/src/server/__tests__/clerkMiddleware.concurrency.test.ts (1)
72-101: ⚡ Quick winConsider adding an explicit return type for clarity.
The
runInterleavedhelper returns a structured object used by multiple test cases. Adding an explicit return type would improve type safety and make the function's contract clearer.📝 Suggested return type annotation
-async function runInterleaved(contextFor: (req: Request) => RouterContextProvider) { +async function runInterleaved(contextFor: (req: Request) => RouterContextProvider): Promise<{ A?: string | null; B?: string | null }> {🤖 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/react-router/src/server/__tests__/clerkMiddleware.concurrency.test.ts` around lines 72 - 101, The runInterleaved function lacks an explicit return type annotation, which reduces type safety and clarity of its contract. Add a return type annotation to the function signature that reflects the Promise type it returns, which should be Promise with a structure containing optional properties A and B, both of type string or null. This will make the function's return type explicit and improve type checking for callers of this helper function.
🤖 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.
Nitpick comments:
In
`@packages/react-router/src/server/__tests__/clerkMiddleware.concurrency.test.ts`:
- Around line 72-101: The runInterleaved function lacks an explicit return type
annotation, which reduces type safety and clarity of its contract. Add a return
type annotation to the function signature that reflects the Promise type it
returns, which should be Promise with a structure containing optional properties
A and B, both of type string or null. This will make the function's return type
explicit and improve type checking for callers of this helper function.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Repository UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 5b3ca5f8-16b3-4a78-9d4e-bbf06e1c4f97
📒 Files selected for processing (1)
packages/react-router/src/server/__tests__/clerkMiddleware.concurrency.test.ts
Summary by CodeRabbit