chore: add a pg #1046
chore: add a pg #1046
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
|
WalkthroughThis pull request introduces a new Changes
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
vercel
bot
temporarily deployed
to
Preview – legacy-docs-uploadthing
📦 Bundle size comparison
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
🧹 Outside diff range and nitpick comments (15)
playground/components/skeleton.tsx (1)
1-8: Add ARIA attributes for better accessibility.The component structure is well-typed and follows React best practices. However, for better accessibility, consider adding appropriate ARIA attributes to indicate the loading state.
export function Skeleton({ className, children, loading = true, ...props -}: React.ComponentProps<"div"> & { loading?: boolean }) { +}: React.ComponentProps<"div"> & { loading?: boolean; "aria-label"?: string }) {playground/components/button.tsx (3)
9-13: Add JSDoc documentation for better prop types visibilityConsider adding JSDoc documentation to improve component usage understanding and IDE support.
+/** + * A reusable button component with predefined color schemes. + * @param props.color - Optional color scheme. Defaults to 'blue'. + * @param props.className - Additional CSS classes to apply. + */ export function Button( props: React.ComponentProps<"button"> & { color?: Color; }, ) {
16-19: Enhance className handling and button structureConsider these improvements:
- Use a utility like
clsxorcnfor more robust className handling- Allow children content by removing self-closing tag
- <button + <button {...props} - className={`${props.className} rounded px-2 py-1 text-sm/6 disabled:pointer-events-none ${colors[props.color ?? "blue"]}`} - /> + className={cn( + "rounded px-2 py-1 text-sm/6 disabled:pointer-events-none", + colors[props.color ?? "blue"], + props.className + )} + > + {props.children} + </button>Don't forget to add the utility function at the top of the file:
import { clsx, type ClassValue } from "clsx"; import { twMerge } from "tailwind-merge"; function cn(...inputs: ClassValue[]) { return twMerge(clsx(inputs)); }
1-21: Consider moving to a shared UI librarySince this appears to be a foundational button component with a well-thought-out design, consider moving it to a shared UI library if it's intended to be used beyond the playground. This would promote consistency and reusability across the main application.
playground/app/page.tsx (2)
7-13: Consider adding a loading fallback to Suspense.While Suspense is correctly used, providing a loading fallback would improve the user experience during the async loading state.
export default function Page() { return ( - <Suspense> + <Suspense fallback={<div className="p-4">Loading files...</div>}> <FileList /> </Suspense> ); }
19-23: Consider adding type safety for the header section.The header section could benefit from proper type definitions and semantic HTML.
- <div className="flex items-center justify-between"> + <header className="flex items-center justify-between"> - <h2 className="text-lg font-medium">Your files:</h2> + <h1 className="text-lg font-medium">Your files</h1> <Uploader /> - </div> + </header>playground/components/uploader.tsx (3)
26-28: Simplify content configuration.The empty allowedContent fragment seems unnecessary, and the button render function could be simplified.
content={{ - allowedContent: <></>, - button: ({ isUploading }) => (isUploading ? null : "Upload"), + button: ({ isUploading }) => isUploading ? "Uploading..." : "Upload", }}
29-32: Review the use of !important in CSS classes.The use of !important (
!text-sm/6and!h-0) suggests potential CSS specificity issues. Consider refactoring the CSS architecture to avoid relying on !important.
18-18: Document the purpose of the empty input prop.The empty input object might be confusing for other developers. Consider adding a comment explaining why it's empty or what values could be provided here.
- input={{}} + // Input options can be provided here if needed for the 'anything' endpoint + input={{}}playground/app/api/uploadthing/route.ts (2)
18-18: Consider adding input validation.The empty input schema (
z.object({})) accepts any input. Consider adding validation for expected metadata or user inputs that might be needed for the upload process.Example enhancement:
-.input(z.object({})) +.input(z.object({ + purpose: z.enum(['profile', 'document', 'other']).optional(), + description: z.string().max(200).optional(), +}))
35-38: Consider documenting configuration options.The empty config object might benefit from documentation explaining available configuration options, even if they're not needed now. This helps future maintainers understand what can be configured.
Example enhancement:
export const { GET, POST } = createRouteHandler({ router: uploadRouter, - config: {}, + config: { + // Available options: + // - handleError: (err: Error) => Response + // - onUploadError: (err: Error) => void + // Add configuration as needed + }, });playground/components/file-card.tsx (2)
26-26: Replace window.alert with a more user-friendly error handling approach.Consider using a toast notification or an error message component instead of
window.alertfor a better user experience.-window.alert(response.error); +toast.error(response.error); // or +setError(response.error);
12-12: Improve code maintainability and accessibility.Several improvements can be made:
- Extract status strings as constants
- Add aria-labels to buttons with emoji-only labels
- Extract common animation classes
+const FILE_STATUS = { + UPLOADED: 'Uploaded', + FAILED: 'Failed', +} as const; +const ANIMATION_CLASSES = { + LOADING: 'animate-pulse', +} as const; -const failed = file.status === "Failed"; +const failed = file.status === FILE_STATUS.FAILED; // In button JSX -className={`min-w-16 ${isOpening && "animate-pulse"} ${failed && "font-bold text-red-600"}`} +className={`min-w-16 ${isOpening && ANIMATION_CLASSES.LOADING} ${failed && "font-bold text-red-600"}`} // For delete button -{isDeleting ? "⏳" : "❌"} +<span aria-label={isDeleting ? "Deleting..." : "Delete file"}> + {isDeleting ? "⏳" : "❌"} +</span>Also applies to: 30-35, 46-46
playground/app/layout.tsx (2)
61-61: Ensure 'token.regions' is defined before accessAccessing
token.regions[0]without checking ifregionsis defined could lead to runtime errors ifregionsis undefined or an empty array. Add a check to prevent potential errors.Apply this diff:
- AppId: {token.appId} Region: {token.regions[0]} + AppId: {token.appId} Region: {token.regions?.[0] ?? "Unknown"}
70-75: Move 'Header' component outside 'SignInOut'Defining
HeaderinsideSignInOutcauses the component to be recreated on every render. MoveHeaderoutside to improve performance.Apply this refactor:
+function Header({ session }: { session: any }) { + return ( + <div className="flex gap-2 text-sm leading-6"> + <span className="font-medium">Hello 👋</span> + <pre>Session: {JSON.stringify(session ?? "null")}</pre> + </div> + ); +} async function SignInOut() { const session = await getSession(); - const Header = () => ( - <div className="flex gap-2 text-sm/6"> - <span className="font-medium">Hello 👋</span> - <pre>Session: {JSON.stringify(session ?? "null")}</pre> - </div> - ); if (!session) { return ( <div className="flex items-center justify-between"> - <Header /> + <Header session={session} /> <form action={signIn}> <Button>Sign In</Button> </form> </div> ); } return ( <div className="flex items-center justify-between"> - <Header /> + <Header session={session} /> <form action={signOut}> <Button>Sign Out</Button> </form> </div> ); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (16)
playground/.env.example(1 hunks)playground/app/api/uploadthing/route.ts(1 hunks)playground/app/layout.tsx(1 hunks)playground/app/page.tsx(1 hunks)playground/components/button.tsx(1 hunks)playground/components/file-card.tsx(1 hunks)playground/components/skeleton.tsx(1 hunks)playground/components/uploader.tsx(1 hunks)playground/lib/actions.ts(1 hunks)playground/lib/const.ts(1 hunks)playground/lib/data.ts(1 hunks)playground/next-env.d.ts(1 hunks)playground/next.config.ts(1 hunks)playground/package.json(1 hunks)playground/tsconfig.json(1 hunks)pnpm-workspace.yaml(1 hunks)
✅ Files skipped from review due to trivial changes (5)
- playground/.env.example
- playground/lib/const.ts
- playground/next-env.d.ts
- playground/package.json
- playground/tsconfig.json
🔇 Additional comments (13)
playground/next.config.ts (2)
4-5: Consider the implications of disabled error checking.
While disabling ESLint and TypeScript error checks might be acceptable for a playground environment, it could mask potential issues. Consider:
- Adding a comment explaining why these checks are disabled
- Using more granular ignores for specific rules instead of disabling all checks
6-9: Be cautious with experimental features.
The enabled experimental features (ppr and dynamicIO) are unstable and may have breaking changes in future Next.js releases. Consider:
- Adding a warning comment about the experimental nature
- Documenting any known limitations or issues
- Having a strategy to handle potential breaking changes
playground/components/skeleton.tsx (1)
9-10: LGTM!
Clean and efficient early return pattern that prevents unnecessary DOM nesting when not in loading state.
playground/lib/data.ts (1)
1-8: Consider the implications of using unstable Next.js API.
The unstable_cacheTag API is experimental and may change in future Next.js releases. Consider documenting this dependency and having a migration strategy in place.
Let's check the Next.js version and documentation:
✅ Verification successful
The use of unstable_cacheTag is appropriate for Next.js 14
The codebase is using Next.js 14.2.11, where unstable_cacheTag is part of the stable caching patterns, despite its naming prefix. This API is commonly used in Next.js 14+ for cache management and revalidation. The usage here is safe and follows current best practices.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check package.json for Next.js version
cat package.json | grep -A 5 '"dependencies"' | grep 'next":'
Length of output: 63
Script:
#!/bin/bash
# Let's try a more flexible search for the package.json
fd package.json --type f --exec cat {} \; | grep -A 5 '"dependencies"' | grep -i 'next'
# Also search for next.config files to understand the setup
fd next.config --type f --exec cat {} \;
# Look for other files using the unstable_cacheTag
rg "unstable_cacheTag" -A 2 -B 2
Length of output: 4685
playground/app/page.tsx (2)
1-5: LGTM! Clean import organization.
The imports are well-organized and all dependencies are properly utilized in the code.
22-22: Verify upload and list integration.
Ensure that the file list updates automatically after successful uploads.
✅ Verification successful
Let me verify the Uploader component implementation to ensure it's properly connected to these cache invalidation mechanisms.
Upload and list integration is properly implemented
The verification shows a complete integration chain:
- The Uploader component calls
router.refresh()after successful uploads - The upload endpoint properly invalidates the file list cache using
revalidateTag(CACHE_TAGS.LIST_FILES) - The file listing is cached with the same tag via
cacheTag(CACHE_TAGS.LIST_FILES)
This ensures the file list will automatically refresh after successful uploads.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the Uploader component includes proper cache invalidation
rg -A 5 "CACHE_TAGS.LIST_FILES" playground/
Length of output: 896
Script:
#!/bin/bash
# Check the Uploader component implementation
ast-grep --pattern 'function Uploader() {
$$$
}'
# Also check for any other Uploader component definition
rg -A 10 "const Uploader" playground/
Length of output: 4736
playground/components/uploader.tsx (2)
1-8: LGTM! Imports and directives are properly organized.
The "use client" directive is correctly placed, and imports are logically grouped between external and internal dependencies.
9-10: LGTM! Type-safe upload helpers are properly configured.
The helpers are correctly generated with type safety through UploadRouter. Let's verify the API route implementation:
✅ Verification successful
API route implementation correctly matches the client-side configuration
The API route properly implements the anything endpoint that matches the client-side configuration:
- Defines a blob route that accepts files up to 256MB
- Includes proper middleware for auth checks
- Handles upload completion with cache revalidation
- Exports the correct type
UploadRouterused by the client components
The type-safe upload helpers are properly configured and backed by a matching server implementation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the UploadRouter implementation in the API route
# Test: Check if the API route properly defines the 'anything' endpoint
ast-grep --pattern 'export const uploadRouter = createUploadRouter({
anything: $$$
})'
Length of output: 90
Script:
#!/bin/bash
# Let's try a broader search for uploadRouter and related implementations
# First, find any files that might contain the upload router configuration
rg -l "uploadRouter|UploadRouter|createUploadRouter" --type ts --type tsx
# Then search for actual implementation patterns
rg "createUploadRouter" -A 5
Length of output: 132
Script:
#!/bin/bash
# Let's try a more basic search without file type restrictions
# Search for any files containing upload-related configurations
rg -l "uploadRouter|UploadRouter|createUploadRouter"
# Also search for uploadthing related imports to find the implementation
rg "@uploadthing" -A 3
Length of output: 153720
Script:
#!/bin/bash
# Let's check the API route implementation in the playground directory
rg -A 10 "createUploadRouter" playground/app/api/uploadthing/route.ts
# Also check for any UploadRouter type definitions
rg -A 5 "type UploadRouter" playground/app/api/uploadthing/route.ts
Length of output: 280
Script:
#!/bin/bash
# Let's look at the entire content of the route file to see the router implementation
cat playground/app/api/uploadthing/route.ts
# Also check for any related configuration files
rg -l "uploadRouter" playground/app/
Length of output: 1108
playground/app/api/uploadthing/route.ts (2)
1-13: LGTM! Imports are well-organized and necessary.
The imports are logically grouped and all serve a clear purpose in the implementation.
17-17: Consider reducing the file size limit for the playground environment.
A 256MB file size limit seems quite high for a testing environment. Consider reducing this to a more reasonable size (e.g., 10MB) to prevent potential abuse or resource exhaustion.
playground/lib/actions.ts (2)
1-12: LGTM! Imports and initialization are well structured.
The server-side setup is correct with appropriate imports and efficient UTApi initialization.
46-61: Verify cache invalidation strategy.
The cache invalidation after file deletion needs verification to ensure consistency.
Consider these improvements:
export async function deleteFile(key: string) {
+ if (!key?.trim()) {
+ return {
+ success: false as const,
+ error: "Invalid file key",
+ };
+ }
+
const session = await getSession();
if (!session) {
return {
success: false as const,
error: "Unauthorized",
};
}
- await utapi.deleteFiles(key);
- revalidateTag(CACHE_TAGS.LIST_FILES);
-
- return {
- success: true as const,
- };
+ try {
+ await utapi.deleteFiles(key);
+ revalidateTag(CACHE_TAGS.LIST_FILES);
+ return {
+ success: true as const,
+ };
+ } catch (error) {
+ console.error("Failed to delete file:", error);
+ return {
+ success: false as const,
+ error: "Failed to delete file",
+ };
+ }
}✅ Verification successful
Let me gather more information about the cache tag usage and data fetching patterns to ensure a comprehensive verification.
Let me check one final aspect about the file listing implementation to complete the verification.
Cache invalidation strategy is correctly implemented
The cache invalidation strategy is properly implemented for the file deletion operation. Here's why:
- The
listFilesfunction inplayground/lib/data.tsuses theCACHE_TAGS.LIST_FILEStag to cache the file listing. - The
deleteFilefunction inplayground/lib/actions.tscorrectly invalidates this cache usingrevalidateTag(CACHE_TAGS.LIST_FILES)after deletion. - The same cache tag is consistently used across the codebase, including in the upload route (
playground/app/api/uploadthing/route.ts), ensuring cache coherency.
The suggested improvements in the review comment for error handling and input validation are good to have, but the cache invalidation strategy itself is correctly implemented.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for other places using the LIST_FILES cache tag
rg "CACHE_TAGS.LIST_FILES" --type ts
# Look for other cache invalidation patterns
rg "revalidateTag|revalidatePath" --type ts
Length of output: 1040
Script:
#!/bin/bash
# Check the implementation of data fetching using the cache tag
ast-grep --pattern 'export $_ function $_($$$) {
$$$
cacheTag(CACHE_TAGS.LIST_FILES);
$$$
}'
# Check the cache tags definition
rg "CACHE_TAGS" -B2 -A2 --type ts
# Look for any file listing functionality
ast-grep --pattern 'export $_ function $_($$_) {
$$$
getFiles($$$)
$$$
}'
Length of output: 2163
Script:
#!/bin/bash
# Check the implementation of listFiles function
ast-grep --pattern 'export $_ function listFiles() {
$$$
}'
# Check for any other functions that might be affected by file operations
rg "utapi\." --type ts -B2 -A2
Length of output: 20689
playground/components/file-card.tsx (1)
5-5: Verify the implementation of imported file operations.
Let's ensure the imported functions deleteFile and getFileUrl are properly implemented and handle all error cases.
✅ Verification successful
Both file operations are properly implemented with authentication and error handling
The verification confirms that both getFileUrl and deleteFile functions:
- Implement session-based authentication checks
- Return typed error responses for unauthorized access
- Handle file operations through the
utapiutility - Include cache invalidation for file listing (in
deleteFile)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check the implementation of file operation functions
echo "Checking file operations implementation..."
rg -A 10 "export (async )?function (deleteFile|getFileUrl)" playground/lib/actions.ts
Length of output: 695
| export async function listFiles() { | ||
| "use cache"; | ||
| cacheTag(CACHE_TAGS.LIST_FILES); | ||
|
|
||
| const { files } = await utapi.listFiles(); | ||
|
|
||
| return files; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling and type safety.
The function could benefit from:
- Error handling for the API call
- TypeScript types for the returned files array
Consider this implementation:
-export async function listFiles() {
+export async function listFiles(): Promise<Array<UploadedFile>> {
"use cache";
cacheTag(CACHE_TAGS.LIST_FILES);
- const { files } = await utapi.listFiles();
+ try {
+ const { files } = await utapi.listFiles();
+ return files;
+ } catch (error) {
+ console.error("Failed to fetch files:", error);
+ throw new Error("Failed to fetch files");
+ }
- return files;
}📝 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 async function listFiles() { | |
| "use cache"; | |
| cacheTag(CACHE_TAGS.LIST_FILES); | |
| const { files } = await utapi.listFiles(); | |
| return files; | |
| } | |
| export async function listFiles(): Promise<Array<UploadedFile>> { | |
| "use cache"; | |
| cacheTag(CACHE_TAGS.LIST_FILES); | |
| try { | |
| const { files } = await utapi.listFiles(); | |
| return files; | |
| } catch (error) { | |
| console.error("Failed to fetch files:", error); | |
| throw new Error("Failed to fetch files"); | |
| } | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Outside diff range and nitpick comments (7)
playground/eslint.config.js (2)
1-6: Consider adding more specific ignore patternsThe current ignore patterns are good but could be more comprehensive. Consider adding other common build/cache directories that might appear in a Next.js project.
{ - ignores: [".next", "node_modules"], + ignores: [".next", "node_modules", ".turbo", "dist", "coverage", "out"], },
17-24: Consider adding explicit project referenceWhile
projectService: trueis set, it might be more explicit to specify the TypeScript project file.parserOptions: { projectService: true, tsconfigRootDir: import.meta.dirname, + project: "./tsconfig.json", },playground/lib/data.ts (2)
10-17: Document the caching behavior.While the caching implementation is correct, it would be beneficial to add JSDoc comments explaining:
- The caching duration
- When/how the cache gets invalidated
- The structure of the returned data
+/** + * Fetches and caches the list of uploaded files. + * Cache is invalidated when CACHE_TAGS.LIST_FILES is revalidated. + * @returns {Promise<Array<UploadedFile>>} List of uploaded files + */ export async function listFiles() { "use cache"; cacheTag(CACHE_TAGS.LIST_FILES);
21-25: Enhance type safety of the Session interface.The
rolefield could benefit from being more strictly typed using a union type of allowed values.+type UserRole = "admin" | "user" | "guest"; + export interface Session { sub: number; iat: number; - role: string; + role: UserRole; }playground/lib/actions.ts (2)
30-33: Add error handling and cleanup delay to signOut.Consider enhancing the signOut function for better reliability:
export async function signOut() { - (await cookies()).delete(SESSION_COOKIE_NAME); - redirect("/"); + try { + await cookies().delete(SESSION_COOKIE_NAME); + // Allow time for cookie deletion and cleanup + setTimeout(() => redirect("/"), 100); + } catch (error) { + console.error("Error during sign out:", error); + // Still redirect even if cookie deletion fails + redirect("/"); + } }
1-66: Add documentation about playground usage.Since this is a playground for internal testing, consider adding a file-level comment to prevent misuse:
"use server"; +/** + * @fileoverview + * This is a playground implementation for internal testing purposes. + * Contains simplified authentication and file management. + * DO NOT USE THIS CODE IN PRODUCTION. + */ import { revalidateTag } from "next/cache";playground/app/api/uploadthing/route.ts (1)
18-18: Consider defining an input schema for request validationThe input schema is currently empty (
z.object({})), which means no validation is performed on the incoming request data. If your route expects specific input, defining a meaningful Zod schema will enhance robustness by ensuring that only valid data is processed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
playground/app/api/uploadthing/route.ts(1 hunks)playground/app/layout.tsx(1 hunks)playground/components/button.tsx(1 hunks)playground/components/file-card.tsx(1 hunks)playground/components/skeleton.tsx(1 hunks)playground/eslint.config.js(1 hunks)playground/lib/actions.ts(1 hunks)playground/lib/data.ts(1 hunks)playground/package.json(1 hunks)playground/tsconfig.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- playground/components/button.tsx
- playground/components/file-card.tsx
- playground/components/skeleton.tsx
- playground/tsconfig.json
🔇 Additional comments (9)
playground/eslint.config.js (1)
7-10: LGTM! Comprehensive TypeScript ESLint configuration
The configuration extends all the recommended rulesets from typescript-eslint, providing a robust foundation for TypeScript linting.
playground/package.json (2)
1-4: LGTM! Project metadata is well configured.
The configuration correctly marks the package as private and uses modern ES modules.
11-19: Verify stability with experimental package versions.
The dependencies configuration has potential stability risks:
- Using Next.js canary with an unreleased version of React (18.3.1)
- Workspace dependencies without explicit versions
Let's check if these versions are compatible:
playground/lib/data.ts (2)
19-19: LGTM! Well-typed using TypeScript utility types.
The type definition will automatically stay in sync with any changes to the listFiles function's return type.
1-1: Consider the stability implications of using unstable Next.js API.
The unstable_cacheTag API is marked as unstable, which means it might change in future Next.js releases. Consider documenting this dependency and having a migration strategy in place.
playground/lib/actions.ts (3)
1-12: LGTM: Imports and setup are properly configured.
The imports and initial setup are well-structured and all dependencies are properly utilized throughout the code.
14-28: Previous security concerns are still valid.
The existing review comment about security improvements for the signIn implementation remains applicable.
35-49: Previous error handling suggestions are still valid.
The existing review comment about enhancing error handling and input validation in getFileUrl remains applicable.
playground/app/layout.tsx (1)
73-73:
Avoid rendering entire session object in the UI
Displaying the entire session object using JSON.stringify might expose sensitive user information. It's safer to display only the necessary details.
Apply this change to display minimal session information:
-<pre>Session: {JSON.stringify(session ?? "null")}</pre>
+<pre>User: {session?.user?.name ?? "Guest"}</pre>Ensure that the properties you access exist to prevent errors.
Likely invalid or redundant comment.
More templates
commit: |
vercel
bot
temporarily deployed
to
Preview – legacy-docs-uploadthing
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
packages/uploadthing/src/types.ts(1 hunks)playground/README.md(1 hunks)playground/app/layout.tsx(1 hunks)playground/package.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- playground/README.md
🚧 Files skipped from review as they are similar to previous changes (1)
- playground/app/layout.tsx
🔇 Additional comments (3)
playground/package.json (2)
1-4: LGTM! Project metadata is well configured.
The configuration is appropriate for a playground project with proper privacy settings and module type.
16-17:
Fix incorrect React version.
React version 18.3.1 doesn't exist (latest stable is 18.2.0). This will cause installation failures.
- "react": "18.3.1",
- "react-dom": "18.3.1",
+ "react": "18.2.0",
+ "react-dom": "18.2.0",packages/uploadthing/src/types.ts (1)
27-27: LGTM! Clean addition of the UploadThingToken export.
The new export aligns with the PR objective of creating a playground with upload functionality, likely used for authentication purposes. The change follows TypeScript best practices for type exports.
There was a problem hiding this comment.
Actionable comments posted: 15
🧹 Outside diff range and nitpick comments (23)
playground-v6/lib/const.ts (2)
1-1: Consider enhancing type safety and security.While the constant follows naming conventions, consider these improvements:
- Add a type annotation for better type safety
- Use a more specific cookie name to avoid conflicts
-export const SESSION_COOKIE_NAME = "ut-session"; +export const SESSION_COOKIE_NAME: string = "uploadthing-playground-session";
3-5: Add type safety to cache tags.Consider adding TypeScript types to prevent typos and ensure consistent usage across the codebase.
-export const CACHE_TAGS = { +export const CACHE_TAGS = { LIST_FILES: "list_files", -}; +} as const; + +export type CacheTag = typeof CACHE_TAGS[keyof typeof CACHE_TAGS];playground-v6/components/skeleton.tsx (3)
1-1: Consider using more specific import from clsxThe current import could be more specific to reduce bundle size.
-import cx from "clsx"; +import { clsx as cx } from "clsx";
11-24: Consider enhancing accessibility attributesWhile the current implementation includes good accessibility features, it could be enhanced further.
- <div className="relative inline-block" role="status"> + <div className="relative inline-block" role="status" aria-busy={loading}> <div className={cx( className, "absolute inset-0 animate-pulse rounded-md bg-gray-300", )} - aria-hidden="true" + aria-hidden="true" + data-testid="skeleton-pulse" {...props} /> <div className="invisible" aria-hidden="true"> {children} </div> </div>The changes:
- Add
aria-busyto better communicate loading state- Add
data-testidfor easier testing
14-17: Consider adding customizable animation durationThe animation pulse timing is currently fixed. Consider making it customizable for different use cases.
+type SkeletonProps = React.ComponentProps<"div"> & { + loading?: boolean; + animationDuration?: 'fast' | 'normal' | 'slow'; +}; +const ANIMATION_CLASSES = { + fast: 'animate-pulse-fast', + normal: 'animate-pulse', + slow: 'animate-pulse-slow', +} as const; -}: React.ComponentProps<"div"> & { loading?: boolean }) +}: SkeletonProps) // ... in the className className, - "absolute inset-0 animate-pulse rounded-md bg-gray-300", + "absolute inset-0 rounded-md bg-gray-300", + ANIMATION_CLASSES[animationDuration ?? 'normal'],playground-v6/eslint.config.js (2)
11-16: Consider documenting rule suppressions.The disabled rules have important security and performance implications:
require-awaitprevents unnecessary async overheadonly-throw-errorensures proper error handlingConsider adding comments explaining why these rules are disabled or if they could be enabled with specific configurations.
rules: { + // TODO: Document why these rules are disabled "@typescript-eslint/require-await": "off", "@typescript-eslint/only-throw-error": "off", },
17-24: Consider performance implications of projectService.Enabling
projectService: truecan impact performance as it keeps the TypeScript compiler service running. This is useful for accurate type checking but might slow down the linting process.Consider if this is necessary for the playground environment.
playground-v6/components/button.tsx (1)
1-8: Add focus states and consider dark mode supportThe color variants are well-defined, but they're missing focus states which is important for keyboard navigation. Also, consider adding dark mode variants for better theme support.
const colors = { - blue: "bg-blue-600 hover:bg-blue-700 text-blue-50", + blue: "bg-blue-600 hover:bg-blue-700 focus:bg-blue-700 text-blue-50 dark:bg-blue-500", - red: "bg-red-600 hover:bg-red-700 text-red-50", + red: "bg-red-600 hover:bg-red-700 focus:bg-red-700 text-red-50 dark:bg-red-500", - lightgray: "bg-gray-300 hover:bg-gray-400 text-gray-950", + lightgray: "bg-gray-300 hover:bg-gray-400 focus:bg-gray-400 text-gray-950 dark:bg-gray-600", - outline: "bg-transparent border hover:bg-gray-100", + outline: "bg-transparent border hover:bg-gray-100 focus:bg-gray-100 dark:hover:bg-gray-800 dark:focus:bg-gray-800", };playground-v6/app/page.tsx (1)
1-5: Consider using path aliases and adding type imports.To improve maintainability and type safety:
- Replace relative imports with path aliases (e.g.,
@/components/*)- Import types for the file prop to ensure type safety
import { Suspense } from "react"; -import { FileCard } from "../components/file-card"; -import { Uploader } from "../components/uploader"; -import { listFiles } from "../lib/data"; +import { FileCard } from "@/components/file-card"; +import { Uploader } from "@/components/uploader"; +import { listFiles } from "@/lib/data"; +import type { FileType } from "@/types";playground-v6/components/uploader.tsx (4)
9-10: Consider adding error handling for helper generation.While the type safety is good, the helper generation functions might fail if the router is misconfigured.
Consider wrapping the generation in a try-catch:
-export const UTButton = generateUploadButton<UploadRouter>(); -export const { useUploadThing } = generateReactHelpers<UploadRouter>(); +export const UTButton = (() => { + try { + return generateUploadButton<UploadRouter>(); + } catch (error) { + console.error("Failed to generate upload button:", error); + throw error; + } +})(); + +export const { useUploadThing } = (() => { + try { + return generateReactHelpers<UploadRouter>(); + } catch (error) { + console.error("Failed to generate upload helpers:", error); + throw error; + } +})();
26-28: Clarify the purpose of empty allowedContent and improve button state handling.The empty fragment for allowedContent and the null rendering during upload might not provide the best user experience.
Consider this improvement:
content={{ - allowedContent: <></>, + allowedContent: ( + <p className="text-sm text-gray-500"> + Upload any file type + </p> + ), - button: ({ isUploading }) => (isUploading ? null : "Upload"), + button: ({ isUploading }) => ( + isUploading ? "Uploading..." : "Upload" + ), }}
22-24: Add loading state handling during refresh.The current implementation might not provide enough feedback during the refresh operation.
Consider adding a loading state:
+import { useState } from "react"; + export function Uploader() { const router = useRouter(); + const [isRefreshing, setIsRefreshing] = useState(false); + + const handleUploadComplete = async () => { + setIsRefreshing(true); + try { + await router.refresh(); + } finally { + setIsRefreshing(false); + } + }; + return ( <UTButton // ... other props - onClientUploadComplete={() => { - router.refresh(); - }} + onClientUploadComplete={handleUploadComplete} + disabled={isRefreshing} /> ); }
29-32: Avoid using !important in CSS classes.Using !important flags (
!text-sm/6and!h-0) can lead to specificity wars and maintenance issues.Consider using more specific selectors or adjusting the base styles:
appearance={{ - button: "!text-sm/6", - allowedContent: "!h-0", + button: "text-sm leading-6", + allowedContent: "h-0", }}If these styles are being overridden, consider reviewing the CSS architecture to avoid specificity conflicts.
playground-v6/app/api/uploadthing/route.ts (3)
19-28: Consider implementing rate limiting in the middleware.While session validation is implemented, there's no rate limiting to prevent abuse. Consider adding rate limiting based on the session ID.
Also, consider moving the console.log to a debug logging utility:
- console.log("middleware ::", session.sub, opts.input); + logger.debug("Upload middleware", { sessionId: session.sub, input: opts.input });
29-32: Consider error handling in onUploadComplete callback.The callback should handle potential errors during cache revalidation to ensure robust error reporting.
.onUploadComplete(async (opts) => { - console.log("Upload complete", opts.file); - revalidateTag(CACHE_TAGS.LIST_FILES); + try { + logger.debug("Upload complete", { file: opts.file }); + await revalidateTag(CACHE_TAGS.LIST_FILES); + } catch (error) { + logger.error("Failed to revalidate cache", { error }); + throw new Error("Failed to update file list"); + } }),
37-40: Consider configuring additional options in createRouteHandler.The empty config object could be utilized to configure important handler options such as:
- CORS settings
- Custom error handling
- Request body size limits
Example configuration:
export const { GET, POST } = createRouteHandler({ router: uploadRouter, - config: {}, + config: { + handleError: (err, req) => { + logger.error("Upload error", { error: err, url: req.url }); + return { message: "Upload failed. Please try again." }; + }, + }, });playground-v6/lib/actions.ts (3)
30-33: Consider adding error handling and flexible navigation.The signOut implementation could be more robust and flexible.
Consider this enhanced implementation:
export async function signOut() { - (await cookies()).delete(SESSION_COOKIE_NAME); - redirect("/"); + try { + (await cookies()).delete(SESSION_COOKIE_NAME); + return { success: true }; + } catch (error) { + console.error("Failed to sign out:", error); + return { success: false, error: "Failed to sign out" }; + } }
35-49: Enhance error handling and type safety.While the implementation is solid, it could benefit from more robust error handling and type validation.
Consider these improvements:
export async function getFileUrl(key: string) { + if (!key) { + return { + success: false as const, + error: "Invalid file key", + }; + } + const session = await getSession(); if (!session) { return { success: false as const, error: "Unauthorized", }; } - const { url } = await utapi.getSignedURL(key); - return { - success: true as const, - url, - }; + try { + const { url } = await utapi.getSignedURL(key); + if (!url) throw new Error("Failed to generate URL"); + return { + success: true as const, + url, + }; + } catch (error) { + console.error("Failed to get file URL:", error); + return { + success: false as const, + error: "Failed to generate file URL", + }; + } }
51-66: Add error handling and input validation.The delete operation should be more defensive with proper error handling and validation.
Consider these improvements:
export async function deleteFile(key: string) { + if (!key) { + return { + success: false as const, + error: "Invalid file key", + }; + } + const session = await getSession(); if (!session) { return { success: false as const, error: "Unauthorized", }; } - await utapi.deleteFiles(key); - revalidateTag(CACHE_TAGS.LIST_FILES); - - return { - success: true as const, - }; + try { + await utapi.deleteFiles(key); + revalidateTag(CACHE_TAGS.LIST_FILES); + return { + success: true as const, + }; + } catch (error) { + console.error("Failed to delete file:", error); + return { + success: false as const, + error: "Failed to delete file", + }; + } }playground-v6/components/file-card.tsx (2)
28-29: Replace window.alert with a better error handling UIUsing
window.alertprovides a poor user experience. Consider using a toast notification or an inline error message component.- window.alert(response.error); + toast.error(response.error); // or + setError(response.error);
25-27: Handle potential popup blocker issuesThe
window.opencall might be blocked by popup blockers. Consider adding error handling for this case.if (response.success) { - window.open(response.url, "_blank"); + const newWindow = window.open(response.url, "_blank"); + if (newWindow === null) { + toast.error("Popup blocked. Please allow popups for this site."); + } } else {playground-v6/app/layout.tsx (2)
47-47: Make skeleton loader length more dynamic.Using a hardcoded length for the skeleton loader might not match the actual content width across different viewports.
-<Skeleton>{".".repeat(100)}</Skeleton> +<Skeleton className="w-[300px]" />
62-67: Move Header component outside SignInOut.Defining components inside other components can lead to unnecessary re-renders and prevents reusability.
+const Header = ({ session }: { session: unknown }) => ( + <div className="flex gap-2 text-sm/6"> + <span className="font-medium">Hello 👋</span> + <pre>Session: {JSON.stringify(session ?? "null")}</pre> + </div> +); + async function SignInOut() { const session = await getSession(); - const Header = () => ( - <div className="flex gap-2 text-sm/6"> - <span className="font-medium">Hello 👋</span> - <pre>Session: {JSON.stringify(session ?? "null")}</pre> - </div> - );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (18)
playground-v6/.env.example(1 hunks)playground-v6/app/api/uploadthing/route.ts(1 hunks)playground-v6/app/layout.tsx(1 hunks)playground-v6/app/page.tsx(1 hunks)playground-v6/components/button.tsx(1 hunks)playground-v6/components/file-card.tsx(1 hunks)playground-v6/components/skeleton.tsx(1 hunks)playground-v6/components/uploader.tsx(1 hunks)playground-v6/eslint.config.js(1 hunks)playground-v6/lib/actions.ts(1 hunks)playground-v6/lib/const.ts(1 hunks)playground-v6/lib/data.ts(1 hunks)playground-v6/next-env.d.ts(1 hunks)playground-v6/next.config.ts(1 hunks)playground-v6/package.json(1 hunks)playground-v6/tsconfig.json(1 hunks)playground/next-env.d.ts(1 hunks)pnpm-workspace.yaml(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- playground-v6/.env.example
- playground-v6/next-env.d.ts
- playground-v6/tsconfig.json
🚧 Files skipped from review as they are similar to previous changes (2)
- playground/next-env.d.ts
- pnpm-workspace.yaml
🔇 Additional comments (18)
playground-v6/next.config.ts (2)
6-9: LGTM! Appropriate use of experimental features for playground.
The use of experimental features (ppr and dynamicIO) is well-suited for a playground environment where you're testing new functionality. The type safety is properly maintained with the satisfies NextConfig operator.
4-5: Consider enabling build-time checks for better development experience.
While ignoring ESLint and TypeScript errors might be convenient for a playground, it could mask potential issues during development. Consider:
- Enabling these checks during development
- Adding specific ignore patterns instead of ignoring all errors
playground-v6/components/skeleton.tsx (1)
3-8: LGTM! Well-typed props interface
The component's props interface is well-defined, extending React.ComponentProps<"div"> for complete type safety while adding the custom loading prop.
playground-v6/eslint.config.js (2)
1-6: LGTM! Modern ESLint configuration approach.
Good use of the new flat config format and typescript-eslint unified package. The ignore patterns are appropriate for a Next.js project.
7-10: Verify TypeScript setup for type-checked rules.
The configuration extends type-checked configs which require proper TypeScript setup. Ensure that:
- A valid tsconfig.json exists
- All necessary TypeScript dependencies are installed
✅ Verification successful
Let me verify the TypeScript configuration content to ensure it's properly set up for the type-checked rules.
Let me check one final thing to verify the actual dependencies being used, as they might be using the new flat config format.
TypeScript setup is correctly configured for type-checked rules
The project has all the necessary components properly set up:
- Valid
tsconfig.jsonwith appropriate compiler options - TypeScript v5.6.3 installed as dev dependency
- typescript-eslint v8.13.0 installed (new flat config package that combines parser and plugin)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for TypeScript configuration and dependencies
echo "Checking for tsconfig.json..."
fd tsconfig.json playground-v6
echo "Checking for TypeScript dependencies..."
cat playground-v6/package.json | jq '.dependencies["typescript"], .devDependencies["typescript"]'
Length of output: 331
Script:
#!/bin/bash
# Check TypeScript configuration content
echo "TypeScript config content:"
cat playground-v6/tsconfig.json
echo -e "\nChecking for @typescript-eslint dependencies..."
cat playground-v6/package.json | jq '.dependencies["@typescript-eslint/parser"], .devDependencies["@typescript-eslint/parser"], .dependencies["@typescript-eslint/eslint-plugin"], .devDependencies["@typescript-eslint/eslint-plugin"]'
Length of output: 892
Script:
#!/bin/bash
# Check all dependencies that might include typescript-eslint
cat playground-v6/package.json | jq '.dependencies, .devDependencies' | grep -i typescript-eslint
Length of output: 132
playground-v6/components/button.tsx (2)
9-9: LGTM!
Good use of TypeScript's keyof typeof to derive the Color type from the colors object, ensuring type safety and maintainability.
11-26: Verify Button component usage in Uploader
Let's verify how this Button component is used in the Uploader component to ensure our suggested changes are compatible.
✅ Verification successful
Let me gather more information about the color prop usage in these components.
Button component usage is compatible with current implementation
The verification shows that the Button component is used correctly in all instances:
- In
file-card.tsx:- Uses
color="outline"which is supported - Uses
color={failed ? "lightgray" : "blue"}which uses supported colors
- Uses
- In
layout.tsx:- Uses default color (blue) without explicitly setting the color prop
The component usage aligns perfectly with the current implementation where color is an optional prop defaulting to "blue", and the code supports all the color values being used.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for Button component usage
rg -l "Button.*from.*button" playground-v6/
rg -A 5 "<Button" playground-v6/
Length of output: 1832
Script:
#!/bin/bash
# Check the Button imports and color prop usage
rg -B 2 -A 2 "import.*Button" playground-v6/components/file-card.tsx
rg -B 2 -A 2 "import.*Button" playground-v6/app/layout.tsx
# Get full context of Button usage in file-card
cat playground-v6/components/file-card.tsx
Length of output: 2340
playground-v6/package.json (2)
1-4: LGTM! Basic configuration is well-structured.
The project configuration correctly specifies ES modules and is marked as private, which is appropriate for an internal testing playground.
11-20: Verify stability of experimental dependencies.
The project uses several experimental/unreleased versions:
- Next.js canary
- React 18.3.1 (unreleased)
- @uploadthing v6 packages via npm aliases
While this is acceptable for a playground, please ensure these versions are compatible with each other.
playground-v6/lib/data.ts (2)
1-8: Consider the implications of using unstable Next.js API.
The unstable_cacheTag API is experimental and may change in future Next.js releases. Consider adding a comment documenting this dependency or exploring stable alternatives if available.
Let's verify if there are any stable alternatives in the codebase:
#!/bin/bash
# Search for cache-related imports to find potential alternatives
rg -l "import.*cache.*from.*next" 21-25: Consider security implications of the Session interface.
The Session interface contains sensitive information (sub, role). Ensure that this data is properly sanitized before use and not exposed to the client side.
Let's verify the usage of session data in components:
✅ Verification successful
Let me try a different search strategy to verify the usage of Session interface and potential security implications.
Let me try one more search to understand how the session data is being handled, particularly focusing on the getSession function and any client-side usage.
Based on the gathered information, I can now provide the final response:
Session data handling appears to be properly server-side restricted
The Session interface is only used in server-side contexts:
- Stored in HTTP-only cookies via
getSession() - No evidence of client-side exposure or usage
- Implementation in
data.tsandactions.tsfollows secure patterns with server-side only access
The current implementation already follows security best practices by:
- Keeping session data server-side
- Using cookie-based storage
- No direct client-side exposure of sensitive fields (
sub,role)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for components using session data
rg -A 5 "getSession|Session" --type tsx
Length of output: 84
Script:
#!/bin/bash
# Search for Session interface usage across all file types
rg -A 5 "Session.*{|: Session"
# Also search for specific sensitive fields
rg -A 3 "sub:|role:"
Length of output: 3980
Script:
#!/bin/bash
# Search for getSession implementation and usage
rg -A 10 "getSession.*Promise<Session"
# Search for any client-side components or hooks using session
rg -A 5 "'use client'.*Session|useSession"
Length of output: 739
playground-v6/app/page.tsx (1)
4-4: Verify upload integration and error handling.
The Uploader component is integrated but we should verify its implementation includes proper error handling and upload progress indicators.
Also applies to: 22-22
✅ Verification successful
Based on the results, I can now generate the final response since we found the implementation of the Uploader component in playground-v6/components/uploader.tsx which shows proper error handling and upload status indicators.
Upload integration is properly implemented with error handling
The Uploader component includes:
- Error handling via
onUploadErrorcallback that shows error messages to users - Upload status indication through the
isUploadingstate in button content - Success handling with
onClientUploadCompletethat refreshes the page
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if Uploader component handles errors and shows progress
ast-grep --pattern 'export function Uploader() {
$$$
onError($$$) {
$$$
}
$$$
}' playground-v6/components/uploader.tsx
# Check if upload endpoint has proper error handling
ast-grep --pattern 'export const uploadRouter = {
$$$
.middleware($$$)
.onUploadError($$$)
$$$
}' playground-v6/app/api/uploadthing/core.ts
Length of output: 371
Script:
#!/bin/bash
# Let's first check if the files exist and their locations
fd -t f "uploader.tsx"
fd -t f "core.ts"
# Check the content of the uploader component
fd -t f "uploader.tsx" -x cat {}
# Look for any error handling patterns in the codebase
rg -A 3 "onError|error" --type ts --type tsx
Length of output: 16306
playground-v6/components/uploader.tsx (1)
1-8: LGTM! Imports and directives are properly organized.
The "use client" directive is correctly placed, and imports are logically grouped.
playground-v6/app/api/uploadthing/route.ts (2)
1-12: LGTM! Imports are well-organized and necessary.
The imports are logically grouped and all are utilized in the implementation.
17-18: Consider adding file type validation and reducing file size limit.
The current implementation accepts any file type with a large size limit of 256MB, which could pose security risks. Consider:
- Restricting allowed file types using the
fileTypesoption. - Reducing the file size limit unless 256MB is specifically required.
Example implementation with file type restrictions:
- anything: fileRoute({ blob: { maxFileSize: "256MB" } })
+ anything: fileRoute({
+ image: { maxFileSize: "4MB", maxFileCount: 4 },
+ pdf: { maxFileSize: "16MB", maxFileCount: 1 },
+ })playground-v6/lib/actions.ts (1)
1-12: LGTM: Imports and initialization are well-structured.
The server-side directive, imports, and UTApi initialization are properly configured.
playground-v6/components/file-card.tsx (2)
10-14: LGTM! Well-structured component setup
The component follows React best practices with proper TypeScript types and efficient state management using useTransition for async operations.
6-6: Verify error handling in imported actions
The component relies on getFileUrl and deleteFile actions for core functionality. Let's verify their implementation to ensure proper error handling.
✅ Verification successful
Error handling is properly implemented in the actions
Both getFileUrl and deleteFile actions have appropriate error handling:
- Authentication checks with proper error responses
- Type-safe success/error return values using
as const - Session validation before performing operations
- Cache invalidation after successful deletion
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check implementation of file actions
# Expected: Find error handling patterns in the actions
# Search for the action implementations
ast-grep --pattern 'export async function $name($$$) {
try {
$$$
} catch $$$
}'
# Backup: Search for any error handling patterns
rg -A 5 'try|catch|throw' playground-v6/lib/actions.ts
Length of output: 146
Script:
#!/bin/bash
# Let's first check if the actions file exists and see its content
fd actions.ts playground-v6/lib/ --exec cat {}
# Also search for any error-related patterns in a broader context
rg -A 10 'export (async )?function (getFileUrl|deleteFile)' playground-v6/lib/
Length of output: 2641
| export async function listFiles() { | ||
| "use cache"; | ||
| cacheTag(CACHE_TAGS.LIST_FILES); | ||
|
|
||
| const { files } = await utapi.listFiles(); | ||
|
|
||
| return files; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling and type safety for the API call.
The listFiles function should handle potential API errors and provide proper typing for the response.
Consider this improvement:
export async function listFiles() {
"use cache";
cacheTag(CACHE_TAGS.LIST_FILES);
- const { files } = await utapi.listFiles();
+ try {
+ const { files } = await utapi.listFiles();
+ return files;
+ } catch (error) {
+ console.error("Failed to fetch files:", error);
+ throw new Error("Failed to fetch files");
+ }
- return files;
}📝 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 async function listFiles() { | |
| "use cache"; | |
| cacheTag(CACHE_TAGS.LIST_FILES); | |
| const { files } = await utapi.listFiles(); | |
| return files; | |
| } | |
| export async function listFiles() { | |
| "use cache"; | |
| cacheTag(CACHE_TAGS.LIST_FILES); | |
| try { | |
| const { files } = await utapi.listFiles(); | |
| return files; | |
| } catch (error) { | |
| console.error("Failed to fetch files:", error); | |
| throw new Error("Failed to fetch files"); | |
| } | |
| } |
| export async function getSession(): Promise<Session | null> { | ||
| const cook = (await cookies()).get(SESSION_COOKIE_NAME); | ||
| if (!cook?.value) return null; | ||
| return JSON.parse(cook.value) as Session; | ||
| } |
There was a problem hiding this comment.
Enhance session parsing with validation.
The current implementation directly parses JSON without validation, which could lead to runtime errors or security vulnerabilities.
Consider adding validation:
export async function getSession(): Promise<Session | null> {
const cook = (await cookies()).get(SESSION_COOKIE_NAME);
if (!cook?.value) return null;
- return JSON.parse(cook.value) as Session;
+ try {
+ const parsed = JSON.parse(cook.value);
+ if (
+ typeof parsed === "object" &&
+ typeof parsed.sub === "number" &&
+ typeof parsed.iat === "number" &&
+ typeof parsed.role === "string"
+ ) {
+ return parsed as Session;
+ }
+ return null;
+ } catch {
+ return 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 async function getSession(): Promise<Session | null> { | |
| const cook = (await cookies()).get(SESSION_COOKIE_NAME); | |
| if (!cook?.value) return null; | |
| return JSON.parse(cook.value) as Session; | |
| } | |
| export async function getSession(): Promise<Session | null> { | |
| const cook = (await cookies()).get(SESSION_COOKIE_NAME); | |
| if (!cook?.value) return null; | |
| try { | |
| const parsed = JSON.parse(cook.value); | |
| if ( | |
| typeof parsed === "object" && | |
| typeof parsed.sub === "number" && | |
| typeof parsed.iat === "number" && | |
| typeof parsed.role === "string" | |
| ) { | |
| return parsed as Session; | |
| } | |
| return null; | |
| } catch { | |
| return null; | |
| } | |
| } |
| <Button | ||
| onClick={() => { | ||
| startOpenTransition(async () => { | ||
| const response = await getFileUrl(file.key); | ||
| if (response.success) { | ||
| window.open(response.url, "_blank"); | ||
| } else { | ||
| window.alert(response.error); | ||
| } | ||
| }); | ||
| }} | ||
| color={failed ? "lightgray" : "blue"} | ||
| className={cx( | ||
| "min-w-16", | ||
| isOpening && "animate-pulse", | ||
| failed && "font-bold text-red-600", | ||
| )} | ||
| disabled={file.status !== "Uploaded" || isOpening} | ||
| > | ||
| {isOpening ? "⏳" : file.status === "Uploaded" ? "Open" : file.status} | ||
| </Button> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve accessibility for file actions
The buttons lack proper ARIA labels and keyboard focus indicators.
<Button
onClick={() => {
startOpenTransition(async () => {
const response = await getFileUrl(file.key);
if (response.success) {
window.open(response.url, "_blank");
} else {
window.alert(response.error);
}
});
}}
color={failed ? "lightgray" : "blue"}
className={cx(
"min-w-16",
isOpening && "animate-pulse",
failed && "font-bold text-red-600",
)}
+ aria-label={`Open file ${file.name}`}
+ aria-busy={isOpening}
disabled={file.status !== "Uploaded" || isOpening}
>Committable suggestion skipped: line range outside the PR's diff.
| const Header = () => ( | ||
| <div className="flex gap-2 text-sm/6"> | ||
| <span className="font-medium">Hello 👋</span> | ||
| <pre>Session: {JSON.stringify(session ?? "null")}</pre> |
There was a problem hiding this comment.
Avoid exposing raw session data.
Displaying the entire session object could potentially expose sensitive information.
-<pre>Session: {JSON.stringify(session ?? "null")}</pre>
+<pre>Status: {session ? "Authenticated" : "Not authenticated"}</pre>📝 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.
| <pre>Session: {JSON.stringify(session ?? "null")}</pre> | |
| <pre>Status: {session ? "Authenticated" : "Not authenticated"}</pre> |
| async function UTSSR() { | ||
| await connection(); | ||
|
|
||
| return <NextSSRPlugin routerConfig={extractRouterConfig(uploadRouter)} />; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for connection failures.
The connection attempt should be wrapped in try-catch to gracefully handle potential failures.
async function UTSSR() {
- await connection();
+ try {
+ await connection();
+ } catch (error) {
+ console.error("Failed to establish connection:", error);
+ // Consider showing a user-friendly error message
+ return null;
+ }
return <NextSSRPlugin routerConfig={extractRouterConfig(uploadRouter)} />;
}📝 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.
| async function UTSSR() { | |
| await connection(); | |
| return <NextSSRPlugin routerConfig={extractRouterConfig(uploadRouter)} />; | |
| } | |
| async function UTSSR() { | |
| try { | |
| await connection(); | |
| } catch (error) { | |
| console.error("Failed to establish connection:", error); | |
| // Consider showing a user-friendly error message | |
| return null; | |
| } | |
| return <NextSSRPlugin routerConfig={extractRouterConfig(uploadRouter)} />; | |
| } |
| export default function RootLayout({ | ||
| children, | ||
| }: { | ||
| children: React.ReactNode; | ||
| }) { | ||
| return ( | ||
| <html lang="en"> | ||
| <head> | ||
| <script src="https://cdn.tailwindcss.com"></script> | ||
| </head> | ||
| <body> | ||
| <Suspense> | ||
| <UTSSR /> | ||
| </Suspense> | ||
| <Nav /> | ||
| {children} | ||
| </body> | ||
| </html> | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add essential metadata configuration.
The layout is missing important metadata configuration which is essential for SEO and responsive design.
Add the following metadata configuration:
+import { Metadata } from "next"
+
+export const metadata: Metadata = {
+ title: "UploadThing Playground",
+ description: "A playground for testing UploadThing functionality",
+}
export default function RootLayout({
children,
}: {
children: React.ReactNode;
}) {
return (
<html lang="en">
<head>
+ <meta name="viewport" content="width=device-width, initial-scale=1" />
<script src="https://cdn.tailwindcss.com"></script>
</head>📝 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 default function RootLayout({ | |
| children, | |
| }: { | |
| children: React.ReactNode; | |
| }) { | |
| return ( | |
| <html lang="en"> | |
| <head> | |
| <script src="https://cdn.tailwindcss.com"></script> | |
| </head> | |
| <body> | |
| <Suspense> | |
| <UTSSR /> | |
| </Suspense> | |
| <Nav /> | |
| {children} | |
| </body> | |
| </html> | |
| ); | |
| } | |
| import { Metadata } from "next" | |
| export const metadata: Metadata = { | |
| title: "UploadThing Playground", | |
| description: "A playground for testing UploadThing functionality", | |
| } | |
| export default function RootLayout({ | |
| children, | |
| }: { | |
| children: React.ReactNode; | |
| }) { | |
| return ( | |
| <html lang="en"> | |
| <head> | |
| <meta name="viewport" content="width=device-width, initial-scale=1" /> | |
| <script src="https://cdn.tailwindcss.com"></script> | |
| </head> | |
| <body> | |
| <Suspense> | |
| <UTSSR /> | |
| </Suspense> | |
| <Nav /> | |
| {children} | |
| </body> | |
| </html> | |
| ); | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
playground/README.md (2)
1-7: Add missing period in the warning message.The warning message is clear and well-formatted, but for consistency, add a period at the end of the sentence.
- app does not adhere to best practices and should not be deployed to production + app does not adhere to best practices and should not be deployed to production.🧰 Tools
🪛 LanguageTool
[uncategorized] ~6-~6: A period might be missing here.
Context: ...practices and should not be deployed to production ## Getting started 1. Copy [`.env.exa...(AI_EN_LECTOR_MISSING_PUNCTUATION_PERIOD)
8-14: Enhance the Getting Started section with more details.The steps could be more informative to help users get started quickly.
## Getting started -1. Copy [`.env.example`](./.env.example) to `.env` and fill in the values +1. Copy [`.env.example`](./.env.example) to `.env` and fill in the required values: + - `UPLOADTHING_TOKEN`: Your UploadThing API token + - `UPLOADTHING_LOG_LEVEL`: Set to "Debug" for development -2. Run app +2. Run the app: + ```bash + pnpm dev + ``` -3. Have fun +3. Have fun exploring the playground!🧰 Tools
🪛 LanguageTool
[uncategorized] ~10-~10: A punctuation mark might be missing here.
Context: ...production ## Getting started 1. Copy.env.exampleto `.env...(AI_EN_LECTOR_MISSING_PUNCTUATION)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
playground-v6/README.md(1 hunks)playground-v6/lib/actions.ts(1 hunks)playground/.env.example(1 hunks)playground/README.md(1 hunks)playground/lib/actions.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- playground-v6/lib/actions.ts
- playground/.env.example
- playground/lib/actions.ts
🧰 Additional context used
🪛 LanguageTool
playground-v6/README.md
[uncategorized] ~6-~6: A period might be missing here.
Context: ...practices and should not be deployed to production ## Getting started 1. Copy [`.env.exa...
(AI_EN_LECTOR_MISSING_PUNCTUATION_PERIOD)
[uncategorized] ~10-~10: A punctuation mark might be missing here.
Context: ...production ## Getting started 1. Copy .env.example to `.env...
(AI_EN_LECTOR_MISSING_PUNCTUATION)
playground/README.md
[uncategorized] ~6-~6: A period might be missing here.
Context: ...practices and should not be deployed to production ## Getting started 1. Copy [`.env.exa...
(AI_EN_LECTOR_MISSING_PUNCTUATION_PERIOD)
[uncategorized] ~10-~10: A punctuation mark might be missing here.
Context: ...production ## Getting started 1. Copy .env.example to `.env...
(AI_EN_LECTOR_MISSING_PUNCTUATION)
add a playground (one for v6, one for v7) with some mixing of utapi and upload button useful for some internal testing since it interacts with both ingeest and api
Summary by CodeRabbit
Release Notes
New Features
Uploadercomponent for handling file uploads.FileCardcomponent for displaying file details with open and delete options.Skeletoncomponent for loading states.Buttoncomponent for customizable button styles.Documentation
Configuration
playgrounddirectory in the workspace management.