-
Notifications
You must be signed in to change notification settings - Fork 140
feat: update tanstack start #593
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
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis update migrates the project from the Vinxi build system to Vite, updating configuration files, scripts, and dependencies accordingly. Server API route definitions are refactored to use Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Server
participant Vite
participant TanstackRouter
Note over Client,Server: App Startup (Vite-based)
Client->>Vite: Request app entry
Vite->>Server: Serve app with Vite dev/build server
Server->>TanstackRouter: Route request (SSR or API)
TanstackRouter-->>Server: Resolve route (client/server)
Server-->>Client: Return rendered response or API data
Note over Client,Server: API Route Handling (New ServerRoute)
Client->>Server: HTTP request to /api/...
Server->>TanstackRouter: Match server route (ServerRoute)
TanstackRouter->>Server: Call .methods({ ... }) handler
Server-->>Client: Return API response
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/routes/api/rest.ts (1)
14-14
: Consider providing meaningful initial context.The empty context object might be intentional, but consider documenting what context should be provided or if this is a placeholder for future implementation.
- context: {}, // Provide initial context if needed + context: {}, // TODO: Add request-specific context (user, session, etc.)src/routes/api/rest.$.ts (1)
3-26
: LGTM! Migration completed correctly, but consider refactoring duplicate code.The server-side migration is implemented correctly. However, this file has identical implementation to
src/routes/api/rest.ts
(lines 7-18 are duplicated).Consider extracting the common handler logic to reduce duplication:
+// Create a shared handler file: src/lib/api-handlers.ts +export const createRestHandler = () => { + const handler = new OpenAPIHandler(router, { + plugins: [new CORSPlugin(), new ResponseHeadersPlugin()], + }); + + return async ({ request }: { request: Request }) => { + const { response } = await handler.handle(request, { + prefix: '/api/rest', + context: {}, + }); + return response ?? new Response('Not Found', { status: 404 }); + }; +};Then import and use it in both route files.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (29)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
src/components/icons/generated/IconBookOpen.tsx
is excluded by!**/generated/**
src/components/icons/generated/IconBookOpenDuotone.tsx
is excluded by!**/generated/**
src/components/icons/generated/IconBookOpenFill.tsx
is excluded by!**/generated/**
src/components/icons/generated/IconGitBranch.tsx
is excluded by!**/generated/**
src/components/icons/generated/IconGitBranchDuotone.tsx
is excluded by!**/generated/**
src/components/icons/generated/IconGitBranchFill.tsx
is excluded by!**/generated/**
src/components/icons/generated/IconHouse.tsx
is excluded by!**/generated/**
src/components/icons/generated/IconHouseDuotone.tsx
is excluded by!**/generated/**
src/components/icons/generated/IconHouseFill.tsx
is excluded by!**/generated/**
src/components/icons/generated/IconUserCircle.tsx
is excluded by!**/generated/**
src/components/icons/generated/IconUserCircleDuotone.tsx
is excluded by!**/generated/**
src/components/icons/generated/IconUserCircleFill.tsx
is excluded by!**/generated/**
src/components/icons/generated/index.ts
is excluded by!**/generated/**
src/components/icons/svg-sources/icon-book-open-duotone.svg
is excluded by!**/*.svg
src/components/icons/svg-sources/icon-book-open-fill.svg
is excluded by!**/*.svg
src/components/icons/svg-sources/icon-book-open.svg
is excluded by!**/*.svg
src/components/icons/svg-sources/icon-git-branch-duotone.svg
is excluded by!**/*.svg
src/components/icons/svg-sources/icon-git-branch-fill.svg
is excluded by!**/*.svg
src/components/icons/svg-sources/icon-git-branch.svg
is excluded by!**/*.svg
src/components/icons/svg-sources/icon-house-duotone.svg
is excluded by!**/*.svg
src/components/icons/svg-sources/icon-house-fill.svg
is excluded by!**/*.svg
src/components/icons/svg-sources/icon-house.svg
is excluded by!**/*.svg
src/components/icons/svg-sources/icon-user-circle-duotone.svg
is excluded by!**/*.svg
src/components/icons/svg-sources/icon-user-circle-fill.svg
is excluded by!**/*.svg
src/components/icons/svg-sources/icon-user-circle.svg
is excluded by!**/*.svg
src/features/auth/layout-login-image.jpg
is excluded by!**/*.jpg
src/features/auth/mascot-error.png
is excluded by!**/*.png
src/features/auth/mascot.png
is excluded by!**/*.png
📒 Files selected for processing (26)
.prettierrc.cjs
(1 hunks).storybook/preview.tsx
(1 hunks)app.config.ts
(0 hunks)app/api.ts
(0 hunks)app/client.tsx
(0 hunks)app/routeTree.gen.ts
(0 hunks)app/routes/api/rest.ts
(0 hunks)app/routes/api/rpc.ts
(0 hunks)app/ssr.tsx
(0 hunks)components.json
(1 hunks)eslint.config.ts
(3 hunks)package.json
(3 hunks)src/components/icons/svgr.config.cjs
(1 hunks)src/env/client.ts
(1 hunks)src/features/build-info/script-to-generate-json.ts
(1 hunks)src/routeTree.gen.ts
(1 hunks)src/routes/api/auth.$.ts
(1 hunks)src/routes/api/dev.email.$template.ts
(1 hunks)src/routes/api/openapi/app.schema.ts
(2 hunks)src/routes/api/openapi/app.ts
(1 hunks)src/routes/api/openapi/auth.schema.ts
(1 hunks)src/routes/api/openapi/auth.ts
(1 hunks)src/routes/api/rest.$.ts
(2 hunks)src/routes/api/rest.ts
(1 hunks)src/routes/api/rpc.$.ts
(2 hunks)src/routes/api/rpc.ts
(1 hunks)
💤 Files with no reviewable changes (7)
- app/api.ts
- app/routes/api/rest.ts
- app/ssr.tsx
- app.config.ts
- app/client.tsx
- app/routes/api/rpc.ts
- app/routeTree.gen.ts
🧰 Additional context used
🧠 Learnings (1)
.storybook/preview.tsx (1)
Learnt from: ivan-dalmet
PR: BearStudio/start-ui-web#532
File: src/features/auth/PageOAuthCallback.tsx:43-45
Timestamp: 2024-09-30T11:07:14.833Z
Learning: When suggesting changes to `useEffect` dependencies in React components, ensure that removing dependencies doesn't cause React Hook errors about missing dependencies.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Playwright E2E Tests
🔇 Additional comments (33)
src/features/build-info/script-to-generate-json.ts (1)
37-37
: LGTM - Path update aligns with directory migration.The path update from
./app/
to./src/
is consistent with the project's directory restructuring.src/env/client.ts (1)
9-9
: Verify the environment property change is correct for TanStack Start.The change from
import.meta.env?.DEV
toimport.meta.dev
modifies how the development environment is detected. This could be a breaking change ifimport.meta.dev
is not available or behaves differently thanimport.meta.env.DEV
.Please verify that
import.meta.dev
is the correct property for TanStack Start and that it provides the same boolean development mode detection as the previous implementation.What is the correct way to detect development mode in TanStack Start? Should I use import.meta.dev or import.meta.env.DEV?
components.json (1)
8-8
: LGTM - CSS path update aligns with directory migration.The Tailwind CSS path update from
app/styles/app.css
tosrc/styles/app.css
is consistent with the project's directory restructuring.src/components/icons/svgr.config.cjs (1)
27-27
: LGTM - Output directory update aligns with directory migration.The SVGR output directory update from
./app/components/icons/generated
to./src/components/icons/generated
is consistent with the project's directory restructuring..prettierrc.cjs (1)
13-13
: LGTM - Tailwind stylesheet path update aligns with directory migration.The Tailwind stylesheet path update from
'./app/styles/app.css'
to'./src/styles/app.css'
is consistent with the project's directory restructuring..storybook/preview.tsx (3)
4-4
: LGTM! Import reorganization looks good.Moving the
useEffect
import to group with other React hooks improves import organization.
8-8
: Verify CSS import order impact.The CSS import order was changed with
preview.css
now loading afterapp.css
. Ensure this doesn't affect styling precedence or break any existing styles.
13-15
: Import path migration completed correctly.The import paths have been successfully updated from
../app/
to../src/
to align with the directory restructuring.eslint.config.ts (3)
52-52
: Directory path updated correctly for unicorn plugin.The ignore pattern has been properly updated from
app/routes/**/*.*
tosrc/routes/**/*.*
to align with the directory restructuring.
64-64
: File pattern updated for src directory.The file matching pattern has been correctly updated to target the new
src/**/*
directory structure.
73-73
: Error message updated for new directory structure.The error message correctly references the new environment variable location at
@/src/env
instead of@/app/env
.src/routes/api/dev.email.$template.ts (2)
1-1
: API route migration to server-side route completed.The import has been correctly updated from
createAPIFileRoute
tocreateServerFileRoute
as part of the migration to TanStack Start server routes.
5-7
: Server route creation follows correct pattern.The route creation has been properly migrated to use
createServerFileRoute
with the.methods()
chaining syntax. The export name change fromAPIRoute
toServerRoute
is consistent with the migration pattern.src/routes/api/rpc.ts (3)
7-9
: RPC handler setup with useful plugins.The RPC handler is properly configured with CORS and ResponseHeaders plugins, which are essential for API endpoints.
11-18
: Well-implemented request handler with proper error handling.The handler function correctly processes requests with the
/api/rpc
prefix and provides a 404 fallback when no response is available.
20-26
: Comprehensive HTTP method support.The server route correctly supports all standard HTTP methods (GET, POST, PUT, PATCH, DELETE) using the same handler function, which is appropriate for RPC endpoints.
src/routes/api/auth.$.ts (2)
1-1
: Auth route migration completed correctly.The import has been properly updated to use
createServerFileRoute
from the server package, consistent with the API route migration pattern.
5-12
: Server route with appropriate HTTP method support for auth.The auth route correctly supports both GET and POST methods, which is appropriate for authentication endpoints that need to handle both authentication status checks and login/logout requests.
src/routes/api/rest.ts (1)
1-26
: LGTM! Clean server-side API route implementation.The migration to server-side routing is well-implemented with proper middleware setup and error handling. The fallback 404 response is a good practice.
src/routes/api/openapi/auth.schema.ts (1)
2-21
: LGTM! Consistent migration to server-side routing.The migration from
createAPIFileRoute
tocreateServerFileRoute
is correctly implemented with proper import updates and export naming convention. The core OpenAPI schema generation logic is preserved.src/routes/api/openapi/auth.ts (1)
1-8
: LGTM! Consistent server-side route migration.The migration follows the established pattern correctly with proper imports, export naming, and preserved functionality.
src/routes/api/openapi/app.ts (1)
1-8
: LGTM! Migration pattern completed consistently.The final API route migration follows the established pattern perfectly with correct server-side imports, export naming, and preserved functionality.
src/routes/api/openapi/app.schema.ts (3)
4-4
: LGTM: Correct import update for server-side routing.The import change from
@tanstack/react-start/api
to@tanstack/react-start/server
is appropriate for the migration to server-side routes.
13-15
: LGTM: Proper server route creation pattern.The migration from
createAPIFileRoute
tocreateServerFileRoute
with.methods()
chaining follows the correct TanStack Start server route pattern. The export name change toServerRoute
accurately reflects the server-side nature.
16-52
: LGTM: OpenAPI schema generation logic preserved correctly.The internal handler logic for generating the OpenAPI schema using the OpenAPIGenerator is correctly maintained during the migration to server-side routing.
src/routes/api/rpc.$.ts (2)
3-3
: LGTM: Correct import for server-side routing.The import update to
@tanstack/react-start/server
is appropriate for the server route migration.
20-26
: LGTM: Appropriate RPC endpoint implementation.The server route implementation correctly:
- Uses the catch-all path pattern
/$
for dynamic RPC handling- Maps multiple HTTP methods to the same handler function, which is standard for RPC endpoints
- Exports as
ServerRoute
to reflect server-side natureThis pattern allows the RPC handler to process various HTTP verbs through a unified interface.
package.json (3)
19-20
: LGTM: Consistent directory path updates.All script paths have been correctly updated from
./app/...
to./src/...
, maintaining consistency with the directory restructuring.Also applies to: 25-25, 28-29
69-71
: Verify package version compatibility.The TanStack package versions have been updated and
vite
has been added as a devDependency. Ensure these versions are compatible with each other and the new server routing approach.What are the latest compatible versions of @tanstack/react-router, @tanstack/react-start, and @tanstack/zod-adapter packages?
Also applies to: 147-147
13-13
: Verify vite configuration exists for the build tool migration.The migration from
vinxi
tovite
commands represents a significant build tool change. Ensure that the corresponding vite configuration file exists and is properly set up.#!/bin/bash # Description: Verify vite configuration exists and check for any remaining vinxi references # Check for vite configuration files echo "=== Checking for vite configuration files ===" fd "vite.config" . fd "vitest.config" . # Check for any remaining vinxi references echo "=== Checking for remaining vinxi references ===" rg -i "vinxi" --type json --type js --type ts # Check if vite is properly installed echo "=== Checking vite installation ===" cat package.json | jq '.devDependencies.vite // "not found"'Also applies to: 15-17
src/routeTree.gen.ts (3)
1-10
: LGTM: Auto-generated file with proper warnings.This file is correctly marked as auto-generated with appropriate warnings about not making manual changes. The eslint and TypeScript disable comments are appropriate for generated code.
11-49
: LGTM: Comprehensive route imports and server route setup.The imports correctly reference the new
src/routes
structure and include both client routes and server routes. ThecreateServerRootRoute
import and usage is appropriate for the server-side routing setup.
878-891
: LGTM: Dual route tree exports for client and server.The file correctly exports both
routeTree
for client-side routing andserverRouteTree
for server-side routing, which aligns with the hybrid routing approach in TanStack Start.
42dd665
to
2325573
Compare
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)
README.md (2)
43-46
: Add language identifier to fenced code blockmarkdownlint (
MD040
) warns because the fenced block lacks a language hint.
Addingbash
keeps style consistent with earlier blocks and improves syntax highlighting.-``` +```bash pnpm build pnpm start -``` +```
36-40
: Explicitly warn against committing secretsThe new deployment section instructs users to populate environment variables but doesn’t remind them to keep the
.env
file out of version control. A one-liner notice here can save newcomers from accidentally pushing credentials.
Consider appending:Note: Never commit
.env
files or secrets to your repository; add.env
to.gitignore
if it is not already.
📜 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 (6)
README.md
(1 hunks)app/routeTree.gen.ts
(2 hunks)app/routes/api/rest.ts
(0 hunks)app/routes/api/rpc.ts
(0 hunks)package.json
(4 hunks)vite.config.ts
(1 hunks)
💤 Files with no reviewable changes (2)
- app/routes/api/rest.ts
- app/routes/api/rpc.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- vite.config.ts
- package.json
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
README.md
43-43: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Playwright E2E Tests
🔇 Additional comments (7)
README.md (1)
48-51
: EnsureDEPLOY_TARGET
-aware scripts existThis paragraph assumes the codebase reacts to
DEPLOY_TARGET=vercel
, typically viavite.config.ts
or runtime checks. Please verify:
vite.config.ts
reads and uses this variable.- No other deployment paths are unintentionally affected.
If anything is missing, document or implement the handling logic.
app/routeTree.gen.ts (6)
11-47
: Well-structured import organization for dual route architecture.The imports are properly organized with clear separation between client routes (using
Route
alias) and server routes (usingServerRoute
alias). The naming convention with explicitRouteImport
andServerRouteImport
suffixes improves clarity and prevents naming conflicts.
48-48
: Correct server root route implementation.The server root route is properly created using
createServerRootRoute()
from the TanStack Start server package, establishing the foundation for the server route tree.
181-223
: Server routes properly configured with correct parent relationships.The server route definitions correctly:
- Use
/api
namespace for all server endpoints- Reference
rootServerRouteImport
as parent for top-level API routes- Establish proper parent-child relationships for nested routes (e.g., schema routes under their respective OpenAPI routes)
- Maintain consistent path and ID structures
379-450
: Comprehensive server route type definitions.The server route type interfaces mirror the client route structure effectively, providing:
- Complete type safety for server routes
- Consistent naming patterns with
FileServer*
prefixes- Proper full path, route-by-to, and route-by-id mappings
- Well-defined root server route children interface
631-690
: Proper module augmentation for server routes.The new module augmentation for
@tanstack/react-start/server
correctly extends the framework's type system with the project's server route definitions, maintaining type safety across the server-side routing infrastructure.
816-829
: Clean separation of client and server route trees.The dual export structure with
routeTree
for client routes andserverRouteTree
for server routes provides a clean architectural separation that aligns well with the TanStack Start framework's design patterns.
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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/routeTree.gen.ts
(2 hunks)app/routes/storybook.index.ts
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Playwright E2E Tests
🔇 Additional comments (6)
app/routeTree.gen.ts (6)
11-11
: Good addition of server root route import.The import of
createServerRootRoute
aligns with the migration to support server-side routing.
39-47
: Server route imports are well-structured.The server route imports follow a consistent naming pattern with
ServerRoute
aliases, which clearly distinguishes them from client routes.
182-229
: Server route definitions maintain consistency.The server route definitions correctly use the same update pattern as client routes, with proper parent-child relationships established.
385-463
: Comprehensive TypeScript interfaces for server routes.The server route type definitions mirror the client route structure, providing strong typing for the new server-side functionality.
644-710
: Module augmentation properly extends TanStack Start server types.The server route module augmentation follows the same pattern as the client routes, ensuring type safety across the server routing system.
836-850
: Clean separation of client and server route trees.The dual export approach (
routeTree
andserverRouteTree
) provides clear separation between client-side and server-side routing while maintaining type safety.
a48c9ee
to
6e658b2
Compare
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 (1)
vite.config.ts (1)
10-13
: Consider the implications ofstrictPort: true
.The
strictPort: true
setting will cause the dev server to exit if the specified port is unavailable, rather than trying the next available port. This could cause issues in development environments where the port might be occupied.Consider whether this strict behavior is necessary for your use case, or if you'd prefer the server to find an available port:
server: { port: env.PORT ? Number(env.PORT) : 3000, - strictPort: true, + strictPort: false, // or remove this line entirely as false is the default },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.env.example
(1 hunks)vercel.json
(1 hunks)vite.config.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- vercel.json
- .env.example
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Playwright E2E Tests
🔇 Additional comments (3)
vite.config.ts (3)
1-3
: LGTM! Clean import structure.The imports are well-organized and include the necessary dependencies for the Vite configuration.
14-24
: Plugin configuration looks good.The plugin setup is correct:
tsConfigPaths
properly references the TypeScript configurationtanstackStart
plugin is configured with appropriate source and routes directories
5-26
: Overall configuration structure is solid.The Vite configuration follows best practices with a function-based config that adapts based on the mode. The migration from the previous build system appears well-structured.
|
* wip: update tanstack start * fix: run prettier * fix: upgrade orpc * fix: deploy target * fix: test without target * fix: revert readme * fix: storybook build * fix: storybook redirect * fix: PR feedbacks * fix: e2e * fix: limite env load to VITE_ prefix for security
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores
Style