-
Notifications
You must be signed in to change notification settings - Fork 308
feat: support --turbo build for Next.js #2422
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: canary
Are you sure you want to change the base?
Conversation
@seawatts is attempting to deploy a commit to the Boundary Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Caution
Changes requested ❌
Reviewed everything up to 4156ffe in 1 minute and 41 seconds. Click for details.
- Reviewed
127
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
3
draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. typescript/packages/nextjs-plugin/src/index.ts:67
- Draft comment:
Merging experimental config with object spread does not deep merge nested objects. Ensure this shallow merge meets your intended config override behavior. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
2. typescript/packages/nextjs-plugin/src/index.ts:113
- Draft comment:
The webpack rule for .node files is only added when not in Turbo mode, relying on turboConfig for Turbo builds. Verify that this separation is intentional to avoid misconfiguration. - Reason this comment was not posted:
Comment was on unchanged code.
3. typescript/packages/nextjs-plugin/src/index.ts:98
- Draft comment:
A hard‐coded list of BAML packages is appended to externals on the server side. Consider documenting or externalizing this list so future changes can be managed more easily. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_TBBnq3HRsuZjV9v6
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
} | ||
|
||
export function withBaml(bamlConfig: BamlNextConfig = {}) { | ||
export function withBaml(_bamlConfig: BamlNextConfig = {}) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The _bamlConfig parameter is currently unused. Consider removing it or using it for configurable behavior to avoid confusion.
🔒 Entelligence AI Vulnerability Scanner ✅ No security vulnerabilities found! Your code passed our comprehensive security analysis. 📊 Files Analyzed: 1 files |
Review Summary🔍 Comments beyond diff scope (1)
|
// Add WebAssembly loading configuration (properly indented) | ||
|
||
return config; | ||
return webpackConfig; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
correctness: webpackConfig
is not returned if context.isServer
is false, causing plugin webpack modifications to be lost in client builds.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In typescript/packages/nextjs-plugin/src/index.ts, line 129, ensure that the function always returns the modified `webpackConfig` object, not the original `config`, regardless of whether `context.isServer` is true or false. This prevents loss of plugin webpack modifications in client builds. Double-check that all plugin modifications are applied to `webpackConfig` and that it is always returned.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@seawatts is this a real bug?
const nextPackageJson = require(projectNextPath); | ||
return nextPackageJson.version || null; | ||
} catch (error) { | ||
} catch { | ||
try { | ||
// Fallback to checking in the plugin's dependencies | ||
const nextPackageJson = require('next/package.json'); | ||
return nextPackageJson.version || null; | ||
} catch (error) { | ||
} catch { | ||
console.warn( | ||
'Warning: Could not determine Next.js version, defaulting to latest config', | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
performance: The function getNextJsVersion
synchronously reads and requires package.json files on every invocation, which can cause measurable startup latency in large monorepos or repeated plugin invocations.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In typescript/packages/nextjs-plugin/src/index.ts, lines 3-17, the function `getNextJsVersion` synchronously reads and requires package.json files on every invocation, which can cause measurable startup latency in large monorepos or repeated plugin invocations. Add a simple in-memory cache so the version is only resolved once per process.
📝 Committable Code Suggestion
‼️ Ensure you review the code suggestion before committing it to the branch. Make sure it replaces the highlighted code, contains no missing lines, and has no issues with indentation.
const nextPackageJson = require(projectNextPath); | |
return nextPackageJson.version || null; | |
} catch (error) { | |
} catch { | |
try { | |
// Fallback to checking in the plugin's dependencies | |
const nextPackageJson = require('next/package.json'); | |
return nextPackageJson.version || null; | |
} catch (error) { | |
} catch { | |
console.warn( | |
'Warning: Could not determine Next.js version, defaulting to latest config', | |
); | |
let cachedNextJsVersion: string | null | undefined = undefined; | |
function getNextJsVersion(): string | null { | |
if (cachedNextJsVersion !== undefined) return cachedNextJsVersion; | |
try { | |
const projectNextPath = require.resolve('next/package.json', { | |
paths: [process.cwd()], | |
}); | |
const nextPackageJson = require(projectNextPath); | |
cachedNextJsVersion = nextPackageJson.version || null; | |
return cachedNextJsVersion; | |
} catch { | |
try { | |
const nextPackageJson = require('next/package.json'); | |
cachedNextJsVersion = nextPackageJson.version || null; | |
return cachedNextJsVersion; | |
} catch { | |
console.warn( | |
'Warning: Could not determine Next.js version, defaulting to latest config', | |
); | |
cachedNextJsVersion = null; | |
return null; | |
} | |
} | |
} |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Any plan on getting this in? Would love turbopack support for this already amazing product ❤️ |
Add Turbopack Support to BAML Next.js Plugin
Summary
This PR adds full support for Next.js Turbopack to the
@boundaryml/baml-nextjs-plugin
, allowing users to run their Next.js applications with the--turbo
flag without encountering native binding errors.Problem
Previously, when using BAML with Next.js Turbopack (
next dev --turbo
), users would encounter the following error:This was because the plugin didn't properly handle BAML's native bindings in Turbopack mode.
Solution
process.env.TURBOPACK === '1'
.node
files usingnextjs-node-loader
Key Changes
1. Turbopack Configuration
2. Conditional Server External Packages
3. Improved Webpack Configuration
webpackConfig
variableTesting
next dev --turbo
- server starts successfullynext dev
(regular webpack) - maintains existing functionalityBreaking Changes
None. This change is fully backward compatible.
Migration Guide
No migration required. Users can now simply run
next dev --turbo
with BAML without any additional configuration.Related Issues
Fixes issues where BAML applications couldn't use Next.js Turbopack due to native binding conflicts.
Important
Adds support for Next.js Turbopack in BAML plugin by detecting Turbopack mode, externalizing native bindings, and adjusting webpack configurations.
process.env.TURBOPACK === '1'
inindex.ts
..node
files withnextjs-node-loader
.webpackConfig
to avoid parameter mutation issues.next dev --turbo
andnext dev
.This description was created by
for 4156ffe. You can customize this summary. It will automatically update as commits are pushed.