-
Notifications
You must be signed in to change notification settings - Fork 382
fix(tanstack-react-start): Handle serialization issue when doing handshake #6345
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
fix(tanstack-react-start): Handle serialization issue when doing handshake #6345
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
🦋 Changeset detectedLatest commit: f813e9f 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 |
📝 WalkthroughWalkthroughA new changeset file was added to document a patch update for the 📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous 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). (5)
🪧 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: 0
🧹 Nitpick comments (1)
packages/tanstack-react-start/src/server/errors.ts (1)
1-11
: LGTM! Well-implemented custom error class.The
ClerkHandshakeRedirect
error class follows TypeScript best practices for custom errors. It properly extendsError
, sets the error name, and encapsulates the necessary redirect information.Minor optimization: The property assignments on lines 8-9 are redundant since
status
andheaders
are already declared as public constructor parameters.export class ClerkHandshakeRedirect extends Error { constructor( public status: number, public headers: Headers, ) { super('Clerk handshake redirect required'); this.name = 'ClerkHandshakeRedirect'; - this.status = status; - this.headers = headers; } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.changeset/fifty-buses-fix.md
(1 hunks)packages/tanstack-react-start/src/server/authenticateRequest.ts
(2 hunks)packages/tanstack-react-start/src/server/errors.ts
(1 hunks)packages/tanstack-react-start/src/server/middlewareHandler.ts
(2 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,jsx,ts,tsx}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/development.mdc
packages/**/*.ts
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/development.mdc
packages/**/*.{ts,tsx,d.ts}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/development.mdc
**/*.{ts,tsx}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/development.mdc
- .cursor/rules/typescript.mdc
**/*
Instructions used from:
Sources:
⚙️ CodeRabbit Configuration File
🧠 Learnings (5)
📓 Common learnings
Learnt from: dstaley
PR: clerk/javascript#6116
File: .changeset/tangy-garlics-say.md:1-2
Timestamp: 2025-06-13T16:09:53.061Z
Learning: In the Clerk JavaScript repository, contributors create intentionally empty changeset files (containing only the YAML delimiters) when a PR touches only non-published parts of the codebase (e.g., sandbox assets). This signals that no package release is required, so such changesets should not be flagged as missing content.
Learnt from: jacekradko
PR: clerk/javascript#5905
File: .changeset/six-ears-wash.md:1-3
Timestamp: 2025-06-26T03:27:05.535Z
Learning: In the Clerk JavaScript repository, changeset headers support single quotes syntax (e.g., '@clerk/backend': minor) and work fine with their current changesets integration, so there's no need to change them to double quotes.
Learnt from: wobsoriano
PR: clerk/javascript#5858
File: packages/clerk-js/src/core/modules/apiKeys/index.ts:84-97
Timestamp: 2025-06-10T17:35:08.986Z
Learning: In the APIKeys service methods (packages/clerk-js/src/core/modules/apiKeys/index.ts), error handling is intentionally delegated to the component level rather than being implemented within the service methods themselves. This architectural pattern allows calling components to handle errors according to their specific UI needs.
Learnt from: wobsoriano
PR: clerk/javascript#6163
File: packages/backend/src/api/endpoints/APIKeysApi.ts:60-70
Timestamp: 2025-06-20T17:44:17.570Z
Learning: The Clerk codebase uses POST method for API key update operations instead of the typical PATCH method, as clarified by the maintainer wobsoriano.
Learnt from: panteliselef
PR: clerk/javascript#6285
File: packages/types/src/commerce.ts:1305-1305
Timestamp: 2025-07-11T18:08:14.697Z
Learning: In the Clerk JavaScript repository, when there's a conflict between naming consistency (camelCase) and avoiding breaking changes, the team prioritizes maintaining backward compatibility over enforcing naming conventions, even for experimental APIs.
Learnt from: LauraBeatris
PR: clerk/javascript#6273
File: packages/testing/src/playwright/unstable/page-objects/sessionTask.ts:22-27
Timestamp: 2025-07-11T17:12:28.495Z
Learning: In Clerk's test utilities, particularly for page objects like sessionTask.ts, when handling task type enums, prefer graceful handling of unknown/unsupported task types rather than throwing errors. This prevents breaking existing tests when new task types are introduced to the SessionTask['key'] union type.
Learnt from: panteliselef
PR: clerk/javascript#6327
File: .changeset/eight-socks-lead.md:2-4
Timestamp: 2025-07-16T10:43:17.706Z
Learning: In the Clerk JavaScript repository, APIs marked with @experimental JSDoc annotations can have breaking changes released with minor version bumps rather than major version bumps, as consumers are warned about the instability of experimental features.
.changeset/fifty-buses-fix.md (5)
Learnt from: dstaley
PR: clerk/javascript#6116
File: .changeset/tangy-garlics-say.md:1-2
Timestamp: 2025-06-13T16:09:53.061Z
Learning: In the Clerk JavaScript repository, contributors create intentionally empty changeset files (containing only the YAML delimiters) when a PR touches only non-published parts of the codebase (e.g., sandbox assets). This signals that no package release is required, so such changesets should not be flagged as missing content.
Learnt from: jacekradko
PR: clerk/javascript#5905
File: .changeset/six-ears-wash.md:1-3
Timestamp: 2025-06-26T03:27:05.535Z
Learning: In the Clerk JavaScript repository, changeset headers support single quotes syntax (e.g., '@clerk/backend': minor) and work fine with their current changesets integration, so there's no need to change them to double quotes.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to .changeset/config.json : Automated releases must be managed with Changesets.
Learnt from: wobsoriano
PR: clerk/javascript#6163
File: packages/backend/src/api/endpoints/APIKeysApi.ts:60-70
Timestamp: 2025-06-20T17:44:17.570Z
Learning: The Clerk codebase uses POST method for API key update operations instead of the typical PATCH method, as clarified by the maintainer wobsoriano.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-06-30T10:32:37.848Z
Learning: Applies to **/*.{jsx,tsx} : Implement proper error recovery
packages/tanstack-react-start/src/server/middlewareHandler.ts (12)
Learnt from: wobsoriano
PR: clerk/javascript#5858
File: packages/clerk-js/src/core/modules/apiKeys/index.ts:84-97
Timestamp: 2025-06-10T17:35:08.986Z
Learning: In the APIKeys service methods (packages/clerk-js/src/core/modules/apiKeys/index.ts), error handling is intentionally delegated to the component level rather than being implemented within the service methods themselves. This architectural pattern allows calling components to handle errors according to their specific UI needs.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/nextjs.mdc:0-0
Timestamp: 2025-06-30T10:31:43.578Z
Learning: Applies to app/**/*.tsx : Use redirect function for server-side redirects in Server Components
Learnt from: LauraBeatris
PR: clerk/javascript#6273
File: packages/testing/src/playwright/unstable/page-objects/sessionTask.ts:22-27
Timestamp: 2025-07-11T17:12:28.495Z
Learning: In Clerk's test utilities, particularly for page objects like sessionTask.ts, when handling task type enums, prefer graceful handling of unknown/unsupported task types rather than throwing errors. This prevents breaking existing tests when new task types are introduced to the SessionTask['key'] union type.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/typescript.mdc:0-0
Timestamp: 2025-06-30T10:33:45.961Z
Learning: Applies to **/*.{ts,tsx} : Proper error handling with typed errors
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/nextjs.mdc:0-0
Timestamp: 2025-06-30T10:31:43.578Z
Learning: Applies to {middleware.ts,src/middleware.ts} : Implement middleware for route-level authentication
Learnt from: LauraBeatris
PR: clerk/javascript#6117
File: packages/clerk-js/src/ui/components/SessionTasks/tasks/ForceOrganizationSelection.tsx:17-21
Timestamp: 2025-06-18T23:27:13.537Z
Learning: In Clerk's JavaScript codebase, query errors from hooks like useOrganizationList are not typically handled in AIO (All-in-One) components. Error handling may be connected with the Card context instead, suggesting a centralized error handling approach rather than component-level error handling.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-06-30T10:32:37.848Z
Learning: Applies to **/*.tsx : Use proper event types for handlers
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/nextjs.mdc:0-0
Timestamp: 2025-06-30T10:31:43.578Z
Learning: Applies to {middleware.ts,src/middleware.ts} : Place middleware.ts in project root or src directory
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/typescript.mdc:0-0
Timestamp: 2025-06-30T10:33:45.961Z
Learning: Applies to **/*.{ts,tsx} : Define custom error classes for domain-specific errors
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/nextjs.mdc:0-0
Timestamp: 2025-06-30T10:31:43.578Z
Learning: Applies to app/**/error.tsx : Use error.tsx for error boundaries at the route level in the App Router
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/nextjs.mdc:0-0
Timestamp: 2025-06-30T10:31:43.578Z
Learning: Applies to app/**/*.tsx : Add 'use client' directive at the top of files that need client-side features
Learnt from: LauraBeatris
PR: clerk/javascript#6117
File: packages/clerk-js/src/ui/components/SessionTasks/index.tsx:84-88
Timestamp: 2025-06-18T16:32:03.760Z
Learning: In the Clerk JavaScript codebase, navigation errors from `navigate` and `__experimental_navigateToTask` are generally not caught and handled at the call site. The `navigateToTask` method primarily performs navigation and `session.reload` (GET request), and these operations don't require special error surfacing in the UI components.
packages/tanstack-react-start/src/server/authenticateRequest.ts (8)
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/nextjs.mdc:0-0
Timestamp: 2025-06-30T10:31:43.578Z
Learning: Applies to app/**/*.tsx : Use redirect function for server-side redirects in Server Components
Learnt from: wobsoriano
PR: clerk/javascript#5858
File: packages/clerk-js/src/core/modules/apiKeys/index.ts:84-97
Timestamp: 2025-06-10T17:35:08.986Z
Learning: In the APIKeys service methods (packages/clerk-js/src/core/modules/apiKeys/index.ts), error handling is intentionally delegated to the component level rather than being implemented within the service methods themselves. This architectural pattern allows calling components to handle errors according to their specific UI needs.
Learnt from: wobsoriano
PR: clerk/javascript#6123
File: packages/nextjs/src/server/__tests__/getAuthDataFromRequest.test.ts:63-75
Timestamp: 2025-06-16T01:27:54.563Z
Learning: In packages/nextjs/src/server/data/getAuthDataFromRequest.ts, the tokenType behavior on mismatch is intentionally different between array and single acceptsToken values: when acceptsToken is an array and the token type doesn't match any in the array, tokenType returns null; when acceptsToken is a single value and the token type doesn't match, tokenType returns the requested single value. This design aligns with developer intent and provides a more ergonomic API for common use cases.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/nextjs.mdc:0-0
Timestamp: 2025-06-30T10:31:43.578Z
Learning: Applies to {middleware.ts,src/middleware.ts} : Implement middleware for route-level authentication
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/typescript.mdc:0-0
Timestamp: 2025-06-30T10:33:45.961Z
Learning: Applies to **/*.{ts,tsx} : Define custom error classes for domain-specific errors
Learnt from: LauraBeatris
PR: clerk/javascript#6273
File: packages/testing/src/playwright/unstable/page-objects/sessionTask.ts:22-27
Timestamp: 2025-07-11T17:12:28.495Z
Learning: In Clerk's test utilities, particularly for page objects like sessionTask.ts, when handling task type enums, prefer graceful handling of unknown/unsupported task types rather than throwing errors. This prevents breaking existing tests when new task types are introduced to the SessionTask['key'] union type.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/typescript.mdc:0-0
Timestamp: 2025-06-30T10:33:45.961Z
Learning: Applies to **/*.{ts,tsx} : Proper error handling with typed errors
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Applies to **/*.{ts,tsx} : Use proper TypeScript error types
packages/tanstack-react-start/src/server/errors.ts (9)
Learnt from: wobsoriano
PR: clerk/javascript#5858
File: packages/clerk-js/src/core/modules/apiKeys/index.ts:84-97
Timestamp: 2025-06-10T17:35:08.986Z
Learning: In the APIKeys service methods (packages/clerk-js/src/core/modules/apiKeys/index.ts), error handling is intentionally delegated to the component level rather than being implemented within the service methods themselves. This architectural pattern allows calling components to handle errors according to their specific UI needs.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/typescript.mdc:0-0
Timestamp: 2025-06-30T10:33:45.961Z
Learning: Applies to **/*.{ts,tsx} : Define custom error classes for domain-specific errors
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/nextjs.mdc:0-0
Timestamp: 2025-06-30T10:31:43.578Z
Learning: Applies to app/**/*.tsx : Use redirect function for server-side redirects in Server Components
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/typescript.mdc:0-0
Timestamp: 2025-06-30T10:33:45.961Z
Learning: Applies to **/*.{ts,tsx} : Proper error handling with typed errors
Learnt from: wobsoriano
PR: clerk/javascript#6229
File: packages/backend/src/api/endpoints/MachineTokensApi.ts:47-89
Timestamp: 2025-07-01T15:20:41.834Z
Learning: In the Clerk JavaScript repository, for the MachineTokensApi class (packages/backend/src/api/endpoints/MachineTokensApi.ts), the maintainers prefer to rely on TypeScript types and readable property names for API documentation rather than JSDoc comments.
Learnt from: LauraBeatris
PR: clerk/javascript#6273
File: packages/testing/src/playwright/unstable/page-objects/sessionTask.ts:22-27
Timestamp: 2025-07-11T17:12:28.495Z
Learning: In Clerk's test utilities, particularly for page objects like sessionTask.ts, when handling task type enums, prefer graceful handling of unknown/unsupported task types rather than throwing errors. This prevents breaking existing tests when new task types are introduced to the SessionTask['key'] union type.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Applies to **/*.{ts,tsx} : Use proper TypeScript error types
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/nextjs.mdc:0-0
Timestamp: 2025-06-30T10:31:43.578Z
Learning: Applies to app/**/error.tsx : Use error.tsx for error boundaries at the route level in the App Router
Learnt from: LauraBeatris
PR: clerk/javascript#6117
File: packages/clerk-js/src/ui/components/SessionTasks/tasks/ForceOrganizationSelection.tsx:17-21
Timestamp: 2025-06-18T23:27:13.537Z
Learning: In Clerk's JavaScript codebase, query errors from hooks like useOrganizationList are not typically handled in AIO (All-in-One) components. Error handling may be connected with the Card context instead, suggesting a centralized error handling approach rather than component-level error handling.
🧬 Code Graph Analysis (2)
packages/tanstack-react-start/src/server/middlewareHandler.ts (1)
packages/tanstack-react-start/src/server/errors.ts (1)
ClerkHandshakeRedirect
(1-11)
packages/tanstack-react-start/src/server/authenticateRequest.ts (1)
packages/tanstack-react-start/src/server/errors.ts (1)
ClerkHandshakeRedirect
(1-11)
⏰ 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: Analyze (javascript-typescript)
🔇 Additional comments (5)
.changeset/fifty-buses-fix.md (1)
1-6
: LGTM! Changeset follows proper format.The changeset correctly documents a patch release for the serialization fix with appropriate version type and descriptive message.
packages/tanstack-react-start/src/server/middlewareHandler.ts (2)
5-5
: LGTM! Proper import of the new error class.The import correctly brings in the
ClerkHandshakeRedirect
error class for use in the error handling logic.
37-43
: LGTM! Improved error handling with typed errors.The error handling now properly catches the specific
ClerkHandshakeRedirect
error type and constructs aResponse
with the appropriate status and headers. This is more type-safe and explicit than handling genericResponse
objects.Security Note: This change affects authentication middleware logic. @clerk/security - Please review this change as it modifies the handshake redirect mechanism in the authentication flow.
packages/tanstack-react-start/src/server/authenticateRequest.ts (2)
7-7
: LGTM! Proper import of the new error class.The import correctly brings in the
ClerkHandshakeRedirect
error class for the authentication flow.
47-47
: LGTM! Improved error handling with typed errors.The change from throwing a generic
Response
object to throwing the specificClerkHandshakeRedirect
error is a good improvement. This provides better type safety and prevents serialization issues with Seroval.Security Note: This change affects authentication request handling. @clerk/security - Please review this change as it modifies how handshake redirects are handled in the authentication flow.
@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: 1
🧹 Nitpick comments (4)
integration/templates/tanstack-react-start/src/routes/user.tsx (3)
6-15
: Consider improving error handling and type safety.The server function implementation is correct, but could benefit from:
- More descriptive error handling: The current error message is generic
- Type safety: Add explicit return type annotation
- Robust error handling: Consider cases where
getAuth
might fail-const fetchClerkAuth = createServerFn({ method: 'GET' }).handler(async () => { +const fetchClerkAuth = createServerFn({ method: 'GET' }).handler(async (): Promise<{ userId: string | null }> => { const request = getWebRequest(); - if (!request) throw new Error('No request found'); + if (!request) throw new Error('Failed to retrieve web request in server context'); - const { userId } = await getAuth(request); + try { + const { userId } = await getAuth(request); + return { userId }; + } catch (error) { + console.error('Failed to get auth:', error); + return { userId: null }; + } - - return { - userId, - }; });
25-29
: Improve type safety and component structure.The component logic is correct, but could benefit from better TypeScript typing and structure.
+interface LoaderData { + userId: string | null; +} + -function Page() { +function UserPage() { - const state = Route.useLoaderData(); + const state = Route.useLoaderData() as LoaderData; return state.userId ? <h1>Welcome! Your ID is {state.userId}!</h1> : <h1>You are not signed in</h1>; } export const Route = createFileRoute('/')({ - component: Page, + component: UserPage, // ... rest of configuration });
17-17
: Consider the route path choice for integration templates.Using the root path
'/'
in an integration template might conflict with existing application routes. Consider using a more specific path like'/user'
to avoid conflicts and make the purpose clearer.-export const Route = createFileRoute('/')({ +export const Route = createFileRoute('/user')({This would also align better with the test that navigates to
/user
as seen in the integration test file.integration/tests/tanstack-start/basic.test.ts (1)
77-94
: Excellent integration test for server-side authentication!This test comprehensively verifies that
getAuth()
works correctly inside server functions, which directly addresses the serialization issue mentioned in the PR objectives. The test follows Playwright best practices and properly tests both authentication states.Minor suggestions for robustness:
test('getAuth() works inside server functions', async ({ page, context }) => { const u = createTestUtils({ app, page, context }); await u.page.goToRelative('/user'); - await expect(u.page.getByText('You are not signed in')).toBeVisible(); + await expect(u.page.getByText('You are not signed in')).toBeVisible({ timeout: 10000 }); await u.po.signIn.goTo(); await u.po.signIn.setIdentifier(fakeUser.email); await u.po.signIn.setPassword(fakeUser.password); await u.po.signIn.continue(); await u.po.expect.toBeSignedIn(); await u.page.goToRelative('/user'); - await expect(u.page.getByText(`Welcome! Your ID is ${bapiUser.id}!`)).toBeVisible(); + await expect(u.page.getByText(`Welcome! Your ID is ${bapiUser.id}!`)).toBeVisible({ timeout: 10000 }); });Adding explicit timeouts can help with flaky tests in CI environments.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
integration/templates/tanstack-react-router/package.json
(1 hunks)integration/templates/tanstack-react-start/package.json
(1 hunks)integration/templates/tanstack-react-start/src/routes/user.tsx
(1 hunks)integration/tests/tanstack-start/basic.test.ts
(4 hunks)
✅ Files skipped from review due to trivial changes (2)
- integration/templates/tanstack-react-router/package.json
- integration/templates/tanstack-react-start/package.json
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{js,jsx,ts,tsx}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/development.mdc
**/*.{ts,tsx}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/development.mdc
- .cursor/rules/typescript.mdc
integration/**/*
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/monorepo.mdc
**/*.{test,spec}.{js,ts,tsx}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/monorepo.mdc
integration/**/*.{test,spec}.{js,ts,tsx}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/monorepo.mdc
**/*
Instructions used from:
Sources:
⚙️ CodeRabbit Configuration File
**/*.{tsx,jsx}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/development.mdc
**/*.{jsx,tsx}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/react.mdc
**/*.tsx
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/react.mdc
🧠 Learnings (3)
📓 Common learnings
Learnt from: dstaley
PR: clerk/javascript#6116
File: .changeset/tangy-garlics-say.md:1-2
Timestamp: 2025-06-13T16:09:53.061Z
Learning: In the Clerk JavaScript repository, contributors create intentionally empty changeset files (containing only the YAML delimiters) when a PR touches only non-published parts of the codebase (e.g., sandbox assets). This signals that no package release is required, so such changesets should not be flagged as missing content.
Learnt from: wobsoriano
PR: clerk/javascript#5858
File: packages/clerk-js/src/core/modules/apiKeys/index.ts:84-97
Timestamp: 2025-06-10T17:35:08.986Z
Learning: In the APIKeys service methods (packages/clerk-js/src/core/modules/apiKeys/index.ts), error handling is intentionally delegated to the component level rather than being implemented within the service methods themselves. This architectural pattern allows calling components to handle errors according to their specific UI needs.
Learnt from: jacekradko
PR: clerk/javascript#5905
File: .changeset/six-ears-wash.md:1-3
Timestamp: 2025-06-26T03:27:05.535Z
Learning: In the Clerk JavaScript repository, changeset headers support single quotes syntax (e.g., '@clerk/backend': minor) and work fine with their current changesets integration, so there's no need to change them to double quotes.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to packages/clerk-react/**/*.{test,spec}.{js,ts,tsx} : Component testing should use React Testing Library.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/nextjs.mdc:0-0
Timestamp: 2025-06-30T10:31:43.578Z
Learning: Applies to app/**/*.tsx : Use redirect function for server-side redirects in Server Components
Learnt from: wobsoriano
PR: clerk/javascript#6123
File: packages/nextjs/src/server/__tests__/getAuthDataFromRequest.test.ts:63-75
Timestamp: 2025-06-16T01:27:54.563Z
Learning: In packages/nextjs/src/server/data/getAuthDataFromRequest.ts, the tokenType behavior on mismatch is intentionally different between array and single acceptsToken values: when acceptsToken is an array and the token type doesn't match any in the array, tokenType returns null; when acceptsToken is a single value and the token type doesn't match, tokenType returns the requested single value. This design aligns with developer intent and provides a more ergonomic API for common use cases.
Learnt from: wobsoriano
PR: clerk/javascript#6163
File: packages/backend/src/api/endpoints/APIKeysApi.ts:60-70
Timestamp: 2025-06-20T17:44:17.570Z
Learning: The Clerk codebase uses POST method for API key update operations instead of the typical PATCH method, as clarified by the maintainer wobsoriano.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/nextjs.mdc:0-0
Timestamp: 2025-06-30T10:31:43.578Z
Learning: Applies to {middleware.ts,src/middleware.ts} : Implement middleware for route-level authentication
Learnt from: panteliselef
PR: clerk/javascript#6327
File: .changeset/eight-socks-lead.md:2-4
Timestamp: 2025-07-16T10:43:17.706Z
Learning: In the Clerk JavaScript repository, APIs marked with @experimental JSDoc annotations can have breaking changes released with minor version bumps rather than major version bumps, as consumers are warned about the instability of experimental features.
integration/tests/tanstack-start/basic.test.ts (18)
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/typescript.mdc:0-0
Timestamp: 2025-06-30T10:33:45.961Z
Learning: Applies to **/__tests__/**/*.{ts,tsx} : Use branded types for test isolation
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/typescript.mdc:0-0
Timestamp: 2025-06-30T10:33:45.961Z
Learning: Applies to **/__tests__/**/*.{ts,tsx} : Create type-safe test builders/factories
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/typescript.mdc:0-0
Timestamp: 2025-06-30T10:33:45.961Z
Learning: Applies to **/__tests__/**/*.{ts,tsx} : Implement proper mock types that match interfaces in tests
Learnt from: LauraBeatris
PR: clerk/javascript#6273
File: packages/testing/src/playwright/unstable/page-objects/sessionTask.ts:22-27
Timestamp: 2025-07-11T17:12:28.495Z
Learning: In Clerk's test utilities, particularly for page objects like sessionTask.ts, when handling task type enums, prefer graceful handling of unknown/unsupported task types rather than throwing errors. This prevents breaking existing tests when new task types are introduced to the SessionTask['key'] union type.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Test authentication flows end-to-end
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/typescript.mdc:0-0
Timestamp: 2025-06-30T10:33:45.961Z
Learning: Applies to **/__tests__/**/*.{ts,tsx} : Use Vitest for type-safe testing in TypeScript
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-06-30T10:32:37.848Z
Learning: Applies to **/*.test.{jsx,tsx} : Implement proper test setup
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to integration/**/*.{test,spec}.{js,ts,tsx} : Integration tests should use Playwright as the test runner.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Integration tests using Playwright for E2E scenarios
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to packages/clerk-react/**/*.{test,spec}.{js,ts,tsx} : Component testing should use React Testing Library.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-06-30T10:32:37.848Z
Learning: Applies to **/*.test.{jsx,tsx} : Use proper test data
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/monorepo.mdc:0-0
Timestamp: 2025-06-30T10:30:56.197Z
Learning: Applies to packages/@clerk/*/jest.config.{js,ts} : Each framework integration package must have its own test configuration.
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-06-30T10:32:37.848Z
Learning: Applies to **/*.test.{jsx,tsx} : Implement proper test isolation
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-06-30T10:32:37.848Z
Learning: Applies to **/*.test.{jsx,tsx} : Use proper test cleanup
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-06-30T10:32:37.848Z
Learning: Applies to **/*.test.{jsx,tsx} : Implement proper test assertions
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: Use real Clerk instances for integration tests
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-06-30T10:32:37.848Z
Learning: Applies to **/*.test.{jsx,tsx} : Test component interactions
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2025-06-30T10:29:42.997Z
Learning: React Testing Library for component testing
integration/templates/tanstack-react-start/src/routes/user.tsx (12)
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/nextjs.mdc:0-0
Timestamp: 2025-06-30T10:31:43.578Z
Learning: Applies to app/**/page.tsx : Use page.tsx for route segments that render UI in the App Router
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/nextjs.mdc:0-0
Timestamp: 2025-06-30T10:31:43.578Z
Learning: Applies to app/**/default.tsx : Use default.tsx for parallel routes fallbacks in the App Router
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/nextjs.mdc:0-0
Timestamp: 2025-06-30T10:31:43.578Z
Learning: Applies to app/**/*.tsx : Use useRouter hook for programmatic navigation in Client Components
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/nextjs.mdc:0-0
Timestamp: 2025-06-30T10:31:43.578Z
Learning: Applies to app/**/error.tsx : Use error.tsx for error boundaries at the route level in the App Router
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/nextjs.mdc:0-0
Timestamp: 2025-06-30T10:31:43.578Z
Learning: Applies to pages/**/*.{js,ts,jsx,tsx} : Use SWR for client-side data fetching in Pages Router
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/nextjs.mdc:0-0
Timestamp: 2025-06-30T10:31:43.578Z
Learning: Applies to {middleware.ts,src/middleware.ts} : Implement middleware for route-level authentication
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/nextjs.mdc:0-0
Timestamp: 2025-06-30T10:31:43.578Z
Learning: Applies to app/**/layout.tsx : Use layout.tsx for shared UI that wraps multiple pages in the App Router
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/nextjs.mdc:0-0
Timestamp: 2025-06-30T10:31:43.578Z
Learning: Applies to app/**/*.tsx : Use redirect function for server-side redirects in Server Components
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/nextjs.mdc:0-0
Timestamp: 2025-06-30T10:31:43.578Z
Learning: Applies to pages/**/*.{js,ts,jsx,tsx} : Implement ISR (Incremental Static Regeneration) with revalidate option in Pages Router
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/nextjs.mdc:0-0
Timestamp: 2025-06-30T10:31:43.578Z
Learning: Applies to app/**/not-found.tsx : Use not-found.tsx for custom 404 pages in the App Router
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/nextjs.mdc:0-0
Timestamp: 2025-06-30T10:31:43.578Z
Learning: Applies to pages/**/*.{js,ts,jsx,tsx} : Use getServerSideProps for server-side rendering on each request in Pages Router
Learnt from: CR
PR: clerk/javascript#0
File: .cursor/rules/nextjs.mdc:0-0
Timestamp: 2025-06-30T10:31:43.578Z
Learning: Applies to pages/**/*.{js,ts,jsx,tsx} : Use getStaticPaths with getStaticProps for dynamic static generation in Pages Router
⏰ 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). (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 (4)
integration/templates/tanstack-react-start/src/routes/user.tsx (1)
1-4
: LGTM! Imports are well-structured and appropriate.The imports correctly bring in the necessary TanStack and Clerk dependencies for server-side authentication functionality.
integration/tests/tanstack-start/basic.test.ts (3)
1-1
: LGTM! Type import follows best practices.The type-only import for
User
from@clerk/backend
is correctly added to support the new test functionality.
20-20
: LGTM! Proper user data capture and typing.The addition of the
bapiUser
variable with proper typing and assignment fromcreateBapiUser
result is excellent. This follows integration testing best practices and enables verification of the user ID in the new test.Also applies to: 29-29
77-94
: Critical test for validating the serialization fix.This test is particularly important because it validates that the
ClerkHandshakeRedirect
error handling introduced in this PR works correctly in practice. By testing server-side authentication end-to-end, it ensures that the serialization issues withResponse
objects in TanStack Start v1.127+ are properly resolved.The test structure follows integration testing best practices and will help prevent regressions of the serialization issue addressed in USER-2383.
export const Route = createFileRoute('/')({ | ||
component: Page, | ||
beforeLoad: async () => await fetchClerkAuth(), | ||
loader: async ({ context }) => { | ||
return { userId: context.userId }; | ||
}, | ||
}); |
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.
Fix the route configuration logic.
The route configuration has several critical issues:
- Incorrect beforeLoad implementation: The
beforeLoad
hook should return data that becomes available in the context - Missing error handling: No handling for failed authentication
- Type safety: The loader assumes
context.userId
exists without validation
export const Route = createFileRoute('/')({
component: Page,
- beforeLoad: async () => await fetchClerkAuth(),
+ beforeLoad: async () => {
+ const authData = await fetchClerkAuth();
+ return authData;
+ },
loader: async ({ context }) => {
- return { userId: context.userId };
+ return { userId: (context as any).userId || null };
},
});
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
export const Route = createFileRoute('/')({ | |
component: Page, | |
beforeLoad: async () => await fetchClerkAuth(), | |
loader: async ({ context }) => { | |
return { userId: context.userId }; | |
}, | |
}); | |
export const Route = createFileRoute('/')({ | |
component: Page, | |
beforeLoad: async () => { | |
const authData = await fetchClerkAuth(); | |
return authData; | |
}, | |
loader: async ({ context }) => { | |
return { userId: (context as any).userId || null }; | |
}, | |
}); |
🤖 Prompt for AI Agents
In integration/templates/tanstack-react-start/src/routes/user.tsx around lines
17 to 23, fix the route configuration by updating beforeLoad to return the
authentication data so it becomes part of the context, add error handling to
manage failed authentication cases gracefully, and ensure the loader validates
the presence of userId in the context before accessing it to maintain type
safety and avoid runtime errors.
Description
Starting with TanStack Start 1.127.0, users reported issues when using the Clerk TanStack SDK:
This is due to the addition of seroval, which serializes server state and sends it to the client.
Right now we throw a
Response
object in authenticateRequest() and catches it in the middleware handler to handle the handshake redirect. For some reason, theResponse
object end up in Seroval's serialization path.This PR replaces the
Response
throw mechanism with a custom error class that avoids the Seroval serialization issue.Related:
Resolves USER-2383
Checklist
pnpm test
runs as expected.pnpm build
runs as expected.Type of change
Summary by CodeRabbit
Bug Fixes
New Features
Chores