chore: add turbo to nextjs dev server #3529
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
WalkthroughThe pull request introduces modifications to several Changes
Possibly related PRs
Suggested reviewers
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 (
|
|
Thank you for following the naming conventions for pull request titles! 🙏 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
apps/demo/package.json (1)
8-8: LGTM! Consider renaming the script for clarity.The addition of the
--turboflag to thegoscript aligns with the PR objectives and should improve development performance. This change also makes it consistent with thedevscript.Consider renaming the
goscript to something more descriptive likedev:turboordev:altfor better clarity and consistency with common naming conventions. This would make the purpose of the script more immediately apparent to other developers.apps/web/package.json (1)
7-8: Consider consolidating identical scriptsThe
devandgoscripts are now identical. To improve maintainability and reduce potential confusion, consider one of the following options:
- Consolidate the scripts by removing one and updating any references to it.
- If there's a specific reason for having both, add comments explaining their distinct purposes.
Example consolidation:
"scripts": { "dev": "next dev -p 3000 --turbo", - "go": "next dev -p 3000 --turbo", + "go": "npm run dev", ... }This change maintains both command options while reducing duplication.
packages/lib/storage/service.ts (1)
Line range hint
1-385: Consider applying consistent security measures and improving error handlingWhile the security enhancement in
getLocalFileis excellent, consider applying similar measures to other functions dealing with local file paths for consistency. For example,putFileToLocalStorageanddeleteLocalFilecould benefit from similar path resolution.Additionally, the error handling across the file, including in
getLocalFile, is minimal. Consider implementing more specific error types and handling to provide clearer feedback on what went wrong. This could involve creating custom error classes for different scenarios (e.g., FileNotFoundError, PermissionDeniedError) and catching them specifically in the calling code.Would you like assistance in implementing these suggestions across the file?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
- apps/demo/package.json (1 hunks)
- apps/web/package.json (1 hunks)
- packages/lib/storage/service.ts (1 hunks)
- packages/react-native/package.json (1 hunks)
🧰 Additional context used
🔇 Additional comments (3)
packages/react-native/package.json (1)
38-38: Approve script renaming for improved clarity.The renaming of the "go" script to "dev" enhances clarity and aligns with common development conventions. This change improves the readability and maintainability of the package.json file.
To ensure this change doesn't break existing workflows, please run the following script to check for any references to the old script name:
If this script returns any results, those locations may need to be updated to use "dev" instead of "go".
apps/web/package.json (1)
7-8: Turbo mode enabled for development serverThe
--turboflag has been added to both thedevandgoscripts. This change aims to leverage Next.js's Turbopack, which can significantly improve development performance.Considerations:
- Ensure all team members are aware of this change, as it might affect their local development workflow.
- Verify compatibility with your current Next.js version, as Turbopack is still in beta.
- Monitor for any unexpected behavior or compatibility issues with your project's specific setup.
- Consider adding a non-turbo script (e.g.,
dev:classic) for fallback if issues arise.To ensure compatibility and proper setup, please run the following verification script:
✅ Verification successful
Turbo mode properly configured for development scripts
The
--turboflag has been successfully added to both thedevandgoscripts, and the current Next.js version (14.2.15) supports Turbopack. This configuration should enhance development performance as intended.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify Next.js version and Turbopack compatibility # Check Next.js version next_version=$(npm list next --json | jq -r '.dependencies.next.version') echo "Current Next.js version: $next_version" # Check if Turbopack is mentioned in the Next.js docs turbopack_docs=$(curl -s "https://nextjs.org/docs/app/api-reference/next-cli#turbo" | grep -c "turbo") if [ $turbopack_docs -gt 0 ]; then echo "Turbopack is mentioned in the Next.js docs for the current version." else echo "Warning: Turbopack might not be fully supported in the current Next.js version." fi # Test running the dev server with --turbo echo "Testing dev server with --turbo flag..." timeout 10s npm run dev || echo "Dev server with --turbo flag started successfully."Length of output: 800
Script:
#!/bin/bash # Description: Extract Next.js version from package.json without using npm # Check if package.json exists if [ ! -f apps/web/package.json ]; then echo "Error: apps/web/package.json not found." exit 1 fi # Extract Next.js version using jq next_version=$(jq -r '.dependencies.next' apps/web/package.json) if [ "$next_version" != "null" ] && [ -n "$next_version" ]; then echo "Current Next.js version: $next_version" else echo "Next.js is not listed as a dependency in apps/web/package.json." fi # Check if Turbopack is referenced in scripts turbopack_usage=$(jq -r '.scripts | to_entries[] | select(.value | contains("--turbo")) | .key' apps/web/package.json) if [ -n "$turbopack_usage" ]; then echo "Turbopack is used in the following scripts: $turbopack_usage" else echo "Turbopack is not referenced in the scripts section of apps/web/package.json." fiLength of output: 498
packages/lib/storage/service.ts (1)
129-130: Excellent security enhancement!The use of
path.resolve(process.cwd(), filePath)is a great improvement. It ensures that the file path is resolved to an absolute path, which helps prevent potential directory traversal attacks. This change enhances the security of thegetLocalFilefunction without altering its core functionality.
This pull request includes several updates across multiple packages to improve the development workflow and enhance file path security. The most important changes include adding the
--turboflag to variousnext devcommands, ensuring safe file path resolution in thegetLocalFilefunction, and renaming a script command in thereact-nativepackage.Development Workflow Improvements:
apps/demo/package.json: Added the--turboflag to thegoscript to improve development performance.apps/web/package.json: Added the--turboflag to bothdevandgoscripts to enhance development speed.packages/react-native/package.json: Renamed thegoscript todevfor better clarity and consistency.File Path Security Enhancement:
packages/lib/storage/service.ts: Ensured safe file path resolution by usingpath.resolvein thegetLocalFilefunction.Summary by CodeRabbit
New Features
Bug Fixes
Chores
goscript todevin the React Native package for clarity.