chore: Add tanstack/start example and fix docs
#981
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
juraj98
changed the title
tanstack/start example and fix docstanstack/start example and fix docs
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe pull request introduces several enhancements to the Changes
Possibly related PRs
Suggested labels
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 (
|
|
@coderabbitai review |
Actions performedReview triggered.
|
juraj98
changed the title
tanstack/start example and fix docstanstack/start example and fix docs
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Outside diff range and nitpick comments (15)
examples/minimal-tanstack-start/.env.example (1)
1-2: LGTM! Consider adding a note about copying the file.The
.env.examplefile is well-structured and provides clear instructions for obtaining the UPLOADTHING_TOKEN. This is a good practice for maintaining security by not committing actual tokens to the repository.Consider adding a comment suggesting users copy this file to
.envand replace the placeholder with their actual token. For example:# Go to https://uploadthing.com/dashboard to get your token +# Copy this file to .env and replace the placeholder with your actual token UPLOADTHING_TOKEN='...'This additional instruction can help users, especially those new to environment variable configuration, set up their project correctly.
examples/minimal-tanstack-start/app.config.ts (1)
1-3: Consider adding documentation and expanding the configuration.While the current setup is minimal and functional, it could be improved in the following ways:
- Add comments explaining the purpose of this file and how to extend the configuration.
- Consider adding some basic configuration options to demonstrate TanStack Start's capabilities.
Example:
import { defineConfig } from "@tanstack/start/config"; // This file configures the TanStack Start application. // Add your custom configuration options here. export default defineConfig({ // Example configuration options: // server: { // port: 3000, // }, // build: { // outDir: 'dist', // }, });This would provide more context for developers new to TanStack Start and serve as a better example in the PR.
examples/minimal-tanstack-start/app/api.ts (1)
6-6: LGTM: Default API handler correctly exportedThe default export creates an API handler using the imported functions, which is the correct setup for a TanStack Start application. This establishes a basic API handler for the application.
Consider adding a brief comment explaining the purpose of this file and how it integrates with the rest of the application. This would be helpful for developers who are new to TanStack Start. For example:
// This file sets up the default API handler for our TanStack Start application. // It uses the createStartAPIHandler function with the defaultAPIFileRouteHandler // to handle API routes defined in our application. export default createStartAPIHandler(defaultAPIFileRouteHandler);examples/minimal-tanstack-start/tsconfig.json (1)
4-4: LGTM: Modern module resolution strategy.The
"moduleResolution": "Bundler"setting is appropriate for projects using modern bundlers. It provides better integration with tools like Webpack, Vite, or esbuild.Consider adding
"allowImportingTsExtensions": trueto the compiler options. This allows importing TypeScript files with.tsor.tsxextensions, which can be useful in a bundled environment.examples/minimal-tanstack-start/app/client.tsx (2)
6-6: LGTM: Router creation is correct.The router instance is created correctly using the imported
createRouterfunction. This approach is suitable for a minimal example.Consider adding a comment explaining the purpose of the router or any specific configuration it might have in the future. This can help other developers understand the router's role in the application.
8-8: Application rendering looks good, but consider handling potential null case.The use of
hydrateRootfor client-side hydration is correct, and theStartClientcomponent is properly used with the router prop. However, the non-null assertion (!) when getting the root element might lead to runtime errors if the element is not found.Consider adding a null check or using a more robust method to ensure the root element exists. Here's an example of how you could improve this:
-hydrateRoot(document.getElementById("root")!, <StartClient router={router} />); +const rootElement = document.getElementById("root"); +if (rootElement) { + hydrateRoot(rootElement, <StartClient router={router} />); +} else { + console.error("Root element not found"); +}This change will prevent potential runtime errors and provide better error handling.
examples/minimal-tanstack-start/app/ssr.tsx (1)
9-12: SSR setup looks good, consider enhancements for production.The SSR handler setup is correctly implemented using
createStartHandlerwith the necessary configuration. The use ofdefaultStreamHandleris a good practice for streaming server-rendered content, which can improve performance.For a minimal example, this setup is sufficient. However, for production use, consider the following enhancements:
- Add error handling to manage potential issues during SSR.
- Implement custom configurations or middleware if needed for your specific use case.
- Consider adding performance optimizations such as caching strategies.
examples/minimal-tanstack-start/app/routes/api/uploadthing.ts (1)
8-11: LGTM: API route export is correctly implemented.The
Routeexport is properly set up usingcreateAPIFileRoutefor the '/api/uploadthing' endpoint. Both GET and POST methods correctly use the samehandlers, which is typical for file upload endpoints.Consider destructuring the
handlersobject for improved readability:export const Route = createAPIFileRoute("/api/uploadthing")({ - GET: handlers, - POST: handlers, + GET: handlers.GET, + POST: handlers.POST, });This change makes it explicit which methods are being used from the
handlersobject.examples/minimal-tanstack-start/app/router.tsx (2)
13-17: LGTM: Module augmentation is correct and enhances type safety.The module augmentation for "@tanstack/react-router" is well-implemented:
- It extends the
Registerinterface to include the custom router type.- Using
ReturnType<typeof createRouter>ensures type consistency.This augmentation will improve type checking and autocompletion when using the router in other parts of the application.
Consider adding a brief comment explaining the purpose of this module augmentation for better code documentation:
declare module "@tanstack/react-router" { + // Augment the Register interface to include our custom router type interface Register { router: ReturnType<typeof createRouter>; } }
1-17: Overall implementation looks great, with a minor suggestion for improvement.The router setup is well-structured and follows best practices:
- Clear separation of concerns with route definitions in a separate file.
- Concise
createRouterfunction for easy instantiation.- Proper type augmentation for enhanced type safety.
Consider adding basic error handling to make the router creation more robust:
export function createRouter() { - const router = createTanStackRouter({ - routeTree, - }); + try { + const router = createTanStackRouter({ + routeTree, + }); + return router; + } catch (error) { + console.error("Failed to create router:", error); + throw error; // Re-throw to allow handling by the caller if needed + } - return router; }This change would help catch and log any unexpected errors during router creation, making debugging easier if issues arise.
examples/minimal-tanstack-start/app/utils/uploadthing.ts (1)
1-12: Consider adding explanatory comments.The implementation is clean and follows best practices. To improve maintainability and ease of use for other developers, consider adding a brief comment at the top of the file explaining its purpose and how to use the exported components and hook.
Here's a suggested comment to add at the beginning of the file:
/** * This file sets up and exports the necessary components and hooks for file uploads * using the @uploadthing/react library. It includes: * - UploadButton: A button component for initiating file uploads * - UploadDropzone: A dropzone component for drag-and-drop file uploads * - useUploadThing: A custom hook for managing the upload process * * All components and hooks are type-safe and integrated with the server-side upload router. */examples/minimal-tanstack-start/app/routes/__root.tsx (1)
17-38: LGTM: Component structure is well-organized. Consider adding a lang attribute for accessibility.The
RootComponentandRootDocumentare well-structured and follow best practices for a root component in a React application. The use ofOutletallows for proper rendering of nested routes, and the placement ofScrollRestorationandScriptsis correct for optimal performance.Consider adding a
langattribute to theHtmlcomponent for better accessibility:- <Html> + <Html lang="en">This helps screen readers and other assistive technologies to properly interpret the content.
examples/minimal-tanstack-start/app/server/uploadthing.ts (2)
1-16: LGTM! Consider enhancing error logging.The imports and error formatter setup look good. The error formatter effectively logs detailed information while providing a simplified message to the client, which is a good security practice.
Consider using a structured logging format for better error tracking:
errorFormatter: (err) => { - console.log("Error uploading file", err.message); - console.log(" - Above error caused by:", err.cause); + console.error({ + message: "Error uploading file", + error: err.message, + cause: err.cause + }); return { message: err.message }; },
22-35: LGTM! Consider adding file type validation.The uploadRouter configuration looks good, with appropriate size limits and access control settings for different file types.
Consider adding file type validation to ensure only specific image and video formats are accepted. This can be done using the
acceptoption:image: { maxFileSize: "32MB", maxFileCount: 4, acl: "public-read", + accept: ["image/jpeg", "image/png", "image/gif"], }, video: { maxFileSize: "16MB", + accept: ["video/mp4", "video/webm"], },Also applies to: 60-60
docs/src/app/(docs)/getting-started/tanstack-start/page.mdx (1)
Line range hint
1-168: Summary: Documentation updates successfully address PR objectivesThe changes in this file effectively address the PR objectives:
Switching from absolute to relative imports: All import statements have been consistently updated to use relative paths, which should resolve the issues with absolute imports in
tanstack/start.Updating CSS inclusion: The documentation now correctly specifies including CSS in the root route when Tailwind is not being used.
These modifications improve the getting started guide for using UploadThing with Tanstack/Start. However, there are two points that require further clarification:
- The reason for the TypeScript error suppression in the CSS import.
- The implications of using a URL query parameter for the CSS import.
Once these points are addressed, the documentation will provide a more robust and clear guide for users.
To further improve the documentation:
- Consider adding a troubleshooting section that addresses common issues, such as the
Uncaught TypeError: $RefreshSig$ is not a functionerror mentioned in the PR summary.- Provide more context on why relative imports are necessary for Tanstack/Start, possibly linking to relevant Tanstack documentation.
- If possible, include a complete working example of a Tanstack/Start project with UploadThing integration, which could help users quickly get up and running.
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)
- README.md (1 hunks)
- docs/src/app/(docs)/getting-started/tanstack-start/page.mdx (4 hunks)
- examples/minimal-tanstack-start/.env.example (1 hunks)
- examples/minimal-tanstack-start/app.config.ts (1 hunks)
- examples/minimal-tanstack-start/app/api.ts (1 hunks)
- examples/minimal-tanstack-start/app/client.tsx (1 hunks)
- examples/minimal-tanstack-start/app/routeTree.gen.ts (1 hunks)
- examples/minimal-tanstack-start/app/router.tsx (1 hunks)
- examples/minimal-tanstack-start/app/routes/__root.tsx (1 hunks)
- examples/minimal-tanstack-start/app/routes/api/uploadthing.ts (1 hunks)
- examples/minimal-tanstack-start/app/routes/index.tsx (1 hunks)
- examples/minimal-tanstack-start/app/server/uploadthing.ts (1 hunks)
- examples/minimal-tanstack-start/app/ssr.tsx (1 hunks)
- examples/minimal-tanstack-start/app/utils/uploadthing.ts (1 hunks)
- examples/minimal-tanstack-start/package.json (1 hunks)
- examples/minimal-tanstack-start/tsconfig.json (1 hunks)
Files skipped from review due to trivial changes (1)
- examples/minimal-tanstack-start/app/routeTree.gen.ts
Additional comments not posted (31)
examples/minimal-tanstack-start/app.config.ts (1)
1-3: LGTM! Basic configuration setup is correct.The file correctly imports
defineConfigfrom@tanstack/start/configand exports a default configuration. This provides a good starting point for a TanStack Start application.examples/minimal-tanstack-start/app/api.ts (2)
1-4: LGTM: Imports are correctly structuredThe imports from "@tanstack/start/api" are well-organized and align with the PR objective of adding a TanStack Start example. These functions are essential for setting up the API handler in a TanStack Start application.
1-6: Overall: Well-structured and minimal TanStack Start API setupThis file successfully sets up a basic API handler for the TanStack Start example, aligning with the PR objectives. The implementation is concise, follows best practices, and effectively leverages the TanStack library's functionality.
To ensure this setup is consistent with other parts of the example, let's check for any other API-related files:
This will help verify that the API setup is consistent across the example and that we haven't missed any related files in our review.
Verification successful
Additional API-related file detected:
routes/api/uploadthing.tsTo ensure consistency and completeness in the API setup, let's examine the contents of
routes/api/uploadthing.ts:[/run_scripts]
Verification Complete: API setup is consistent and follows best practices
Both
app/api.tsandapp/routes/api/uploadthing.tshave been reviewed and are consistently implemented. The API handlers are concise, adhere to best practices, and effectively utilize the TanStack library's functionality.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for other API-related files in the example # Test: Search for files with 'api' in their name fd -t f api examples/minimal-tanstack-start # Test: Search for files that import from @tanstack/start/api rg -t ts "from\s+['\"]@tanstack/start/api['\"]" examples/minimal-tanstack-startLength of output: 367
Script:
#!/bin/bash # Description: Retrieve the contents of the uploadthing.ts file for review cat examples/minimal-tanstack-start/app/routes/api/uploadthing.tsLength of output: 406
examples/minimal-tanstack-start/tsconfig.json (4)
3-3: LGTM: Appropriate JSX setting for React.The
"jsx": "react-jsx"setting is correct for a modern React project. It enables the new JSX transform, which improves bundle size and simplifies code by not requiring React import in every file using JSX.
7-7: Consider the trade-offs of skipping library checks.The
"skipLibCheck": truesetting can significantly speed up compilation, which is beneficial for large projects. However, be aware that this option skips type checking of declaration files (.d.ts), potentially missing some type errors in dependencies.To ensure this setting doesn't hide critical type issues, consider running occasional full type checks in your CI pipeline. Here's a script to check if there's a CI configuration file that might be suitable for adding this check:
#!/bin/bash # Description: Check for CI configuration files in the project. # Test: Look for common CI configuration files fd -g '.github/workflows/*.yml' fd -g '.gitlab-ci.yml' fd -g '.circleci/config.yml' fd -g 'azure-pipelines.yml'
1-9: 🛠️ Refactor suggestionConsider adding stricter type-checking options.
While the current configuration is suitable for a minimal setup, consider enhancing type safety by adding the following compiler options:
{ "compilerOptions": { "jsx": "react-jsx", "moduleResolution": "Bundler", "module": "ESNext", "target": "ES2022", - "skipLibCheck": true + "skipLibCheck": true, + "strict": true, + "esModuleInterop": true, + "forceConsistentCasingInFileNames": true } }These additional options will:
- Enable strict type-checking (
strict)- Improve compatibility with CommonJS modules (
esModuleInterop)- Ensure consistent file naming across different operating systems (
forceConsistentCasingInFileNames)To check if these stricter options might cause issues in the existing codebase, run the following script:
#!/bin/bash # Description: Check for potential strict mode issues in TypeScript files. # Test: Look for common patterns that might cause issues in strict mode rg -g '!examples' --type typescript '(any|as\s+any|\!\w+|undefined|null)'
5-6: LGTM: Modern JavaScript settings.The
"module": "ESNext"and"target": "ES2022"settings are appropriate for leveraging modern JavaScript features. This configuration allows for the latest ECMAScript module syntax and targets recent language features.Consider your target browser support. If you need to support older browsers, you might need to adjust these settings or ensure proper transpilation in your build process. Run the following script to check if there are any browser support specifications in your project:
Verification successful
LGTM: Modern JavaScript settings confirmed.
The project specifies a
"browserslist": "defaults, not ie <= 11"indocs/package.json, ensuring compatibility with modern browsers while excluding older Internet Explorer versions. The"module": "ESNext"and"target": "ES2022"settings intsconfig.jsonare appropriate based on these browser support specifications.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for browser support specifications in the project. # Test: Look for browserslist config or similar in package.json or separate config files rg -g '!examples' --type json '"browserslist":' fd -g '.browserslistrc'Length of output: 286
Script:
#!/bin/bash # Description: Check for browser support specifications in the project. # Search for "browserslist" configurations in JSON files, excluding the 'examples' directory rg --glob '!examples/**' --type json '"browserslist"' # Find any .browserslistrc files, excluding the 'examples' directory fd --exclude 'examples' --glob '.browserslistrc'Length of output: 372
Script:
#!/bin/bash # Description: Check for browser support specifications in the project. # Search for "browserslist" configurations in JSON files, excluding the 'examples' directory rg --glob '!examples/**' --type json '"browserslist"' # Find any .browserslistrc files, excluding the 'examples' directory and including hidden files fd --exclude examples --hidden --glob '.browserslistrc'Length of output: 173
examples/minimal-tanstack-start/app/client.tsx (2)
1-2: LGTM: Import statements are correct and concise.The import statements are well-organized and include all necessary dependencies for the application. The use of a local import for the router configuration promotes good code organization.
Also applies to: 4-4
1-8: Overall, the minimal example is well-structured and functional.This file successfully demonstrates a basic setup for a React application using
@tanstack/start. It correctly imports necessary dependencies, creates a router instance, and renders the application using client-side hydration. The code is concise and achieves its purpose as a minimal example.While there are no major issues, consider implementing the suggested improvements:
- Add comments to explain the router's purpose and potential future configurations.
- Implement a null check when getting the root element to enhance error handling.
These changes would further improve the code quality and make it more robust for educational purposes.
examples/minimal-tanstack-start/app/ssr.tsx (2)
1-7: Imports look good and align with PR objectives.The imports are correctly structured and include the necessary functions for SSR setup. The use of a relative import for the router (
./router) aligns with the PR objective of switching from absolute to relative imports, which addresses the issue withtanstack/startnot includingvite-tsconfig-pathsby default.
1-12: Overall, the SSR setup looks correct, but further investigation is needed for the reported error.This file successfully sets up a basic SSR handler for a TanStack application and aligns with the PR objectives. The implementation follows the recommended structure and uses the correct imports.
However, the reported error
Uncaught TypeError: $RefreshSig$ is not a functionis not directly related to the code in this file. This error typically occurs during development and is often associated with hot module replacement or fast refresh functionality.To troubleshoot this issue:
- Verify that all dependencies are correctly installed and up-to-date.
- Check the build configuration (e.g., webpack or vite setup) for any conflicts or misconfigurations.
- Investigate how the application is initiated within the UploadThing repository, as the error only occurs in that context.
- Consider adding a minimal reproduction of the error in a separate branch or repository to isolate the issue.
To help identify potential causes, let's check for any custom configurations or development dependencies that might interfere with the normal operation of
tanstack/start:This script will help identify any custom configurations or dependencies that might be causing conflicts with
tanstack/start.examples/minimal-tanstack-start/app/routes/api/uploadthing.ts (3)
1-5: LGTM: Imports are correctly structured and align with PR objectives.The imports are well-organized and include all necessary functions for the file's functionality. The use of a relative import for
uploadRouteraligns with the PR objective of switching from absolute to relative imports, which addresses issues withtanstack/startnot includingvite-tsconfig-pathsby default.
7-7: LGTM: Route handler creation is correct.The route handlers are properly created using the
createRouteHandlerfunction with the importeduploadRouter. This setup ensures that the upload functionality is correctly integrated into the API route.
1-11: Overall implementation looks good, but verification needed.The file successfully sets up an API route for handling file uploads, integrating well with both
tanstack/startanduploadthinglibraries. The implementation aligns with the PR objectives, particularly in using relative imports.However, we need to verify if this implementation resolves the reported error:
Uncaught TypeError: $RefreshSig$ is not a function. Please run the following script to check for any occurrences of$RefreshSig$in the codebase:If the script returns any results, it might indicate that the error is still present in some parts of the codebase. In that case, further investigation would be needed to resolve the issue.
examples/minimal-tanstack-start/app/router.tsx (2)
1-3: LGTM: Imports are correctly set up.The imports are well-organized and follow good practices:
- Renaming
createRoutertocreateTanStackRouteravoids naming conflicts.- Importing
routeTreefrom a separate file promotes modularity and maintainability.
5-11: LGTM:createRouterfunction is well-implemented.The
createRouterfunction is concise and correctly sets up the TanStack router with the importedrouteTree. The implementation is clean and easy to understand.examples/minimal-tanstack-start/app/utils/uploadthing.ts (3)
1-7: LGTM: Imports are well-structured and appropriate.The import statements are clean and follow best practices. They import the necessary functions from "@uploadthing/react" and the custom type from the local server module, which is good for maintaining type safety across the client-server boundary.
9-10: LGTM: UploadButton and UploadDropzone are correctly generated and exported.The UploadButton and UploadDropzone components are properly generated using the functions from @uploadthing/react, and they're correctly parameterized with the OurFileRouter type. This ensures type safety and consistency with your server-side router configuration.
12-12: LGTM: useUploadThing hook is correctly generated and exported.The useUploadThing hook is properly generated using the generateReactHelpers function and correctly parameterized with the OurFileRouter type. The use of destructuring in the export statement is a clean way to expose only the necessary part of the generated helpers.
examples/minimal-tanstack-start/package.json (2)
1-4: LGTM: Project metadata is well-configured.The project metadata is correctly set up:
- The scoped package name is appropriate for an example project.
- "private": true prevents accidental publication.
- "type": "module" correctly specifies the use of ECMAScript modules.
5-9: LGTM: Scripts are appropriately defined.The scripts section is well-configured:
- It includes standard scripts for development, building, and starting the application.
- The use of "vinxi" for all scripts is consistent with the project's dependencies.
examples/minimal-tanstack-start/app/routes/__root.tsx (1)
12-15: LGTM: Root route definition is correct and includes necessary styles.The root route is properly defined using
createRootRoute, specifying theRootComponentand including the UploadThing CSS. This ensures that the necessary styles are available throughout the application.examples/minimal-tanstack-start/app/server/uploadthing.ts (1)
62-62: LGTM! Good type export practice.Exporting the
OurFileRoutertype is a good practice. It allows other parts of the application to use the correct type for the upload router, maintaining type safety across the application.examples/minimal-tanstack-start/app/routes/index.tsx (3)
1-7: LGTM: Imports are correctly structured and necessary.The imports from "@tanstack/react-router" and "../utils/uploadthing" are appropriate for the functionality of this component.
9-11: LGTM: Route definition is correct.The route is properly defined using
createFileRoutefrom TanStack Router, setting the path to "/" and using theHomecomponent.
1-79: Overall, the implementation is correct but could benefit from enhanced error handling and user feedback.The file successfully implements a file upload interface using TanStack Router and UploadThing. The structure and logic are sound, but consider implementing the suggested improvements throughout the component:
- Enhance error handling in all upload-related functions.
- Replace
alertandconsole.logstatements with more appropriate user feedback mechanisms.- Implement loading states to improve user experience during uploads.
- Add the suggested pre-upload processing for the file input.
These changes will significantly improve the robustness and user-friendliness of the component.
README.md (1)
26-27: Approve the new example addition with suggestionsThe addition of the Tanstack/Start example to the list is good and consistent with the existing format. However, there are a few points to address:
The PR objectives mention issues with the Tanstack/Start example when started from within the repository. Consider adding a note about this known issue in the description, such as:
A simple example using Tanstack/Start (Note: Currently experiencing issues when started within the repository)The PR objectives also mention changes to documentation regarding imports and CSS. These changes are not reflected in the README.md. Could you clarify if these changes should be documented here or in a separate file?
To ensure the example directory exists and contains the necessary files, please run the following script:
docs/src/app/(docs)/getting-started/tanstack-start/page.mdx (4)
99-99: Approved: Import path updated to use relative importThis change from an absolute import to a relative import aligns with the PR objectives. It addresses the issue with absolute imports in
tanstack/start, which doesn't includevite-tsconfig-pathsby default. This modification should resolve the import problems mentioned in the PR summary.
123-123: Approved: Consistent update to use relative importThis change is consistent with the previous import modification, switching from an absolute import to a relative import for
OurFileRouter. It further addresses the issue with absolute imports intanstack/startand maintains consistency throughout the documentation.
148-157: Approved with comments: CSS inclusion updated, but clarification neededThe changes to include CSS in the root route instead of the root layout are appropriate and align with the PR objectives. However, there are two points that require attention:
The addition of a TypeScript error suppression comment (
// @ts-expect-error) suggests a potential type mismatch. Could you please clarify why this is necessary and if there's a way to resolve the type issue without suppressing the error?The use of a URL query parameter for the CSS import (
from "@uploadthing/react/styles.css?url") is an interesting approach to ensure the latest version is loaded. However, this might have implications for caching and performance. Could you explain the reasoning behind this choice and confirm if this is the recommended approach for all users?
168-168: Approved: Consistent update to use relative importThis change is in line with the previous import modifications, switching from an absolute import to a relative import for
UploadButton. It maintains consistency throughout the documentation and addresses the issue with absolute imports intanstack/start.
| "devDependencies": { | ||
| "@types/react": "18.3.3", | ||
| "@types/react-dom": "18.3.0", | ||
| "typescript": "^5.5.2" | ||
| } |
There was a problem hiding this comment.
Align devDependencies versions with dependencies.
The devDependencies section includes appropriate type definitions, but there are version mismatches:
-
React type definitions: The versions for "@types/react" (18.3.3) and "@types/react-dom" (18.3.0) do not match the React versions in the dependencies (18.3.1).
-
Version consistency: Consider using caret ranges for all devDependencies to allow for minor updates and bug fixes.
Consider applying these changes:
"devDependencies": {
- "@types/react": "18.3.3",
- "@types/react-dom": "18.3.0",
+ "@types/react": "^18.2.0",
+ "@types/react-dom": "^18.2.0",
"typescript": "^5.5.2"
}These changes will align the type definition versions with the React versions suggested in the previous comment and allow for minor updates across all devDependencies.
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.
| "devDependencies": { | |
| "@types/react": "18.3.3", | |
| "@types/react-dom": "18.3.0", | |
| "typescript": "^5.5.2" | |
| } | |
| "devDependencies": { | |
| "@types/react": "^18.2.0", | |
| "@types/react-dom": "^18.2.0", | |
| "typescript": "^5.5.2" | |
| } |
| "dependencies": { | ||
| "@uploadthing/react": "7.0.2", | ||
| "@tanstack/react-router": "^1.58.7", | ||
| "@tanstack/start": "^1.58.7", | ||
| "@vitejs/plugin-react": "^4.3.1", | ||
| "react": "18.3.1", | ||
| "react-dom": "18.3.1", | ||
| "vinxi": "0.3.11", | ||
| "uploadthing": "7.0.2" | ||
| }, |
There was a problem hiding this comment.
Review and update dependency versions.
The dependencies section includes appropriate packages for the project, but there are some inconsistencies and potential issues:
-
React version: The specified version "18.3.1" for both "react" and "react-dom" is not a released version. The latest stable version is 18.2.0.
-
Version consistency: Some packages use exact versions, while others use caret ranges. Consider using consistent versioning across all dependencies.
-
Vinxi version: The exact version "0.3.11" is used. For a build tool, it might be beneficial to use a caret range to allow for minor updates and bug fixes.
Consider applying these changes:
"dependencies": {
"@uploadthing/react": "^7.0.2",
"@tanstack/react-router": "^1.58.7",
"@tanstack/start": "^1.58.7",
"@vitejs/plugin-react": "^4.3.1",
- "react": "18.3.1",
- "react-dom": "18.3.1",
+ "react": "^18.2.0",
+ "react-dom": "^18.2.0",
- "vinxi": "0.3.11",
+ "vinxi": "^0.3.11",
- "uploadthing": "7.0.2"
+ "uploadthing": "^7.0.2"
}These changes will ensure you're using the latest stable version of React and allow for minor updates and bug fixes across all dependencies.
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.
| "dependencies": { | |
| "@uploadthing/react": "7.0.2", | |
| "@tanstack/react-router": "^1.58.7", | |
| "@tanstack/start": "^1.58.7", | |
| "@vitejs/plugin-react": "^4.3.1", | |
| "react": "18.3.1", | |
| "react-dom": "18.3.1", | |
| "vinxi": "0.3.11", | |
| "uploadthing": "7.0.2" | |
| }, | |
| "dependencies": { | |
| "@uploadthing/react": "^7.0.2", | |
| "@tanstack/react-router": "^1.58.7", | |
| "@tanstack/start": "^1.58.7", | |
| "@vitejs/plugin-react": "^4.3.1", | |
| "react": "^18.2.0", | |
| "react-dom": "^18.2.0", | |
| "vinxi": "^0.3.11", | |
| "uploadthing": "^7.0.2" | |
| }, |
| // @ts-expect-error | ||
| import uploadthingCss from "@uploadthing/react/styles.css?url"; |
There was a problem hiding this comment.
Consider addressing the TypeScript error for the uploadthingCss import.
The @ts-expect-error comment suggests there's a type issue with the uploadthingCss import. This might be due to missing type definitions for the CSS module.
Consider the following alternatives:
- Create a declaration file (e.g.,
custom.d.ts) to declare the module:declare module '@uploadthing/react/styles.css?url' { const content: string; export default content; }
- Use a more specific comment:
// @ts-ignore: CSS module import - Investigate why the types are missing and consider updating
@uploadthing/reactif it's outdated.
| .onUploadComplete(({ file, metadata }) => { | ||
| metadata; | ||
| // ^? | ||
| file.customId; | ||
| // ^? | ||
| console.log("upload completed", file); | ||
| }), |
There was a problem hiding this comment.
Remove development assertions and enhance upload completion handling.
The onUploadComplete callback provides basic logging functionality. However, there are a few points to address:
- Remove the TypeScript assertions (^?) as they are likely leftover from development.
- Consider enhancing the upload completion handling with more detailed logging or additional actions.
Here's a suggested improvement:
.onUploadComplete(({ file, metadata }) => {
- metadata;
- // ^?
- file.customId;
- // ^?
- console.log("upload completed", file);
+ console.log("Upload completed", {
+ fileId: file.id,
+ fileName: file.name,
+ fileSize: file.size,
+ customId: file.customId,
+ metadata
+ });
+ // Perform additional actions here, e.g., update database, send notifications, etc.
}),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.
| .onUploadComplete(({ file, metadata }) => { | |
| metadata; | |
| // ^? | |
| file.customId; | |
| // ^? | |
| console.log("upload completed", file); | |
| }), | |
| .onUploadComplete(({ file, metadata }) => { | |
| console.log("Upload completed", { | |
| fileId: file.id, | |
| fileName: file.name, | |
| fileSize: file.size, | |
| customId: file.customId, | |
| metadata | |
| }); | |
| // Perform additional actions here, e.g., update database, send notifications, etc. | |
| }), |
| .middleware(({ req, files }) => { | ||
| // Check some condition based on the incoming requrest | ||
| console.log("Request", req); | ||
| //^? | ||
| // if (!req.headers.get("x-some-header")) { | ||
| // throw new Error("x-some-header is required"); | ||
| // } | ||
|
|
||
| // (Optional) Label your files with a custom identifier | ||
| const filesWithMyIds = files.map((file, idx) => ({ | ||
| ...file, | ||
| customId: `${idx}-${randomUUID()}`, | ||
| })); | ||
|
|
||
| // Return some metadata to be stored with the file | ||
| return { foo: "bar" as const, [UTFiles]: filesWithMyIds }; | ||
| }) |
There was a problem hiding this comment.
💡 Codebase verification
🛠️ Refactor suggestion
Implement the x-some-header security check.
The middleware currently does not enforce the required x-some-header security check, as the relevant code is only present in commented-out sections. To maintain security standards, please implement this header validation. If the security check is no longer necessary, consider removing the commented-out code to enhance code clarity.
Analysis chain
Consider implementing the commented-out security check.
The middleware implementation looks good, especially the addition of custom identifiers for file tracking. However, there's a commented-out section that suggests a potential security check.
If the header check is necessary for security, consider implementing it. If not, remove the commented code to improve readability.
Consider moving the file labeling logic to a separate function for better readability:
const labelFilesWithCustomId = (files: UTFiles) =>
files.map((file, idx) => ({
...file,
customId: `${idx}-${randomUUID()}`,
}));
// In the middleware:
const filesWithMyIds = labelFilesWithCustomId(files);Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the x-some-header check is implemented elsewhere
rg --type typescript 'x-some-header'
Length of output: 71
Script:
#!/bin/bash
# Check if the x-some-header check is implemented elsewhere
rg --type ts 'x-some-header' || rg --type tsx 'x-some-header' || rg --glob '*.ts' 'x-some-header' || rg --glob '*.tsx' 'x-some-header'
Length of output: 2170
| const { startUpload } = useUploadThing("videoAndImage", { | ||
| /** | ||
| * @see https://docs.uploadthing.com/api-reference/react#useuploadthing | ||
| */ | ||
| onBeforeUploadBegin: (files) => { | ||
| console.log("Uploading", files.length, "files"); | ||
| return files; | ||
| }, | ||
| onUploadBegin: (name) => { | ||
| console.log("Beginning upload of", name); | ||
| }, | ||
| onClientUploadComplete: (res) => { | ||
| console.log("Upload Completed.", res.length, "files uploaded"); | ||
| }, | ||
| onUploadProgress(p) { | ||
| console.log("onUploadProgress", p); | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider improving error handling and removing console.log statements.
The useUploadThing hook is correctly used, but consider the following improvements:
- Add error handling to the
onClientUploadCompletecallback. - Remove or conditionally render
console.logstatements for production builds.
Here's a suggested improvement:
const { startUpload } = useUploadThing("videoAndImage", {
onBeforeUploadBegin: (files) => {
// Consider using a logger that can be disabled in production
console.log("Uploading", files.length, "files");
return files;
},
onUploadBegin: (name) => {
console.log("Beginning upload of", name);
},
onClientUploadComplete: (res) => {
console.log("Upload Completed.", res.length, "files uploaded");
// Consider adding success feedback to the user
},
onUploadError: (error) => {
console.error("Upload failed:", error);
// Consider adding error feedback to the user
},
onUploadProgress(p) {
console.log("onUploadProgress", p);
// Consider updating a progress indicator for the user
},
});| <UploadButton | ||
| /** | ||
| * @see https://docs.uploadthing.com/api-reference/react#uploadbutton | ||
| */ | ||
| endpoint="videoAndImage" | ||
| onClientUploadComplete={(res) => { | ||
| console.log(`onClientUploadComplete`, res); | ||
| alert("Upload Completed"); | ||
| }} | ||
| onUploadBegin={() => { | ||
| console.log("upload begin"); | ||
| }} | ||
| config={{ appendOnPaste: true, mode: "manual" }} | ||
| /> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance UploadButton with better user feedback and error handling.
The UploadButton is correctly implemented, but consider the following improvements:
- Add error handling with the
onUploadErrorprop. - Replace
alertwith a more user-friendly notification system. - Consider adding a loading state while the upload is in progress.
Here's a suggested improvement:
<UploadButton
endpoint="videoAndImage"
onClientUploadComplete={(res) => {
console.log(`onClientUploadComplete`, res);
// Replace with a more user-friendly notification
// notifyUser("Upload Completed");
}}
onUploadBegin={() => {
console.log("upload begin");
// Consider updating UI to show upload has started
}}
onUploadError={(error) => {
console.error("Upload failed:", error);
// notifyUser("Upload failed. Please try again.");
}}
config={{ appendOnPaste: true, mode: "manual" }}
/>| <UploadDropzone | ||
| /** | ||
| * @see https://docs.uploadthing.com/api-reference/react#uploaddropzone | ||
| */ | ||
| endpoint="videoAndImage" | ||
| onUploadAborted={() => { | ||
| alert("Upload Aborted"); | ||
| }} | ||
| onClientUploadComplete={(res) => { | ||
| console.log(`onClientUploadComplete`, res); | ||
| alert("Upload Completed"); | ||
| }} | ||
| onUploadBegin={() => { | ||
| console.log("upload begin"); | ||
| }} | ||
| /> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance UploadDropzone with better user feedback and error handling.
The UploadDropzone is correctly implemented, but consider the following improvements:
- Add error handling with the
onUploadErrorprop. - Replace
alertwith a more user-friendly notification system. - Consider adding a loading state while the upload is in progress.
Here's a suggested improvement:
<UploadDropzone
endpoint="videoAndImage"
onUploadAborted={() => {
// Replace with a more user-friendly notification
// notifyUser("Upload Aborted");
}}
onClientUploadComplete={(res) => {
console.log(`onClientUploadComplete`, res);
// Replace with a more user-friendly notification
// notifyUser("Upload Completed");
}}
onUploadBegin={() => {
console.log("upload begin");
// Consider updating UI to show upload has started
}}
onUploadError={(error) => {
console.error("Upload failed:", error);
// notifyUser("Upload failed. Please try again.");
}}
/>| <input | ||
| type="file" | ||
| multiple | ||
| onChange={async (e) => { | ||
| const files = Array.from(e.target.files ?? []); | ||
|
|
||
| // Do something with files | ||
|
|
||
| // Then start the upload | ||
| await startUpload(files); | ||
| }} | ||
| /> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance file input with error handling and consider pre-upload processing.
The file input is correctly implemented, but consider the following improvements:
- Add error handling for the
startUploadfunction. - Implement the suggested pre-upload processing.
- Provide user feedback during the upload process.
Here's a suggested improvement:
<input
type="file"
multiple
onChange={async (e) => {
const files = Array.from(e.target.files ?? []);
// Implement pre-upload processing here
// const processedFiles = await processFiles(files);
try {
// Update UI to show upload has started
// setIsUploading(true);
// Then start the upload
const result = await startUpload(files);
console.log("Upload completed:", result);
// notifyUser("Upload Completed");
} catch (error) {
console.error("Upload failed:", error);
// notifyUser("Upload failed. Please try again.");
} finally {
// Update UI to show upload has finished
// setIsUploading(false);
}
}}
/>
|
Closing this PR. The scope to make this work keeps growing. I'll create separate PRs for the doc changes, update for solidstart example, and finally the tanstack/start example. |
This PR is mainly to add example for
tanstack/start. The example works if it's started outside of the UT repo, but throwsUncaught TypeError: $RefreshSig$ is not a functionif started from the repo. No idea why that is. I'm opening this draft pull request in case someone already encountered this issue and knows a quick fix. Otherwise, I'll revisit this in a few days.There are also additional changes to docs for getting started in
tanstack/start:tanstack/startdoesn't includevite-tsconfig-pathsby default, so absolute imports do not work, even ifpathsare configured intsconfig.json. More info here: TSS Docs: Path AliasesSummary by CodeRabbit
Release Notes
New Features
Documentation
README.mdwith a new example link..env.examplefor environment variable configuration guidance.Configuration
package.jsonandtsconfig.jsonfor project setup.