-
Notifications
You must be signed in to change notification settings - Fork 359
chore(tanstack-react-start): WIP WIP Remove vinxi dependency #5973
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: 3569aaf 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 |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
00090d3
to
76d48ef
Compare
📝 Walkthrough## Walkthrough
This update restructures the TanStack React Start template, replacing the previous Vinxi-based setup with a Vite-based configuration. It removes the old `app` directory, moving source files to `src`, and introduces new route, server, and style files. Multiple configuration and support files are deleted or replaced, and dependencies are updated to newer TanStack and related package versions.
## Changes
| Files/Groups | Change Summary |
|----------------------------------------------------------------------------------------------------------------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| integration/presets/tanstack.ts | Reordered properties in exported `tanstack` object: `reactRouter` now precedes `reactStart`. |
| integration/templates/tanstack-react-router/src/main.tsx | Added `scrollRestoration: true` to router configuration. |
| integration/templates/tanstack-react-router/src/posts.tsx | Deleted file: removed exported type `PostType` and async functions `fetchPost` and `fetchPosts`. |
| integration/templates/tanstack-react-router/src/routeTree.gen.ts | Simplified and refactored: reduced to only root, sign-in, and index routes; removed nested structures and extensive type declarations. |
| integration/templates/tanstack-react-router/src/routes/__root.tsx | Modified `RootComponent` to add container and header `<div>`s around `<Outlet />`. |
| integration/templates/tanstack-react-router/src/routes/_layout.tsx | Deleted layout route component and its export. |
| integration/templates/tanstack-react-router/tsconfig.json | Added `"lib"` (DOM, DOM.Iterable, ES2022) and `"skipLibCheck": true` options. |
| integration/templates/tanstack-react-router/vite.config.js | Changed import and usage of TanStack Router Vite plugin; now uses `tanstackRouter({ target: 'react', autoCodeSplitting: true })`. |
| integration/templates/tanstack-react-router/package.json | Updated TanStack dependencies to newer caret versions; removed `@tanstack/router-generator`. |
| integration/templates/tanstack-react-start/.gitignore | Replaced `.vinxi` with `.nitro`, joined comment to `/public/build`, and added `.tanstack`, Playwright cache/report directories. |
| integration/templates/tanstack-react-start/.prettierignore<br/>.prettierrc | Deleted Prettier ignore and configuration files. |
| integration/templates/tanstack-react-start/app.config.ts | Deleted Vite config file using TypeScript paths plugin. |
| integration/templates/tanstack-react-start/app/client.tsx | Deleted client-side hydration entry file. |
| integration/templates/tanstack-react-start/app/components/DefaultCatchBoundary.tsx<br/>NotFound.tsx | Deleted error boundary and not-found React components. |
| integration/templates/tanstack-react-start/app/routeTree.gen.ts | Deleted auto-generated route tree/types module. |
| integration/templates/tanstack-react-start/app/router.tsx | Deleted router creation function and type augmentation. |
| integration/templates/tanstack-react-start/app/routes/__root.tsx | Deleted root route and root document components. |
| integration/templates/tanstack-react-start/app/routes/_layout.tsx | Deleted layout route component and its export. |
| integration/templates/tanstack-react-start/app/routes/sign-in.tsx | Deleted sign-in route component and its export. |
| integration/templates/tanstack-react-start/app/ssr.tsx | Deleted SSR handler setup with Clerk integration. |
| integration/templates/tanstack-react-start/app/styles/app.css | Deleted Tailwind CSS base/app styles file. |
| integration/templates/tanstack-react-start/package.json | Switched scripts from Vinxi to Vite; updated TanStack and related dependencies; removed Vinxi, Prettier, PostCSS, and Autoprefixer; added Tailwind Vite plugin and updated type dependencies. |
| integration/templates/tanstack-react-start/postcss.config.cjs | Deleted PostCSS plugins configuration file. |
| integration/templates/tanstack-react-start/tailwind.config.cjs | Deleted Tailwind CSS configuration file. |
| integration/templates/tanstack-react-start/tsconfig.json | Changed `"~/*"` path alias from `app` to `src`; included `public/script*.js` in compilation. |
| integration/templates/tanstack-react-start/src/routeTree.gen.ts | Added new auto-generated route tree/types module for new `src` structure. |
| integration/templates/tanstack-react-start/src/router.tsx | Added new router creation module with type augmentation. |
| integration/templates/tanstack-react-start/src/routes/__root.tsx | Added new root route module with `RootComponent` and `RootDocument` using Clerk and TanStack devtools. |
| integration/templates/tanstack-react-start/src/routes/index.tsx | Refactored imports: now imports Clerk components directly from package instead of destructuring. |
| integration/templates/tanstack-react-start/src/server.tsx | Added new server entry file composing Clerk and TanStack React Start handlers. |
| integration/templates/tanstack-react-start/src/styles/app.css | Added new CSS file importing Tailwind and setting a custom font stack on `body`. |
| integration/templates/tanstack-react-start/vite.config.ts | Added new Vite config: uses TypeScript paths and TanStack React Start plugins. |
| packages/tanstack-react-start/package.json | Updated `@tanstack/react-router` and `@tanstack/react-start` to `^1.121.0` in dev and peer dependencies. |
| packages/tanstack-react-start/src/server/middlewareHandler.ts | Removed local type redefinitions in favor of imports; changed returned handler type from `EventHandler` to `RequestHandler`. |
| packages/tanstack-react-start/src/server/constants.ts<br/>loadOptions.ts<br/>utils/index.ts | Replaced usage of `getEvent()` from TanStack with a hardcoded event object with empty context. Added TODO comment in `constants.ts`. |
| .changeset/curly-dots-stand.md | Added changeset: documents minimum `@tanstack/react-start` version is now 1.121.0, drops Vinxi, and links to migration guide. | 🪧 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 (
|
!snapshot |
Hey @wobsoriano - the snapshot version command generated the following package versions:
Tip: Use the snippet copy button below to quickly install the required packages. npm i @clerk/agent-toolkit@0.1.1-snapshot.v20250611045520 --save-exact
npm i @clerk/astro@2.9.1-snapshot.v20250611045520 --save-exact
npm i @clerk/backend@2.0.1-snapshot.v20250611045520 --save-exact
npm i @clerk/chrome-extension@2.4.12-snapshot.v20250611045520 --save-exact
npm i @clerk/clerk-js@5.68.1-snapshot.v20250611045520 --save-exact
npm i @clerk/elements@0.23.33-snapshot.v20250611045520 --save-exact
npm i @clerk/clerk-expo@2.13.1-snapshot.v20250611045520 --save-exact
npm i @clerk/expo-passkeys@0.3.10-snapshot.v20250611045520 --save-exact
npm i @clerk/express@1.7.0-snapshot.v20250611045520 --save-exact
npm i @clerk/fastify@2.4.0-snapshot.v20250611045520 --save-exact
npm i @clerk/localizations@3.16.5-snapshot.v20250611045520 --save-exact
npm i @clerk/nextjs@6.21.1-snapshot.v20250611045520 --save-exact
npm i @clerk/nuxt@1.7.1-snapshot.v20250611045520 --save-exact
npm i @clerk/clerk-react@5.31.10-snapshot.v20250611045520 --save-exact
npm i @clerk/react-router@1.6.0-snapshot.v20250611045520 --save-exact
npm i @clerk/remix@4.8.1-snapshot.v20250611045520 --save-exact
npm i @clerk/shared@3.9.7-snapshot.v20250611045520 --save-exact
npm i @clerk/tanstack-react-start@0.17.0-snapshot.v20250611045520 --save-exact
npm i @clerk/testing@1.8.1-snapshot.v20250611045520 --save-exact
npm i @clerk/themes@2.2.50-snapshot.v20250611045520 --save-exact
npm i @clerk/types@4.60.1-snapshot.v20250611045520 --save-exact
npm i @clerk/vue@1.8.8-snapshot.v20250611045520 --save-exact |
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: 5
🧹 Nitpick comments (7)
.changeset/early-trains-bow.md (1)
1-6
: Complete changeset description
The placeholderTODO
must be replaced with a descriptive summary of what is changing and why for the minor version bump.Would you like help drafting this section?
integration/templates/tanstack-react-start/src/styles/app.css (1)
3-7
: Load custom fontGordita
The font-family referencesGordita
, but there is no import or<link>
for it. Ensure the font is loaded (e.g., via@import
from a CDN or a<link>
in HTML) to avoid falling back to system fonts unexpectedly.integration/templates/tanstack-react-start/vite.config.ts (1)
6-11
: Optional: Add React plugin for enhanced HMR.
Depending on your needs, consider integrating@vitejs/plugin-react
to enable JSX transforms and React Fast Refresh:+import react from '@vitejs/plugin-react'; ... export default defineConfig({ plugins: [ + react(), tsConfigPaths({ projects: ['./tsconfig.json'] }), tanstackStart(), ], });integration/templates/tanstack-react-start/src/router.tsx (1)
8-11
: Avoid exposing stack traces in production.
Renderingerr.error.stack
directly can leak internal details. Consider this pattern:defaultErrorComponent: err => <p> {process.env.NODE_ENV === 'development' ? err.error.stack : 'Something went wrong.'} </p>,integration/templates/tanstack-react-start/src/routes/__root.tsx (1)
26-30
: Add an explicitlang
attribute and essential meta tagsBrowsers and accessibility tooling rely on
<html lang="">
and<meta charset>
to correctly interpret the document. Including them in the SSR root avoids warnings and improves localisation.- <html> + <html lang="en"> + <head> + <meta charSet="utf-8" /> <HeadContent /> - </head> + </head>packages/tanstack-react-start/src/server/middlewareHandler.ts (1)
13-15
: Returnawait cb(…)
to avoid double-wrapped Promises
cb
may itself beasync
. Returning it un-awaited inside anasync
function is safe but produces an unnecessary micro-task hop and obscures error stacks.- return cb({ request, router, responseHeaders }); + return await cb({ request, router, responseHeaders });integration/templates/tanstack-react-start/src/routeTree.gen.ts (1)
1-10
: Generated file: consider excluding from VCS
routeTree.gen.ts
is fully generated and contains a disclaimer; committing it can cause merge noise. Consider adding it to.gitignore
and generating during build instead.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (47)
.changeset/early-trains-bow.md
(1 hunks).github/workflows/ci.yml
(1 hunks)integration/presets/tanstack.ts
(0 hunks)integration/templates/index.ts
(0 hunks)integration/templates/tanstack-react-router/.gitignore
(0 hunks)integration/templates/tanstack-react-router/README.md
(0 hunks)integration/templates/tanstack-react-router/index.html
(0 hunks)integration/templates/tanstack-react-router/package.json
(0 hunks)integration/templates/tanstack-react-router/src/main.tsx
(0 hunks)integration/templates/tanstack-react-router/src/posts.tsx
(0 hunks)integration/templates/tanstack-react-router/src/routeTree.gen.ts
(0 hunks)integration/templates/tanstack-react-router/src/routes/__root.tsx
(0 hunks)integration/templates/tanstack-react-router/src/routes/_layout.tsx
(0 hunks)integration/templates/tanstack-react-router/tsconfig.dev.json
(0 hunks)integration/templates/tanstack-react-router/tsconfig.json
(0 hunks)integration/templates/tanstack-react-router/vite.config.js
(0 hunks)integration/templates/tanstack-react-start/.gitignore
(1 hunks)integration/templates/tanstack-react-start/.prettierignore
(0 hunks)integration/templates/tanstack-react-start/.prettierrc
(0 hunks)integration/templates/tanstack-react-start/app.config.ts
(0 hunks)integration/templates/tanstack-react-start/app/client.tsx
(0 hunks)integration/templates/tanstack-react-start/app/components/DefaultCatchBoundary.tsx
(0 hunks)integration/templates/tanstack-react-start/app/components/NotFound.tsx
(0 hunks)integration/templates/tanstack-react-start/app/routeTree.gen.ts
(0 hunks)integration/templates/tanstack-react-start/app/router.tsx
(0 hunks)integration/templates/tanstack-react-start/app/routes/__root.tsx
(0 hunks)integration/templates/tanstack-react-start/app/routes/_layout.tsx
(0 hunks)integration/templates/tanstack-react-start/app/routes/index.tsx
(0 hunks)integration/templates/tanstack-react-start/app/routes/sign-in.tsx
(0 hunks)integration/templates/tanstack-react-start/app/ssr.tsx
(0 hunks)integration/templates/tanstack-react-start/app/styles/app.css
(0 hunks)integration/templates/tanstack-react-start/package.json
(1 hunks)integration/templates/tanstack-react-start/postcss.config.cjs
(0 hunks)integration/templates/tanstack-react-start/src/routeTree.gen.ts
(1 hunks)integration/templates/tanstack-react-start/src/router.tsx
(1 hunks)integration/templates/tanstack-react-start/src/routes/__root.tsx
(1 hunks)integration/templates/tanstack-react-start/src/routes/index.tsx
(1 hunks)integration/templates/tanstack-react-start/src/server.tsx
(1 hunks)integration/templates/tanstack-react-start/src/styles/app.css
(1 hunks)integration/templates/tanstack-react-start/tailwind.config.cjs
(0 hunks)integration/templates/tanstack-react-start/tsconfig.json
(2 hunks)integration/templates/tanstack-react-start/vite.config.ts
(1 hunks)integration/tests/tanstack-router/basic.test.ts
(0 hunks)package.json
(0 hunks)packages/tanstack-react-start/package.json
(1 hunks)packages/tanstack-react-start/src/server/middlewareHandler.ts
(1 hunks)turbo.json
(0 hunks)
💤 Files with no reviewable changes (33)
- integration/templates/tanstack-react-router/README.md
- package.json
- integration/templates/tanstack-react-router/tsconfig.json
- integration/templates/tanstack-react-start/.prettierrc
- integration/templates/tanstack-react-start/.prettierignore
- integration/templates/tanstack-react-start/app/styles/app.css
- integration/templates/index.ts
- integration/templates/tanstack-react-router/src/main.tsx
- integration/templates/tanstack-react-start/tailwind.config.cjs
- integration/templates/tanstack-react-router/vite.config.js
- integration/templates/tanstack-react-router/src/routes/_layout.tsx
- integration/presets/tanstack.ts
- integration/templates/tanstack-react-start/postcss.config.cjs
- integration/templates/tanstack-react-start/app/router.tsx
- integration/templates/tanstack-react-start/app/routes/sign-in.tsx
- integration/templates/tanstack-react-start/app.config.ts
- integration/templates/tanstack-react-router/package.json
- integration/templates/tanstack-react-router/src/routeTree.gen.ts
- integration/templates/tanstack-react-router/src/routes/__root.tsx
- integration/templates/tanstack-react-start/app/routes/_layout.tsx
- integration/templates/tanstack-react-start/app/components/DefaultCatchBoundary.tsx
- integration/templates/tanstack-react-router/tsconfig.dev.json
- integration/templates/tanstack-react-router/.gitignore
- integration/templates/tanstack-react-start/app/routes/__root.tsx
- integration/templates/tanstack-react-router/index.html
- turbo.json
- integration/templates/tanstack-react-start/app/components/NotFound.tsx
- integration/templates/tanstack-react-start/app/ssr.tsx
- integration/templates/tanstack-react-start/app/client.tsx
- integration/templates/tanstack-react-start/app/routes/index.tsx
- integration/templates/tanstack-react-start/app/routeTree.gen.ts
- integration/tests/tanstack-router/basic.test.ts
- integration/templates/tanstack-react-router/src/posts.tsx
🧰 Additional context used
🧬 Code Graph Analysis (2)
integration/templates/tanstack-react-start/src/server.tsx (1)
packages/tanstack-react-start/src/server/middlewareHandler.ts (1)
createClerkHandler
(9-48)
packages/tanstack-react-start/src/server/middlewareHandler.ts (1)
packages/tanstack-react-start/src/server/types.ts (1)
LoaderOptions
(11-23)
🔇 Additional comments (10)
packages/tanstack-react-start/package.json (1)
77-78
:✅ Verification successful
Bump TanStack package versions
Updating@tanstack/react-router
and@tanstack/react-start
to^1.121.0
in both dev and peer dependencies looks correct.Please verify that version
1.121.0
is published on npm to avoid CI failures:Also applies to: 82-83
🏁 Script executed:
npm view @tanstack/react-router@1.121.0 version npm view @tanstack/react-start@1.121.0 versionLength of output: 114
Bump TanStack package versions – Verified
Both@tanstack/react-router
and@tanstack/react-start
v1.121.0 are published on npm, so updating to^1.121.0
is safe.• packages/tanstack-react-start/package.json (lines 77–78, 82–83)
integration/templates/tanstack-react-start/tsconfig.json (2)
2-2
: Validate JS inclusion in TS project
Including"public/script*.js"
—confirm these scripts require type-checking in this TS config and won't bloat your compile scope unintentionally.
18-18
: Verify updated path alias
The"~/*": ["./src/*"]
mapping aligns with the folder restructure. Ensure all imports using~/
are updated to reflectsrc
instead of the oldapp
paths.integration/templates/tanstack-react-start/.gitignore (1)
10-13
: Ignore new build and cache directories.
Adding.nitro
, Playwright outputs, and.tanstack
correctly excludes Nitro artifacts, Playwright reports/caches, and generated TanStack files from the repo.Also applies to: 17-20
integration/templates/tanstack-react-start/src/routes/index.tsx (1)
1-1
: Consolidated Clerk component imports.
Importing only the required authentication UI components from@clerk/tanstack-react-start
keeps the bundle lean and the code clear.integration/templates/tanstack-react-start/vite.config.ts (1)
1-4
: Vite config setup is clear.
Thevite-tsconfig-paths
plugin and@tanstack/react-start
plugin are correctly wired up for path resolution and template support.integration/templates/tanstack-react-start/src/router.tsx (2)
1-6
: Router initialization is well-structured.
The aliasing ofcreateRouter
and the basicrouteTree
integration cleanly encapsulate the TanStack Router setup.
16-20
: Module augmentation correctly extends Register interface.
Extending theRegister
interface with therouter
type ensures proper typing across your app.integration/templates/tanstack-react-start/src/server.tsx (2)
1-3
: Validate import paths.
Please confirm that@clerk/tanstack-react-start/server
is the correct entrypoint for your server-side handler and that it’s declared inpackage.json
.
5-9
: Clerk handler composition is solid.
WrappingcreateStartHandler
withcreateClerkHandler
and then passing thedefaultStreamHandler
aligns cleanly with the middleware layering you’re aiming for.
.github/workflows/ci.yml
Outdated
@@ -276,7 +276,7 @@ jobs: | |||
strategy: | |||
fail-fast: false | |||
matrix: | |||
test-name: ['generic', 'express', 'quickstart', 'ap-flows', 'elements', 'localhost', 'sessions', 'astro', 'expo-web', 'tanstack-react-start', 'tanstack-react-router', 'vue', 'nuxt', 'react-router', 'billing'] | |||
test-name: ['generic', 'express', 'quickstart', 'ap-flows', 'elements', 'localhost', 'sessions', 'astro', 'expo-web', 'tanstack-react-start', 'vue', 'nuxt', 'react-router', 'billing'] |
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.
Remove react-router
from integration test matrix
The matrix still includes 'react-router'
despite the router integration being removed, which will trigger a non-existent suite. Remove it to align with the removal of the TanStack Router tests.
🤖 Prompt for AI Agents
In .github/workflows/ci.yml at line 279, the test matrix includes
'react-router', which is no longer valid since the router integration and its
tests have been removed. Remove 'react-router' from the test-name array to
prevent running a non-existent test suite and keep the CI configuration
consistent with the current codebase.
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.
tanstack-react-router and react-router are different
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.
@wobsoriano Got it—thanks for the clarification. I conflated the two router suites. Since the PR only removes the TanStack Router integration (tanstack-react-router
) and the separate react-router
suite is still valid, the CI matrix entry should indeed stay as-is. No change is needed here.
Thanks for pointing that out! 🐇
@import 'tailwindcss'; | ||
|
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.
Use Tailwind directives instead of @import 'tailwindcss'
Importing Tailwind via @import 'tailwindcss'
may not trigger the base
, components
, and utilities
layers. Replace with:
@tailwind base;
@tailwind components;
@tailwind utilities;
🤖 Prompt for AI Agents
In integration/templates/tanstack-react-start/src/styles/app.css at lines 1 to
2, replace the single line "@import 'tailwindcss';" with the three Tailwind
directives "@tailwind base;", "@tailwind components;", and "@tailwind
utilities;" to properly include Tailwind's base styles, components, and
utilities layers.
/public/build | ||
.vinxi | ||
# Sentry Config File | ||
/public/build# Sentry Config 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.
🛠️ Refactor suggestion
Fix comment syntax on /public/build
entry.
The line
/public/build# Sentry Config File
will be treated as a literal path pattern rather than an ignore plus comment. Gitignore comments must start the line or be preceded by whitespace. Update to:
-/public/build# Sentry Config File
+/public/build # Sentry Config File
📝 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.
/public/build# Sentry Config File | |
/public/build # Sentry Config File |
🤖 Prompt for AI Agents
In integration/templates/tanstack-react-start/.gitignore at line 14, the comment
on the /public/build entry is incorrectly appended without whitespace, causing
it to be treated as part of the path. Fix this by adding a space before the # so
the line reads "/public/build # Sentry Config File" to properly separate the
ignore pattern from the comment.
<TanStackRouterDevtools position='bottom-right' /> | ||
<Scripts /> |
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
Gate Devtools behind process.env.NODE_ENV !== 'production'
Shipping the dev-tools bundle to production users increases bundle size and may leak internal routing information.
- {children}
- <TanStackRouterDevtools position='bottom-right' />
+ {children}
+ {import.meta.env.MODE !== 'production' && (
+ <TanStackRouterDevtools position='bottom-right' />
+ )}
📝 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.
<TanStackRouterDevtools position='bottom-right' /> | |
<Scripts /> | |
{children} | |
{import.meta.env.MODE !== 'production' && ( | |
<TanStackRouterDevtools position='bottom-right' /> | |
)} | |
<Scripts /> |
🤖 Prompt for AI Agents
In integration/templates/tanstack-react-start/src/routes/__root.tsx around lines
32 to 33, the TanStackRouterDevtools component is rendered unconditionally,
which causes it to be included in production builds. Wrap the
TanStackRouterDevtools component in a conditional that checks if
process.env.NODE_ENV is not equal to 'production' so that it only renders in
development environments, preventing unnecessary bundle size increase and
potential information leaks in production.
"scripts": { | ||
"dev": "vinxi dev --port=$PORT", | ||
"build": "vinxi build", | ||
"start": "vinxi start --port=$PORT" | ||
"build": "vite build && tsc --noEmit", | ||
"dev": "vite dev --port=$PORT", | ||
"start": "vite start --port=$PORT" | ||
}, |
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 start
script and make port flag cross-platform
- Vite’s preview command is
vite preview
, notvite start
. - Direct use of
$PORT
fails on Windows; usecross-env
(already a dep elsewhere) for portability.
- "dev": "vite dev --port=$PORT",
- "start": "vite start --port=$PORT"
+ "dev": "cross-env PORT=${PORT:-5173} vite dev --port $PORT",
+ "start": "cross-env PORT=${PORT:-5173} vite preview --port $PORT"
(Add cross-env
to devDependencies
if it is not already present.)
📝 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.
"scripts": { | |
"dev": "vinxi dev --port=$PORT", | |
"build": "vinxi build", | |
"start": "vinxi start --port=$PORT" | |
"build": "vite build && tsc --noEmit", | |
"dev": "vite dev --port=$PORT", | |
"start": "vite start --port=$PORT" | |
}, | |
"scripts": { | |
"build": "vite build && tsc --noEmit", | |
"dev": "cross-env PORT=${PORT:-5173} vite dev --port $PORT", | |
"start": "cross-env PORT=${PORT:-5173} vite preview --port $PORT" | |
}, |
🤖 Prompt for AI Agents
In integration/templates/tanstack-react-start/package.json lines 5 to 9, the
"start" script incorrectly uses "vite start" instead of "vite preview" and
directly uses the environment variable $PORT, which is not cross-platform
compatible. Change the "start" script to use "vite preview" and prefix the port
environment variable with "cross-env" to ensure it works on all platforms. Also
update the "dev" script similarly to use "cross-env" before setting the PORT
variable. If "cross-env" is not already in devDependencies, add it there.
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: 3
🧹 Nitpick comments (3)
packages/tanstack-react-start/src/server/loadOptions.ts (1)
17-20
: Repeated stub creation – extract to helperThe same
{ context: {} }
literal appears in three server files. Lift it into
src/server/utils/emptyEvent.ts
(or similar) to stay DRY and make the eventual
removal a one-liner.packages/tanstack-react-start/src/server/utils/index.ts (1)
25-29
: Duplicate stub logic – centraliseSame recommendation as in
loadOptions.ts
: create a small
getEventContext()
utility to avoid three copies of the TODO block.packages/tanstack-react-start/src/server/constants.ts (1)
7-10
: Avoid copy-paste TODO blocksThree identical TODOs referencing issue #100 exist. Consolidate into a single
helper or at least a shared constant to prevent drift when the temporary hack
is eventually removed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
package.json
(0 hunks)packages/tanstack-react-start/src/server/constants.ts
(1 hunks)packages/tanstack-react-start/src/server/loadOptions.ts
(1 hunks)packages/tanstack-react-start/src/server/utils/index.ts
(1 hunks)
💤 Files with no reviewable changes (1)
- package.json
// TODO: See https://github.com/clerkinc/tanstack-react-start/issues/100 | ||
const event = { | ||
context: {}, | ||
}; |
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.
Hard-coding an empty event.context
breaks dynamic env resolution
Every call to getEnvVariable
/ getPublicEnvVariables
now receives {}
, so any
per-request or deployment-specific values that used to be injected via
getEvent()
are silently ignored. This can surface as:
• wrong domain / proxy resolution when the app is deployed behind a
CDN/edge-worker
• missing CLERK_*
overrides coming from the platform (e.g. Vercel env-bags)
• impossible debugging – the code will “work” locally but misbehave in prod.
- // TODO: See https://github.com/clerkinc/tanstack-react-start/issues/100
- const event = {
- context: {},
- };
+// FIXME: fallback only – keep the real context if the caller can provide it
+const event = {
+ context: (overrides as any).eventContext ?? {}, // ← temp escape-hatch
+};
Better yet, pass the real RequestEvent
(or a typed subset) down to
loadOptions
and remove the fallback once
tanstack-react-start restores getEvent()
.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/tanstack-react-start/src/server/loadOptions.ts around lines 17 to
20, the code hard-codes event.context as an empty object, which breaks dynamic
environment variable resolution by always passing an empty context to
getEnvVariable and getPublicEnvVariables. To fix this, modify the code to accept
and pass the real RequestEvent or a typed subset to loadOptions instead of using
a hard-coded empty context, and remove the fallback once tanstack-react-start
restores getEvent().
// TODO: See https://github.com/clerkinc/tanstack-react-start/issues/100 | ||
const event = { | ||
context: {}, | ||
}; | ||
|
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.
Empty event context propagates stale config to the client
getResponseClerkState
now hard-codes an empty context, so the generated
__clerkJSUrl
, telemetry flags, etc. will always reflect build-time process.env
and never the runtime environment. Users running multiple deploy previews or
staging/production from the same build will hit inconsistent behaviour.
Consider threading the real context in through additionalStateOptions
(similar to the proposed change in loadOptions.ts
) or at least document the
limitation loudly in the README so integrators are aware.
🤖 Prompt for AI Agents
In packages/tanstack-react-start/src/server/utils/index.ts around lines 25 to
29, the event context is hard-coded as an empty object, causing stale build-time
environment variables to propagate to the client. To fix this, modify the code
to pass the actual runtime context through additionalStateOptions or a similar
mechanism instead of an empty object. If that is not feasible immediately, add
clear documentation in the README warning users about this limitation and its
impact on environment consistency.
// TODO: See https://github.com/clerkinc/tanstack-react-start/issues/100 | ||
const event = { | ||
context: {}, | ||
}; |
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.
commonEnvs
now ignores platform-supplied env overrides
By feeding an always-empty context to getPublicEnvVariables
/
getEnvVariable
, any edge/runtime-specific overrides (e.g. CLERK_API_URL
injected by a hosting provider) are lost. This diverges from previous
behaviour and may cause subtle production bugs.
At minimum, expose the context as a parameter:
-export const commonEnvs = () => {
+export const commonEnvs = (eventContext: Record<string, any> = {}) => {
// TODO: See https://github.com/clerkinc/tanstack-react-start/issues/100
- const event = {
- context: {},
- };
+ const event = { context: eventContext };
Callers that don’t have an event yet can pass {}
until upstream support
returns.
🤖 Prompt for AI Agents
In packages/tanstack-react-start/src/server/constants.ts around lines 7 to 10,
the current code uses a hardcoded empty context object when calling
getPublicEnvVariables or getEnvVariable, which prevents platform-specific
environment overrides from being applied. To fix this, modify the code to accept
the context as a parameter so callers can provide the actual event context when
available, and default to an empty object only if no context is provided. This
will restore support for edge/runtime-specific environment overrides.
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
♻️ Duplicate comments (1)
packages/tanstack-react-start/src/server/utils/index.ts (1)
25-28
: Same hard-coded event issue as aboveSee comments in
constants.ts
. Dropping context here affects__clerkJSUrl
, telemetry flags, and force-redirect URLs, which are often configured via environment bindings at the deployment layer. Use a shared helper or conditionalgetEvent()
import to preserve correctness.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
package.json
(0 hunks)packages/tanstack-react-start/src/server/constants.ts
(1 hunks)packages/tanstack-react-start/src/server/loadOptions.ts
(1 hunks)packages/tanstack-react-start/src/server/utils/index.ts
(1 hunks)
💤 Files with no reviewable changes (1)
- package.json
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (2)
packages/tanstack-react-start/src/server/constants.ts (2)
34-36
:❌ Incorrect review comment
PACKAGE_NAME
/PACKAGE_VERSION
look undefinedNeither constant is imported or declared in this file. TypeScript will error unless they are injected globally by the build tool (e.g. via
define
). If that’s intentional, add an inline comment or an explicit/* global … */
to avoid future clean-up PRs deleting them as “dead code”.
🏁 Script executed:
#!/bin/bash # Search for any declarations or imports of PACKAGE_NAME and PACKAGE_VERSION rg "PACKAGE_NAME" -n . rg "PACKAGE_VERSION" -n .Length of output: 16857
No action needed:
PACKAGE_NAME
andPACKAGE_VERSION
are globally declaredTypeScript will not error here because both constants are declared in your global type definitions and injected at build/test time:
- packages/tanstack-react-start/src/global.d.ts declares
declare const PACKAGE_NAME: string; declare const PACKAGE_VERSION: string;- tsup.config.ts defines them via the build tool and vitest.setup.mts assigns them on
globalThis
.You can safely ignore the import warning.
Likely an incorrect or invalid review comment.
7-10
:❓ Verification inconclusive
Hard-coding
event
silently drops per-request contextReplacing
getEvent()
with a stub{ context: {} }
means every call togetEnvVariable
/getPublicEnvVariables
will ignore values that might be injected through the real event context (e.g. Netlify edge/worker bindings).
This can lead to subtle prod-only mis-configs while all local tests still pass.- // TODO: See https://github.com/clerkinc/tanstack-react-start/issues/100 - const event = { - context: {}, - }; + // Fallback to empty context only when `getEvent` is unavailable. + // Remove once issue #100 is resolved. + let event: { context: Record<string, unknown> } = { context: {} }; + try { + // eslint-disable-next-line import/no-extraneous-dependencies + const { getEvent } = await import('@tanstack/react-start/server'); + event = getEvent?.() ?? event; + } catch { + /* no-op – keep the empty context */ + }This keeps the temporary workaround but transparently restores real context when possible.
To confirm that
getEvent
is indeed exported from the server entrypoint, let’s search for its definition in thepackages/tanstack-react-start/src/server
directory:
🏁 Script executed:
#!/bin/bash rg -n "export.*getEvent" -R packages/tanstack-react-start/src/serverLength of output: 475
Let’s retry searching for
getEvent
exports without the invalid-R
flag:
🏁 Script executed:
#!/bin/bash # Search for any getEvent definitions or exports in the server code rg -n "getEvent" packages/tanstack-react-start/src/serverLength of output: 57
Verify
getEvent
export before applying the dynamic‐import fallbackI wasn’t able to locate any
getEvent
definition or export inpackages/tanstack-react-start/src/server
(nor elsewhere in the repo) using a broad search. Before merging this change, please manually confirm that:
@tanstack/react-start/server
actually exports agetEvent
function- A dynamic
import
will correctly resolve it in your target runtimesIf
getEvent
isn’t available or is defined elsewhere, you’ll need to update the import path or module API accordingly to ensure real request context is restored in production.
// TODO: See https://github.com/clerkinc/tanstack-react-start/issues/100 | ||
const event = { | ||
context: {}, | ||
}; |
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
Duplicate stub context – centralise to avoid drift
The same { context: {} }
snippet now lives in three files. When the TODO is resolved, every location must be touched. Extract a helper in server/event.ts
(or reuse commonEnvs
) that returns the correct event object or a safe stub; import it here instead.
🤖 Prompt for AI Agents
In packages/tanstack-react-start/src/server/loadOptions.ts around lines 17 to
20, the event object with a stub context is duplicated in multiple files. To fix
this, create a helper function in server/event.ts (or reuse commonEnvs) that
returns the event object or a safe stub, then replace the inline event object
here with an import and call to that helper. This centralizes the stub and
prevents drift when changes are needed.
@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 (2)
.changeset/curly-dots-stand.md (2)
5-5
: Add missing article and clarify phrasing
Consider prefacing with "the" and tightening the parenthetical. For example:- Updated minimum required version of `@tanstack/react-start` to 1.121.0 (Devinxi), which removes the need for Vinxi. + Updated the minimum required version of `@tanstack/react-start` to 1.121.0, removing the Vinxi dependency.🧰 Tools
🪛 LanguageTool
[uncategorized] ~5-~5: You might be missing the article “the” here.
Context: ...nstack-react-start': major --- Updated minimum required version of `@tanstack/react-st...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
7-7
: Convert bare URL to Markdown link
Wrap the migration guide in link syntax to satisfy lint rules. For example:- See full migration guide: https://github.com/TanStack/router/discussions/2863#discussioncomment-13104960 + See the [full migration guide](https://github.com/TanStack/router/discussions/2863#discussioncomment-13104960).🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
7-7: Bare URL used
null(MD034, no-bare-urls)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.changeset/curly-dots-stand.md
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
.changeset/curly-dots-stand.md
[uncategorized] ~5-~5: You might be missing the article “the” here.
Context: ...nstack-react-start': major --- Updated minimum required version of `@tanstack/react-st...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
🪛 markdownlint-cli2 (0.17.2)
.changeset/curly-dots-stand.md
7-7: Bare URL used
null
(MD034, no-bare-urls)
⏰ 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)
🔇 Additional comments (1)
.changeset/curly-dots-stand.md (1)
1-3
: Frontmatter formatting looks correct
The YAML header properly specifies the target package and change type.
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 (2)
.changeset/curly-dots-stand.md (2)
5-5
: Add missing article for clarity.Consider revising to “Updated the minimum required version of
@tanstack/react-start
…” to improve readability.🧰 Tools
🪛 LanguageTool
[uncategorized] ~5-~5: You might be missing the article “the” here.
Context: ...nstack-react-start': minor --- Updated minimum required version of `@tanstack/react-st...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
7-7
: Convert bare URL to markdown link.Wrapping the migration guide URL in link text avoids markdownlint MD034 warnings. For example:
See full migration guide: [TanStack Router migration discussion](https://github.com/TanStack/router/discussions/2863#discussioncomment-13104960)🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
7-7: Bare URL used
null(MD034, no-bare-urls)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.changeset/curly-dots-stand.md
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
.changeset/curly-dots-stand.md
[uncategorized] ~5-~5: You might be missing the article “the” here.
Context: ...nstack-react-start': minor --- Updated minimum required version of `@tanstack/react-st...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
🪛 markdownlint-cli2 (0.17.2)
.changeset/curly-dots-stand.md
7-7: Bare URL used
null
(MD034, no-bare-urls)
⏰ 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)
!snapshot |
Hey @wobsoriano - the snapshot version command generated the following package versions:
Tip: Use the snippet copy button below to quickly install the required packages. npm i @clerk/agent-toolkit@0.1.1-snapshot.v20250613153805 --save-exact
npm i @clerk/astro@2.9.1-snapshot.v20250613153805 --save-exact
npm i @clerk/backend@2.1.0-snapshot.v20250613153805 --save-exact
npm i @clerk/chrome-extension@2.4.12-snapshot.v20250613153805 --save-exact
npm i @clerk/clerk-js@5.68.1-snapshot.v20250613153805 --save-exact
npm i @clerk/elements@0.23.33-snapshot.v20250613153805 --save-exact
npm i @clerk/clerk-expo@2.13.1-snapshot.v20250613153805 --save-exact
npm i @clerk/expo-passkeys@0.3.10-snapshot.v20250613153805 --save-exact
npm i @clerk/express@1.7.0-snapshot.v20250613153805 --save-exact
npm i @clerk/fastify@2.4.0-snapshot.v20250613153805 --save-exact
npm i @clerk/localizations@3.16.5-snapshot.v20250613153805 --save-exact
npm i @clerk/nextjs@6.21.1-snapshot.v20250613153805 --save-exact
npm i @clerk/nuxt@1.7.1-snapshot.v20250613153805 --save-exact
npm i @clerk/clerk-react@5.31.10-snapshot.v20250613153805 --save-exact
npm i @clerk/react-router@1.6.0-snapshot.v20250613153805 --save-exact
npm i @clerk/remix@4.8.1-snapshot.v20250613153805 --save-exact
npm i @clerk/shared@3.9.7-snapshot.v20250613153805 --save-exact
npm i @clerk/tanstack-react-start@0.17.0-snapshot.v20250613153805 --save-exact
npm i @clerk/testing@1.8.1-snapshot.v20250613153805 --save-exact
npm i @clerk/themes@2.2.50-snapshot.v20250613153805 --save-exact
npm i @clerk/types@4.60.1-snapshot.v20250613153805 --save-exact
npm i @clerk/vue@1.8.8-snapshot.v20250613153805 --save-exact |
Closed in favor of #6115 |
Description
This PR updates the types to match the new TanStack Start version. This also removes the TanStack Router integration tests, as TanStack Start is built on top of it, and there's no reason it would work in Start and not in Router, saving some CI resources
Checklist
pnpm test
runs as expected.pnpm build
runs as expected.Type of change
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores