-
Notifications
You must be signed in to change notification settings - Fork 908
chore: use nextjs canary #2923
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
chore: use nextjs canary #2923
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
| Name | Type |
|---|---|
| @react-email/components | Patch |
| @react-email/tailwind | Patch |
| playground | Patch |
Click here to learn what changesets are, and how to add one.
Click here if you're a maintainer who wants to add a changeset to this PR
commit: |
b2d9d45 to
b58fe3b
Compare
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.
No issues found across 2 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
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.
2 issues found across 1 file (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/react-email/src/commands/build.ts">
<violation number="1" location="packages/react-email/src/commands/build.ts:231">
P2: `spawnSync` failures are not checked, so a failed install still triggers the “Successfully prepared” message and proceeds to `next build`. Capture the result and throw on non‑zero exit to stop early.</violation>
<violation number="2" location="packages/react-email/src/commands/build.ts:231">
P2: The install step now hardcodes `npm install --force`, ignoring the `packageManager` argument passed into `build()`. This breaks yarn/pnpm workflows and makes the install step inconsistent with the rest of the command. Restore the package-manager-aware install helper.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
|
||
| spinner.text = 'Installing dependencies on `.react-email`'; | ||
| await installDependencies({ | ||
| spawnSync('npm install --force', { |
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.
P2: spawnSync failures are not checked, so a failed install still triggers the “Successfully prepared” message and proceeds to next build. Capture the result and throw on non‑zero exit to stop early.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/react-email/src/commands/build.ts, line 231:
<comment>`spawnSync` failures are not checked, so a failed install still triggers the “Successfully prepared” message and proceeds to `next build`. Capture the result and throw on non‑zero exit to stop early.</comment>
<file context>
@@ -227,10 +228,10 @@ export const build = async ({
spinner.text = 'Installing dependencies on `.react-email`';
- await installDependencies({
+ spawnSync('npm install --force', {
+ shell: true,
cwd: builtPreviewAppPath,
</file context>
| spawnSync('npm install --force', { | ||
| shell: true, | ||
| cwd: builtPreviewAppPath, | ||
| silent: true, | ||
| packageManager, | ||
| stdio: 'inherit', | ||
| }); |
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.
P2: The install step now hardcodes npm install --force, ignoring the packageManager argument passed into build(). This breaks yarn/pnpm workflows and makes the install step inconsistent with the rest of the command. Restore the package-manager-aware install helper.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/react-email/src/commands/build.ts, line 231:
<comment>The install step now hardcodes `npm install --force`, ignoring the `packageManager` argument passed into `build()`. This breaks yarn/pnpm workflows and makes the install step inconsistent with the rest of the command. Restore the package-manager-aware install helper.</comment>
<file context>
@@ -227,10 +228,10 @@ export const build = async ({
spinner.text = 'Installing dependencies on `.react-email`';
- await installDependencies({
+ spawnSync('npm install --force', {
+ shell: true,
cwd: builtPreviewAppPath,
</file context>
| spawnSync('npm install --force', { | |
| shell: true, | |
| cwd: builtPreviewAppPath, | |
| silent: true, | |
| packageManager, | |
| stdio: 'inherit', | |
| }); | |
| await installDependencies({ | |
| cwd: builtPreviewAppPath, | |
| silent: true, | |
| packageManager, | |
| }); |
trying out to see if the nextjs canary doesn't have errors when using the nextjs build from macos and windows
Summary by cubic
Upgrade preview-server to Next.js 16.2.0-canary.23 to test cross-platform builds on macOS and Windows. Update the react-email build to run npm install --force in .react-email so SWC bindings install reliably and logs are visible.
Written for commit d1af7b1. Summary will update on new commits.