-
Notifications
You must be signed in to change notification settings - Fork 647
refactor(react): use rolldown over rollup #7300
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: main
Are you sure you want to change the base?
Conversation
|
|
👋 Hi, this pull request contains changes to the source code that github/github-ui depends on. If you are GitHub staff, test these changes with github/github-ui using the integration workflow. Or, apply the |
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.
Pull request overview
This pull request migrates the @primer/react build system from rollup to rolldown for improved build performance. The migration involves replacing the rollup-plugin-import-css package with a new rolldown-compatible version (rolldown-plugin-import-css) and updating the build configuration to use rolldown's API and plugin structure.
Key Changes
- Replaced rollup bundler with rolldown for the @primer/react package build process
- Created a new
rolldown-plugin-import-csspackage to replace the rollup version, using rolldown's filter-based plugin API - Removed the build process from the plugin package (no longer compiles to dist, exports TypeScript source directly)
Reviewed changes
Copilot reviewed 10 out of 12 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/rollup-plugin-import-css/tsconfig.build.json | Deleted build-specific TypeScript configuration (no longer needed) |
| packages/rollup-plugin-import-css/src/index.ts | Deleted old rollup plugin implementation |
| packages/rollup-plugin-import-css/rollup.config.js | Deleted rollup build configuration for the plugin |
| packages/rollup-plugin-import-css/package.json | Deleted old plugin package definition |
| packages/rolldown-plugin-import-css/tsconfig.json | Added TypeScript configuration for new rolldown plugin |
| packages/rolldown-plugin-import-css/src/index.ts | New rolldown-compatible plugin using filter-based hooks |
| packages/rolldown-plugin-import-css/package.json | New package definition exporting TypeScript source directly |
| packages/rolldown-plugin-import-css/README.md | Updated documentation to reference rolldown instead of rollup |
| packages/react/script/build | Updated build script to use npx rolldown instead of npx rollup |
| packages/react/rolldown.config.mjs | New rolldown configuration replacing rollup config; removed node-resolve, commonjs, and preserve-directives plugins |
| packages/react/package.json | Added rolldown and rolldown-plugin-import-css dependencies, reordered @tanstack/react-virtual |
| package-lock.json | Updated dependencies to include rolldown bindings and related packages |
Comments suppressed due to low confidence (3)
packages/react/rolldown.config.mjs:125
- The previous rollup config included a custom "preserve-directives" plugin that prepended "use client" directives to output chunks when preserveModules was enabled. This plugin has been removed in the rolldown config, but there's no indication that rolldown handles this natively. Without this plugin, React Server Components may not work correctly as the "use client" directives from source files (like src/index.ts, src/experimental/index.ts, src/deprecated/index.ts, src/next/index.ts) won't be preserved in the built output. Verify that rolldown preserves module-level directives or add equivalent functionality.
packages/react/rolldown.config.mjs:124 - The rolldown configuration is missing the "interop" setting that was present in the rollup config output. The previous config specified
interop: 'auto'which controls how default and named exports are handled when mixing ES modules and CommonJS. Without this setting, there may be compatibility issues when consuming this package from projects using different module systems. Consider adding interop configuration if rolldown supports it.
packages/react/rolldown.config.mjs:100 - The rollup config previously included @rollup/plugin-node-resolve and @rollup/plugin-commonjs plugins to handle module resolution and CommonJS dependencies. These plugins have been removed without replacement. While rolldown may have built-in support for these features, this should be explicitly verified. Without proper module resolution, imports may fail, especially for dependencies that use CommonJS format or non-standard module resolution patterns.
Co-authored-by: Copilot <[email protected]>
|
Since you are willing to replace
|
|
You may also want to consider #7304, as also applicable to your PR. |
Update
@primer/reactto use rolldown over rollup for improved build performance.Changelog
New
Changed
rollup-plugin-import-cssto be rolldown compatible@primer/reactto use rolldown over rollup for bundlingRemoved
rollup-plugin-import-csssince it is no longer necessaryRollout strategy
As far as I can tell, this should be a 1:1 replacement so no changeset needed but will use integration tests to confirm 🤞