Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@
"build:generate-exports": "node scripts/generate-exports.cjs",
"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",
Comment on lines 24 to +27

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 👍 / 👎.

Copy link
Contributor

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.

Suggested change
"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.

"compile-components:esbuild": "node build-components.cjs",
"check:types": "tsc --noEmit",
"build-components:esbuild": "npm run compile-components:esbuild && npm run process-components",
Expand Down
7 changes: 0 additions & 7 deletions rollup.config.cjs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
const babel = require('@rollup/plugin-babel');
const resolve = require('@rollup/plugin-node-resolve');
const terser = require('@rollup/plugin-terser');
const typescript = require('@rollup/plugin-typescript');
Expand Down Expand Up @@ -35,11 +34,6 @@ const aliasPluginInstance = alias({
{ find: '~/core', replacement: path.resolve(__dirname, 'src/core') }
]
});
const babelPluginInstance = babel({
exclude: 'node_modules/**',
presets: ['@babel/preset-react'],
babelHelpers: 'bundled'
});
const terserPluginInstance = terser();
const resolvePluginInstance = resolve();
const bannerPluginInstance = banner2(() => '\'use client\';');
Expand All @@ -66,7 +60,6 @@ const jsBundles = {
external: ['react', 'react-dom', 'react/jsx-runtime'],
plugins: [
aliasPluginInstance,
babelPluginInstance,
typescriptPluginInstance,
resolvePluginInstance,
terserPluginInstance,
Expand Down
Loading