-
Notifications
You must be signed in to change notification settings - Fork 334
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
docs: add react router example #1134
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
WalkthroughThis pull request introduces a comprehensive example project demonstrating the integration of React Router, Clerk authentication, and UploadThing in a Remix application. It includes configuration files, routing setup, authentication handling, and file upload functionality. The project provides a complete template for developers looking to build web applications with these technologies, covering aspects like client and server-side rendering, environment configuration, and type-safe implementations. Changes
Suggested labels
Suggested reviewers
Possibly related PRs
Assessment against linked issues
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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: 4
🧹 Nitpick comments (11)
examples/with-clerk-react-router/tsconfig.json (1)
19-26
: Consider enabling additional type-checking options.While the current type-checking configuration is good, you might want to enable these additional checks for better type safety:
{ "compilerOptions": { "strict": true, + "noUncheckedIndexedAccess": true, + "exactOptionalPropertyTypes": true, // ... other options } }examples/with-clerk-react-router/vite.config.ts (1)
5-7
: Consider adding essential Vite configurations.The current configuration is minimal. Consider adding:
- React plugin (either
@vitejs/plugin-react
or@vitejs/plugin-react-swc
)- Environment variables configuration
- Build optimization settings
Example enhancement:
import { reactRouter } from "@react-router/dev/vite"; import { defineConfig } from "vite"; import tsconfigPaths from "vite-tsconfig-paths"; +import react from "@vitejs/plugin-react-swc"; export default defineConfig({ - plugins: [reactRouter(), tsconfigPaths()], + plugins: [react(), reactRouter(), tsconfigPaths()], + build: { + sourcemap: true, + rollupOptions: { + output: { + manualChunks: { + vendor: ['react', 'react-dom', 'react-router-dom'], + }, + }, + }, + }, });examples/with-clerk-react-router/app/entry.server.tsx (2)
41-85
: Consider Refactoring Duplicate Functions to Improve MaintainabilityThe functions
handleBotRequest
andhandleBrowserRequest
have significant duplicated code. The main difference lies in the event handlersonAllReady
andonShellReady
used inrenderToPipeableStream
. Refactoring these into a single function with a parameter for the event handler can reduce redundancy and make the code more maintainable.Here's how you might refactor the code:
+function handleRenderRequest( + request: Request, + responseStatusCode: number, + responseHeaders: Headers, + reactRouterContext: EntryContext, + onReadyEvent: "onAllReady" | "onShellReady", +) { + return new Promise((resolve, reject) => { + let shellRendered = false; + const { pipe, abort } = renderToPipeableStream( + <ServerRouter context={reactRouterContext} url={request.url} />, + { + [onReadyEvent]() { + shellRendered = true; + const body = new PassThrough(); + const stream = createReadableStreamFromReadable(body); + + responseHeaders.set("Content-Type", "text/html"); + + resolve( + new Response(stream, { + headers: responseHeaders, + status: responseStatusCode, + }), + ); + + pipe(body); + }, + onShellError(error: unknown) { + reject(error); + }, + onError(error: unknown) { + responseStatusCode = 500; + if (shellRendered) { + console.error(error); + } + }, + }, + ); + + setTimeout(abort, ABORT_DELAY); + }); +} + function handleRequest( request: Request, responseStatusCode: number, responseHeaders: Headers, reactRouterContext: EntryContext, loadContext: AppLoadContext, ) { return isbot(request.headers.get("user-agent") || "") - ? handleBotRequest( + ? handleRenderRequest( request, responseStatusCode, responseHeaders, reactRouterContext, + "onAllReady", ) - : handleBrowserRequest( + : handleRenderRequest( request, responseStatusCode, responseHeaders, reactRouterContext, + "onShellReady", ); } - -function handleBotRequest( - request: Request, - responseStatusCode: number, - responseHeaders: Headers, - reactRouterContext: EntryContext, -) { - // existing code... -} - -function handleBrowserRequest( - request: Request, - responseStatusCode: number, - responseHeaders: Headers, - reactRouterContext: EntryContext, -) { - // existing code... -}Also applies to: 87-131
57-57
: Specify Character Encoding in Content-Type HeaderWhen setting the
Content-Type
header, it's good practice to include the character encoding to ensure proper rendering on the client side.Apply this diff to include
charset=utf-8
:responseHeaders.set("Content-Type", "text/html"); +responseHeaders.set("Content-Type", "text/html; charset=utf-8");
Also applies to: 103-103
examples/with-clerk-react-router/app/root.tsx (1)
29-29
: Consider enhancing the layout structure.The current body class setup could be improved for better layout control.
Consider this enhancement:
-<body className="m-0 flex min-h-screen min-w-[320px] items-center"> +<body className="m-0 flex min-h-screen min-w-[320px] flex-col items-center justify-center">This change provides better vertical centering and allows for proper stacking of elements.
examples/with-clerk-react-router/app/routes/api.uploadthing.ts (2)
17-17
: Remove console.log statements.Production code should not contain console.log statements. Consider using proper logging middleware or remove them entirely.
- console.log({ authObject }); // ... - console.log("Upload complete for userId:", metadata.userId); - console.log("file url", file.ufsUrl); + // Use proper logging middleware here if neededAlso applies to: 26-27
10-13
: Consider extracting file size limits to configuration.Hardcoded file size limits make it difficult to adjust based on environment or requirements.
+const FILE_SIZE_LIMITS = { + IMAGE_MAX_SIZE: "4MB", + VIDEO_MAX_SIZE: "16MB", +} as const; + videoAndImage: f({ - image: { maxFileSize: "4MB" }, - video: { maxFileSize: "16MB" }, + image: { maxFileSize: FILE_SIZE_LIMITS.IMAGE_MAX_SIZE }, + video: { maxFileSize: FILE_SIZE_LIMITS.VIDEO_MAX_SIZE }, })examples/with-clerk-react-router/app/routes/_index.tsx (2)
1-11
: Consider organizing imports by type/source.Group imports by:
- External dependencies
- Internal utilities
+// Authentication components import { SignedIn, SignedOut, SignInButton, SignOutButton, useAuth, UserButton, } from "@clerk/react-router"; + +// Types import type { MetaFunction } from "react-router"; +// Internal utilities import { UploadButton } from "~/utils/uploadthing";
26-53
: Consider adding loading states and progress indicators.The UI could benefit from loading states during authentication and upload processes to improve user experience.
examples/with-clerk-react-router/app/tailwind.css (1)
11-12
: Consider using CSS variables for color values.Using CSS variables would make it easier to maintain and modify the color scheme.
:root { + --text-light: #213547; + --text-dark: rgba(255, 255, 255, 0.87); + --bg-light: #ffffff; + --bg-dark: #242424; - color: rgba(255, 255, 255, 0.87); - background-color: #242424; + color: var(--text-dark); + background-color: var(--bg-dark); }examples/with-clerk-react-router/README.md (1)
10-21
: Format URLs as markdown links.Convert bare URLs to proper markdown links for better readability.
- https://uploadthing.com/dashboard + [UploadThing Dashboard](https://uploadthing.com/dashboard) - https://clerk.com/docs/quickstarts/remix#set-environment-keys + [Clerk Remix Quickstart](https://clerk.com/docs/quickstarts/remix#set-environment-keys) -Check out the docs at: https://docs.uploadthing.com/getting-started/remix +Check out the docs at: [UploadThing Remix Guide](https://docs.uploadthing.com/getting-started/remix)🧰 Tools
🪛 Markdownlint (0.37.0)
10-10: null
Bare URL used(MD034, no-bare-urls)
12-12: null
Bare URL used(MD034, no-bare-urls)
21-21: null
Bare URL used(MD034, no-bare-urls)
📜 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 (18)
.gitignore
(1 hunks)docs/src/app/(docs)/getting-started/remix/page.mdx
(1 hunks)examples/with-clerk-react-router/.env.example
(1 hunks)examples/with-clerk-react-router/README.md
(1 hunks)examples/with-clerk-react-router/app/entry.client.tsx
(1 hunks)examples/with-clerk-react-router/app/entry.server.tsx
(1 hunks)examples/with-clerk-react-router/app/env.ts
(1 hunks)examples/with-clerk-react-router/app/root.tsx
(1 hunks)examples/with-clerk-react-router/app/routes.ts
(1 hunks)examples/with-clerk-react-router/app/routes/_index.tsx
(1 hunks)examples/with-clerk-react-router/app/routes/api.uploadthing.ts
(1 hunks)examples/with-clerk-react-router/app/tailwind.css
(1 hunks)examples/with-clerk-react-router/app/utils/uploadthing.ts
(1 hunks)examples/with-clerk-react-router/package.json
(1 hunks)examples/with-clerk-react-router/postcss.config.js
(1 hunks)examples/with-clerk-react-router/tailwind.config.ts
(1 hunks)examples/with-clerk-react-router/tsconfig.json
(1 hunks)examples/with-clerk-react-router/vite.config.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (5)
- examples/with-clerk-react-router/postcss.config.js
- .gitignore
- examples/with-clerk-react-router/app/env.ts
- examples/with-clerk-react-router/package.json
- examples/with-clerk-react-router/.env.example
🧰 Additional context used
🪛 Markdownlint (0.37.0)
examples/with-clerk-react-router/README.md
10-10: null
Bare URL used
(MD034, no-bare-urls)
12-12: null
Bare URL used
(MD034, no-bare-urls)
21-21: null
Bare URL used
(MD034, no-bare-urls)
4-4: null
Images should have alternate text (alt text)
(MD045, no-alt-text)
🪛 GitHub Actions: CI
examples/with-clerk-react-router/app/entry.client.tsx
[warning] Code style issues found. File needs to be formatted using Prettier.
🔇 Additional comments (9)
examples/with-clerk-react-router/tsconfig.json (3)
2-10
: Well-structured include patterns!The file patterns appropriately cover both client and server-side TypeScript files while including React Router's type definitions.
28-29
: Correct build configuration for Vite!The
noEmit
setting is properly configured with a clear explanation of its purpose.
12-18
: Appropriate compiler settings for a modern Remix application!The configuration uses modern settings (ES2022) and includes necessary types for Remix and Vite.
Let's verify browser compatibility with ES2022:
✅ Verification successful
ES2022 target is compatible with the project's browser support
The project's browserslist configuration (
defaults, not ie <= 11
) confirms that modern browsers are targeted, which fully supports ES2022 features.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check package.json for browserslist config that might conflict with ES2022 fd package.json --exec cat {} | jq -r '.browserslist'Length of output: 288
examples/with-clerk-react-router/vite.config.ts (1)
1-1
: Verify the React Router Vite plugin import path.The import path
@react-router/dev/vite
seems incorrect. The official React Router package is@remix-run/router
.Let's verify the correct package:
Consider updating to use the official package if available.
examples/with-clerk-react-router/tailwind.config.ts (1)
4-4
: Verify Tailwind CSS Content PathsThe
content
array in your Tailwind CSS configuration is crucial for purging unused styles. Ensure that the glob pattern accurately reflects all the files that use Tailwind classes.Would you like to double-check that the glob pattern
./app/**/{**,.client,.server}/**/*.{js,jsx,ts,tsx}
includes all necessary files and doesn't miss any relevant components?examples/with-clerk-react-router/app/routes.ts (1)
1-6
: Well-Structured Route ConfigurationThe route configuration looks correct. Using
satisfies RouteConfig
ensures type safety, and the routes are properly defined.examples/with-clerk-react-router/app/utils/uploadthing.ts (1)
1-6
: LGTM! Clean and type-safe implementation.The implementation properly leverages type inference from the UploadRouter and follows best practices for React helper generation.
examples/with-clerk-react-router/app/routes/api.uploadthing.ts (1)
10-13
: Consider additional file type validation.The current implementation only validates file sizes. Consider adding mime type validation for better security.
Let's check if there are any file type validation utilities available:
docs/src/app/(docs)/getting-started/remix/page.mdx (1)
14-18
: LGTM! Clear version compatibility note.The added note clearly communicates version compatibility and provides a helpful link to the example.
📦 Bundle size comparison
|
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)
docs/src/app/(docs)/api-reference/server/page.mdx (1)
214-225
: LGTM! Consider adding a note about Remix v2.The Remix integration example is well-structured and correctly demonstrates the usage of
createRouteHandler
. Since this example uses Remix v2 patterns (as mentioned in the AI summary), consider adding a version note similar to other framework examples.<Tab> ```tsx + // Note: This example is for Remix v2 import { createRouteHandler } from "uploadthing/remix"; import { uploadRouter } from "~/server/uploadthing.ts"; export const { loader, action } = createRouteHandler({ router: uploadRouter, // config: { ... }, }); ``` </Tab>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/src/app/(docs)/api-reference/server/page.mdx
(2 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/src/app/(docs)/api-reference/server/page.mdx
[duplication] ~125-~125: Possible typo: you repeated a word.
Context: ...andleris different prior to
v6.3`. <Tabs tabs={["Next App Router", "Next Pages Dir", ...
(ENGLISH_WORD_REPEAT_RULE)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: e2e-node (backend-adapters)
🔇 Additional comments (1)
docs/src/app/(docs)/api-reference/server/page.mdx (1)
125-125
: LGTM! Framework list updated correctly.The addition of "Remix" to the framework list is consistent with the new tab content.
🧰 Tools
🪛 LanguageTool
[duplication] ~125-~125: Possible typo: you repeated a word.
Context: ...andleris different prior to
v6.3`. <Tabs tabs={["Next App Router", "Next Pages Dir", ...(ENGLISH_WORD_REPEAT_RULE)
Closes #1110
Summary by CodeRabbit
New Features
createRouteHandler
in the API reference for Remix.Documentation
Chores