-
-
Notifications
You must be signed in to change notification settings - Fork 52
fix: enlarge rollup build memory limit #1468
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
Conversation
WalkthroughBuild orchestration in package.json was restructured to run CSS and components builds in parallel via a new build:rollup:process script and increased Node.js memory to 8192 MB. Rollup config removed the Babel React/JSX plugin and its usage; other plugins and settings remain unchanged. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant NPM as npm scripts
participant PB as prebuild
participant CSS as build:css
participant CMP as build:components
participant EXP as build:generate-exports
participant Roll as rollup
Dev->>NPM: run build:rollup
note right of NPM: Sets NODE_OPTIONS max-old-space-size=8192
NPM->>NPM: run build:rollup:process
NPM->>PB: npm run prebuild
PB-->>NPM: done
par Parallel builds
NPM->>CSS: npm run build:css
NPM->>CMP: npm run build:components
and
CSS-->>NPM: done
CMP-->>NPM: done
end
NPM->>EXP: npm run build:generate-exports
EXP-->>NPM: done
NPM->>Roll: rollup bundle (no Babel plugin)
Roll-->>Dev: build artifacts
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Codex Review: Here are some suggestions.
Reply with @codex fix comments
to fix any unresolved comments.
About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you open a pull request for review, mark a draft as ready, or comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex fix this CI failure" or "@codex address that feedback".
"build:css": "npm run generate-tokens && npm run bundle-tokens && npm run build-css", | ||
"build:components": "npm run compile-components && npm run process-components", | ||
"build:rollup": "npm run prebuild && npm run build:css && NODE_OPTIONS='--max-old-space-size=4076' npm run build:components && npm run build:generate-exports", | ||
"build:rollup": "NODE_OPTIONS='--max-old-space-size=8192' npm run build:rollup:process", | ||
"build:rollup:process": "npm run prebuild && (npm run build:css & npm run build:components & wait) && npm run build:generate-exports", |
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.
[P1] Ensure parallel rollup steps fail when any subcommand fails
The new build:rollup:process
script backgrounds npm run build:css
and npm run build:components
and then calls wait
. In POSIX shells wait
without arguments returns the exit status of the last job to finish, so if the job that fails happens to finish first, the overall script will still exit 0 and the build proceeds as if everything succeeded. That means the rollup build can silently pass while either the CSS or component build has failed. Consider capturing each job’s PID and checking both exit codes (or using a helper like npm-run-all
) so the script fails whenever either subcommand does.
Useful? React with 👍 / 👎.
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.
Actionable comments posted: 1
🧹 Nitpick comments (2)
package.json (2)
26-26
: Set NODE_OPTIONS portably (use cross-env) and avoid clobbering existing flags.Inline env assignment with single quotes is not Windows-friendly and overwrites any pre-set
NODE_OPTIONS
.Minimal portable change:
-"build:rollup": "NODE_OPTIONS='--max-old-space-size=8192' npm run build:rollup:process", +"build:rollup": "cross-env NODE_OPTIONS=--max-old-space-size=8192 npm run build:rollup:process",Optional: allow overriding/augmenting via
cross-env-shell
:-"build:rollup": "NODE_OPTIONS='--max-old-space-size=8192' npm run build:rollup:process", +"build:rollup": "cross-env-shell \"NODE_OPTIONS=\\\"$NODE_OPTIONS --max-old-space-size=${ROLLUP_HEAP_MB:-8192}\\\" npm run build:rollup:process\"",Add
cross-env
(orcross-env-shell
) to devDependencies.
63-63
: Drop unused@rollup/plugin-babel
No references to@rollup/plugin-babel
inrollup.config.cjs
or any Babel config files—remove it fromdevDependencies
.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
package.json
(1 hunks)rollup.config.cjs
(0 hunks)
💤 Files with no reviewable changes (1)
- rollup.config.cjs
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: GoldGroove06
PR: rad-ui/ui#1214
File: package.json:64-64
Timestamp: 2025-07-14T18:13:36.606Z
Learning: In npm, lifecycle scripts with `pre` and `post` prefixes automatically run before and after their corresponding main scripts. For example, a `prebuild` script will automatically run before the `build` script when executing `npm run build`, so there's no need to explicitly call `npm run prebuild` in the build script.
📚 Learning: 2025-07-14T18:13:36.606Z
Learnt from: GoldGroove06
PR: rad-ui/ui#1214
File: package.json:64-64
Timestamp: 2025-07-14T18:13:36.606Z
Learning: In npm, lifecycle scripts with `pre` and `post` prefixes automatically run before and after their corresponding main scripts. For example, a `prebuild` script will automatically run before the `build` script when executing `npm run build`, so there's no need to explicitly call `npm run prebuild` in the build script.
Applied to files:
package.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (2)
package.json (2)
22-22
: Usingprebuild
explicitly here is correct.Per our past learning, npm only auto-runs
prebuild
for thebuild
script, not for custom scripts likebuild:rollup
. Keeping the explicit call is appropriate.
27-27
: No dependency between generated tokens and component build
I found no imports of generated CSS or JS token files in the component source; build:css always runsgenerate-tokens
andbundle-tokens
first, and components don’t consume those outputs, so parallelizing build:css and build:components is safe.
"build:components": "npm run compile-components && npm run process-components", | ||
"build:rollup": "npm run prebuild && npm run build:css && NODE_OPTIONS='--max-old-space-size=4076' npm run build:components && npm run build:generate-exports", | ||
"build:rollup": "NODE_OPTIONS='--max-old-space-size=8192' npm run build:rollup:process", | ||
"build:rollup:process": "npm run prebuild && (npm run build:css & npm run build:components & wait) && npm run build:generate-exports", |
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.
🛠️ Refactor suggestion
Make the parallel step cross‑platform (replace & ... & wait
).
The current subshell/background syntax is Bash-only and will fail on Windows runners. Use a cross-platform runner like npm-run-all
or concurrently
.
Apply one of these diffs:
Option A (npm-run-all):
-"build:rollup:process": "npm run prebuild && (npm run build:css & npm run build:components & wait) && npm run build:generate-exports",
+"build:rollup:process": "npm run prebuild && npm-run-all -p build:css build:components && npm run build:generate-exports",
Option B (concurrently):
-"build:rollup:process": "npm run prebuild && (npm run build:css & npm run build:components & wait) && npm run build:generate-exports",
+"build:rollup:process": "npm run prebuild && concurrently \"npm run build:css\" \"npm run build:components\" && npm run build:generate-exports",
If you choose one, add the corresponding devDependency (npm-run-all
or concurrently
). I can raise a follow-up PR to wire this.
📝 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.
"build:rollup:process": "npm run prebuild && (npm run build:css & npm run build:components & wait) && npm run build:generate-exports", | |
"build:rollup:process": "npm run prebuild && npm-run-all -p build:css build:components && npm run build:generate-exports", |
🤖 Prompt for AI Agents
In package.json around line 27, the "build:rollup:process" script uses Bash-only
background/ wait syntax ("& ... & wait") which fails on Windows; replace that
part with a cross-platform runner: either (A) add "npm-run-all" as a
devDependency and use npm-run-all to run build:css and build:components in
parallel (then run build:generate-exports), or (B) add "concurrently" as a
devDependency and use concurrently to run build:css and build:components in
parallel (then run build:generate-exports); update the script to the chosen
tool's syntax and add the matching devDependency entry in devDependencies.
Summary
Testing
npm test
npm run build:rollup
https://chatgpt.com/codex/tasks/task_e_68ba86a15fec8331bebfdc17d9562bc4
Summary by CodeRabbit