Turbo #170
Turbo #170
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
|
|
|
|
|
Root layout was always required, e.g. for the 404 page which you were requesting there. With Turbopack we can't auto-generate a root layout like it does when running webpack as mentioned in the documentation: https://nextjs.org/docs/architecture/turbopack#unsupported-features |
This error looks correct, looking at the |
|
|
This seems to be caused by importing Edit: Did some further digging, it's using |
|
Tried to reproduce it by running import { bundleMDX } from "mdx-bundler";
const mdxSource = `
---
title: Example Post
published: 2021-02-13
description: This is some description
---
# Wahoo
import Demo from './demo'
Here's a **neat** demo:
<Demo />
`.trim();
export default async function MdxBundler() {
const result = await bundleMDX({
source: mdxSource,
files: {
"./demo.tsx": `
import * as React from 'react'
function Demo() {
return <div>Neat demo!</div>
}
export default Demo
`,
},
});
const { code, frontmatter } = result;
return (
<div>
<h1>My MDX Blog</h1>
<pre>{JSON.stringify(frontmatter, null, " ")}</pre>
<pre>{code}</pre>
</div>
);
}Only way I can reproduce the error is adding
|
|
Hey @timneutkens, thanks for taking a look!
|
|
Wrt 2. I'll take a look at reproducing the alias not applying, might be related to the |
Co-Authored-By: Tim Neutkens <[email protected]>
📝 WalkthroughWalkthroughThe changes introduced in this pull request include the addition of a new React component named Changes
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
Documentation and Community
|
Co-Authored-By: Tim Neutkens <[email protected]>
Co-Authored-By: Tim Neutkens <[email protected]>
|
@timneutkens All seems to be working now with your recommendations. Feels a bit weird adding dependencies that I'm not directly importing or using + will make it hard to figure out which dependencies are actually needed (can't use |
haydenbleasel
added
minor
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (4)
apps/api/app/layout.tsx (1)
1-1: Consider adding Metadata types for SEO optimization.Since this is a Next.js root layout, consider importing and implementing metadata types for SEO:
-import type { ReactNode } from 'react'; +import type { ReactNode } from 'react'; +import type { Metadata } from 'next';apps/web/tsconfig.json (1)
8-8: Consider using a more specific path alias for content-collectionsGiven the reported issues with
content-collectionsexports in the PR comments, consider using a more specific alias like@content/*to better indicate its scope and prevent potential naming conflicts.- "content-collections": ["./.content-collections/generated"] + "@content/*": ["./.content-collections/generated/*"]apps/api/package.json (1)
6-6: Consider adding a fallback mechanismGiven that Turbopack is still in beta and previous discussions highlighted some issues, consider adding a fallback mechanism:
{ "scripts": { "dev": "concurrently \"pnpm:next\" \"pnpm:stripe\"", - "next": "next dev -p 3002 --turbopack", + "next": "next dev -p 3002 --turbopack", + "next:stable": "next dev -p 3002", ... } }This allows developers to fall back to the stable webpack-based development server if they encounter issues with Turbopack.
apps/web/package.json (1)
30-30: Review unrelated dependency additionsThe following dependencies seem unrelated to the Turbopack implementation:
mdx-bundler: MDX compilationsharp: Image processingshiki: Syntax highlightingWhile these might be needed for other features, consider:
- Creating separate PRs for unrelated changes to maintain clear change history
- Using exact versions (
0.33.5instead of^0.33.5) for better dependency predictability- Documenting the purpose of these additions in the PR description
Also applies to: 37-38
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (3)
packages/typescript-config/nextjs.jsonis excluded by none and included by nonepnpm-lock.yamlis excluded by!**/pnpm-lock.yamland included by nonesplash/package.jsonis excluded by none and included by none
📒 Files selected for processing (5)
apps/api/app/layout.tsx(1 hunks)apps/api/package.json(1 hunks)apps/app/package.json(3 hunks)apps/web/package.json(2 hunks)apps/web/tsconfig.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
apps/api/app/layout.tsx (1)
Pattern **/*.tsx: Follow React best practices. Ensure proper component typing and use of React.FC for functional components.
🔇 Additional comments (9)
apps/api/app/layout.tsx (3)
3-5: LGTM! Type definition follows best practices.
The type definition is well-structured with proper immutability and typing.
13-13: LGTM! Export follows Next.js conventions.
The default export is correct for Next.js app router layout files.
1-13: Verify Turbopack compatibility.
The root layout implementation satisfies Turbopack requirements. Let's verify there are no conflicting layouts in the API app directory.
✅ Verification successful
No conflicting layouts found in the API app directory
The verification confirms that apps/api/app/layout.tsx is the only layout file present in the API app directory, which is the correct implementation for Turbopack compatibility.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any conflicting layout files in the API app directory
# that might cause issues with Turbopack.
echo "Checking for conflicting layout files..."
fd --type f "layout\.(tsx|jsx|js|ts)$" apps/api/app
Length of output: 168
apps/web/tsconfig.json (1)
4-9: Verify path mappings align with project structure
The path mappings look reasonable, but let's verify the directory structure to ensure they're correctly configured.
✅ Verification successful
Path mappings are correctly configured and actively used
The verification confirms that:
- The
@repo/*mapping to../../packages/*is valid and extensively used across the codebase, with imports from all the listed package directories. - The
content-collectionsmapping is actively used in multiple files and has proper dependencies installed (@content-collections/*packages in package.json). - The path mappings align with the monorepo structure and support the Turbopack migration objective.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the existence of directories referenced in path mappings
# Check if packages directory exists and contains files
echo "Checking packages directory..."
if [ -d "packages" ]; then
fd . packages -t d -d 1
fi
# Check if .content-collections/generated exists
echo "Checking content collections directory..."
if [ -d ".content-collections/generated" ]; then
ls -la .content-collections/generated
fi
# Look for any existing imports using these path aliases
echo "Checking usage of path aliases..."
rg -t ts -t tsx "from ['\"]@repo/" .
rg -t ts -t tsx "from ['\"]content-collections['\"]" .
Length of output: 774
Script:
#!/bin/bash
# Let's try a different approach to check imports using ripgrep without file type filters
echo "Checking @repo imports..."
rg "from ['\"]@repo/" .
echo -e "\nChecking content-collections imports..."
rg "from ['\"]content-collections['\"]" .
echo -e "\nChecking if .content-collections exists in any form..."
fd -H "content-collections" .
echo -e "\nChecking package.json for related configurations..."
fd "package.json" apps/web -x cat {}
Length of output: 20972
apps/api/package.json (1)
6-6: Verify Turbopack compatibility with workspace dependencies
While enabling Turbopack can improve development speed, there are some considerations to verify:
- Ensure all workspace dependencies (@repo/*) are compatible with Turbopack
- Confirm that a root layout exists as it's mandatory with Turbopack
- Check for any external dependency issues, particularly with @clerk/nextjs and svix
✅ Verification successful
Turbopack setup appears compatible with current configuration
The verification shows that the necessary components are in place:
- Root layout is properly implemented with the required HTML structure
- Next.js configuration is clean and uses shared workspace config
- Workspace dependencies are properly configured with workspace:* protocol
The setup appears ready for Turbopack usage, with no apparent compatibility issues in the configuration or dependencies. The shared config approach through @repo/next-config suggests consistent configuration across apps, which is ideal for Turbopack adoption.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify root layout existence and potential compatibility issues
# Check for root layout file
echo "Checking for root layout..."
fd "layout" "apps/api/app"
# Check for potential compatibility issues in Next.js config
echo "Checking Next.js configuration..."
fd "next.config" "apps/api"
# Look for any experimental flags or special handling
echo "Checking for experimental configurations..."
rg -l "experimental" "apps/api"
Length of output: 367
Script:
#!/bin/bash
# Let's examine the root layout and next.config content
# Check root layout content
echo "=== Root Layout Content ==="
cat "apps/api/app/layout.tsx"
# Check Next.js config content
echo -e "\n=== Next.js Config Content ==="
cat "apps/api/next.config.ts"
# Check for workspace dependencies
echo -e "\n=== Workspace Dependencies ==="
rg "@repo/" "apps/api/package.json"
Length of output: 1232
apps/app/package.json (2)
14-14: LGTM: Prisma client addition
Adding @prisma/client as a direct dependency is correct, as mentioned in the PR discussion about external dependency issues.
25-25: Clarify the need for module interception packages
The addition of import-in-the-middle and require-in-the-middle raises some concerns:
- These packages are typically used for module interception and instrumentation, which could have security implications if not used carefully
- Using caret ranges (
^) could lead to unexpected updates in minor versions
Consider:
- Documenting why these packages are necessary
- Using fixed versions instead of caret ranges
- Adding security measures if these are used for runtime module modification
#!/bin/bash
# Check for any security advisories for these packages
echo "Checking for security advisories..."
gh api graphql -f query='
{
securityVulnerabilities(first: 5, ecosystem: NPM, package: "import-in-the-middle") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
}
}
}
'
gh api graphql -f query='
{
securityVulnerabilities(first: 5, ecosystem: NPM, package: "require-in-the-middle") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
}
}
}
'Also applies to: 31-31
apps/web/package.json (2)
28-28: Clarify the need for module interception libraries
The addition of import-in-the-middle and require-in-the-middle appears to be related to the external dependency issues discussed in the PR. However, these are typically used for module interception and debugging, which raises some concerns:
- These packages are usually development dependencies rather than runtime dependencies
- They might impact the production bundle size
- They could introduce security risks if not properly configured
Consider:
- Moving these to
devDependenciesif they're only needed during development - Documenting why these packages are necessary
- Exploring alternative solutions for handling external dependencies
Let's check if these packages are actually used in the codebase:
#!/bin/bash
# Check usage of module interception packages
echo "Checking usage of import-in-the-middle and require-in-the-middle..."
rg "import.*['\"]import-in-the-middle['\"]|import.*['\"]require-in-the-middle['\"]" "apps/web/src"Also applies to: 36-36
5-5: Verify Turbopack compatibility with the current setup
The addition of --turbopack aligns with the goal of improving development build speeds. However, based on the PR discussion, please ensure:
- Root layouts are properly implemented across all pages
- All required dependencies are installed locally to avoid external dependency warnings
- Module exports and types are compatible with Turbopack's requirements
Let's verify the presence of root layouts and potential compatibility issues:
✅ Verification successful
Turbopack setup appears compatible with the current project structure
The verification shows:
- Root layout is properly implemented at
apps/web/app/layout.tsx - Project follows Next.js App Router structure with appropriate directory organization
- All required dependencies are present in package.json, including necessary workspace packages
The addition of --turbopack flag is safe to proceed with, as the basic requirements for Turbopack compatibility are met.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for root layouts and potential compatibility issues
# Verify root layouts exist
echo "Checking for root layouts..."
fd "layout.tsx" "apps/web"
# Check for potential compatibility issues in import statements
echo "Checking for potential compatibility issues..."
rg "from ['\"]@" "apps/web/src" --type ts --type tsx
Length of output: 552
Script:
#!/bin/bash
# Let's verify the directory structure and root layout first
echo "Checking directory structure..."
fd . apps/web -t d
echo -e "\nChecking for layout files..."
fd layout "apps/web" -e tsx -e ts -e js -e jsx
echo -e "\nChecking package.json for dependencies..."
cat apps/web/package.json
echo -e "\nChecking for imports from workspace packages..."
rg "from ['\"]@" apps/web --type typescript
Length of output: 2682
| const RootLayout = ({ children }: RootLayoutProperties) => ( | ||
| <html lang="en"> | ||
| <body>{children}</body> | ||
| </html> | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance the layout component with Next.js best practices.
The component should be enhanced with:
- React.FC type annotation (per coding guidelines)
- Metadata implementation
- Required head and viewport meta tags
-const RootLayout = ({ children }: RootLayoutProperties) => (
+export const metadata: Metadata = {
+ title: 'API',
+ description: 'API documentation'
+};
+
+const RootLayout: React.FC<RootLayoutProperties> = ({ children }) => (
<html lang="en">
+ <head>
+ <meta name="viewport" content="width=device-width, initial-scale=1" />
+ </head>
<body>{children}</body>
</html>
);Committable suggestion skipped: line range outside the PR's diff.
| @@ -2,7 +2,7 @@ | |||
| "name": "app", | |||
| "private": true, | |||
| "scripts": { | |||
| "dev": "next dev -p 3000", | |||
| "dev": "next dev -p 3000 --turbopack", | |||
There was a problem hiding this comment.
💡 Codebase verification
Turbopack is compatible but requires security updates
The Next.js version 15.0.3 fully supports Turbopack, however there are two important points to address:
- The
import-in-the-middledependency has a HIGH severity security vulnerability (versions <= 1.4.1). Please update to the latest version. - Consider explicitly enabling Turbopack in
next.config.jsinstead of using the CLI flag for better maintainability.
🔗 Analysis chain
Verify Turbopack compatibility with Next.js version
While enabling Turbopack can improve development speed, please note that it's still in beta and has some limitations. Ensure all team members are aware of:
- Potential compatibility issues with certain Next.js features
- The requirement for root layouts as mentioned in the PR discussion
Let me gather more information about the Next.js version and Turbopack configuration.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the Next.js version supports Turbopack
# and if there are any known issues
# Get the Next.js version from package.json
NEXT_VERSION=$(cat apps/app/package.json | jq -r '.dependencies.next')
# Check Next.js docs for Turbopack compatibility
echo "Checking Next.js $NEXT_VERSION for Turbopack compatibility..."
curl -s "https://nextjs.org/docs/app/api-reference/next-config-js/turbopack"
# Look for any Turbopack-related issues in the codebase
echo "Checking for potential Turbopack configuration issues..."
fd --type f --extension js,ts,tsx . apps/ --exec grep -l "use client"
Length of output: 877
Script:
#!/bin/bash
# Check Next.js version and Turbopack configuration
echo "Next.js version:"
cat apps/app/package.json | jq '.dependencies.next'
# Check for Turbopack configuration in next.config.js
echo -e "\nTurbopack configuration:"
fd next.config.js apps/ --exec cat {}
# Check for any experimental features
echo -e "\nChecking for experimental features:"
fd next.config.js apps/ --exec grep -A 5 "experimental"
Length of output: 353
|
@haydenbleasel I had that error when using the Sentry SDK with turbopack in the new Next 15. It first ask me for the import-in-the-middle when using react 19 RC but when i used the react 18 it asked me for the require-in-the-middle package. You can check the full conversation in getsentry/sentry-javascript#8105 |
This pull request includes updates to the
devscripts in variouspackage.jsonfiles to enable Turbopack for faster development builds.Changes to
devscripts:apps/_docs/package.json: Updateddevscript to include--turbopackoption.apps/api/package.json: Updateddevscript to include--turbopackoption.apps/app/package.json: Updateddevscript to include--turbopackoption.apps/web/package.json: Updateddevscript to include--turbopackoption.Summary by CodeRabbit
New Features
RootLayoutcomponent for improved layout management.Improvements
--turbopackflag for better performance.Bug Fixes