build: Use cjs/esm build flow #5281
Conversation
WalkthroughThe rollup-plugin package build system was updated to output both CommonJS and ES module formats, with improved type declaration handling. The package.json, Rollup config, and TypeScript configs were modified to support dual builds, explicit module entry points, and better type resolution. Two new TypeScript configs for CJS and ESM outputs were introduced. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Node.js (CJS)
participant ESM Loader
participant Package
User->>Node.js (CJS): require('rollup-plugin-bundle-stats')
Node.js (CJS)->>Package: Loads ./lib/cjs/index.js
User->>ESM Loader: import 'rollup-plugin-bundle-stats'
ESM Loader->>Package: Loads ./lib/esm/index.js
User->>Package: import types from 'rollup-plugin-bundle-stats'
Package->>Package: Loads ./lib/types/index.d.ts
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Possibly related PRs
Poem
Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. ✨ 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 comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
#12472 Bundle Size — 382.46KiB (0%).032baca(current) vs 19b2c70 master#12458(baseline) Warning Bundle contains 2 duplicate packages – View duplicate packages Bundle metrics
|
| Current #12472 |
Baseline #12458 |
|
|---|---|---|
 Initial JS |
334.85KiB |
334.85KiB |
| Initial CSS |
47.61KiB |
47.61KiB |
 Cache Invalidation |
0% |
28.65% |
| Chunks |
3 |
3 |
| Assets |
4 |
4 |
| Modules |
701 |
701 |
| Duplicate Modules |
0 |
0 |
| Duplicate Code |
0% |
0% |
| Packages |
39 |
39 |
| Duplicate Packages |
1 |
1 |
Bundle size by type no changes
| Current #12472 |
Baseline #12458 |
|
|---|---|---|
| JS |
334.85KiB |
334.85KiB |
| CSS |
47.61KiB |
47.61KiB |
Bundle analysis report Branch update-build-flows Project dashboard
Generated by RelativeCI Documentation Report issue
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/rollup-plugin/package.json (1)
16-17: Improve cross-platform compatibility of build-type script.The current echo-based approach may have cross-platform issues. Consider using a more robust approach for generating the module type files.
- "build-type": "echo '{\"type\":\"commonjs\"}' > lib/cjs/package.json && echo '{\"type\":\"module\"}' > lib/esm/package.json", + "build-type": "node -e \"const fs = require('fs'); fs.writeFileSync('lib/cjs/package.json', '{\\\"type\\\":\\\"commonjs\\\"}'); fs.writeFileSync('lib/esm/package.json', '{\\\"type\\\":\\\"module\\\"}');\"",This approach is more reliable across different operating systems and shell environments.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
packages/rollup-plugin/package.json(3 hunks)packages/rollup-plugin/rollup.config.mjs(2 hunks)packages/rollup-plugin/tsconfig.base.json(1 hunks)packages/rollup-plugin/tsconfig.cjs.json(1 hunks)packages/rollup-plugin/tsconfig.esm.json(1 hunks)
⏰ 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 (11)
packages/rollup-plugin/tsconfig.base.json (1)
7-8: LGTM! Proper setup for dual module builds.The changes correctly separate type declarations into a dedicated directory and enable source maps, which supports the new dual build architecture where both CJS and ESM builds can share the same type definitions.
packages/rollup-plugin/tsconfig.cjs.json (1)
1-6: LGTM! Clean CommonJS TypeScript configuration.The configuration properly extends the base config and sets the correct output directory for the CommonJS build. The minimal approach is appropriate.
packages/rollup-plugin/tsconfig.esm.json (1)
1-6: LGTM! Clean ESM TypeScript configuration.The configuration properly extends the base config and sets the correct output directory for the ES module build. The structure is consistent with the CJS configuration.
packages/rollup-plugin/rollup.config.mjs (5)
5-5: Good addition for JSON import support.The JSON plugin import enables proper handling of JSON imports in both build configurations.
17-17: Correct entry file naming for dual builds.The entryFileNames configuration properly separates CJS and ESM outputs into their respective subdirectories.
Also applies to: 44-44
25-36: Well-configured CommonJS build setup.The CJS build configuration includes:
- Proper extension resolution for CJS context
defaultIsModuleExports: 'auto'for better CommonJS compatibility- JSON plugin for JSON imports
- Dedicated TypeScript config for CJS output
- Test file exclusion
38-65: ESM build configuration looks good overall.The ESM build properly:
- Uses dedicated TypeScript config for ESM output
- Includes broader extension resolution (.mjs support)
- Maintains consistent plugin setup with JSON support
- Excludes test files appropriately
55-58: ESM build CommonJS plugin settings are correctThe ESM output excludes all
node_modulesviaexternal, and our source contains norequire,module.exports, orexports.patterns—so the default CommonJS plugin behavior is sufficient. No changes are needed here.packages/rollup-plugin/package.json (3)
5-7: Excellent dual module configuration.The package entry points are properly configured:
mainpoints to CommonJS buildmodulepoints to ESM buildtypespoints to shared type declarationsThis follows Node.js best practices for dual module packages.
51-51: Correct dependency addition for JSON plugin.The @rollup/plugin-json dependency is properly added to support JSON imports in the Rollup configuration.
55-61: Well-structured exports field for modern module resolution.The exports field properly defines entry points for:
require(CommonJS)import(ESM)types(TypeScript declarations)This enables proper module resolution in modern Node.js environments and bundlers.
vio
force-pushed
the
update-build-flows
branch
2 times, most recently
from
e80aa9b to
d6d5fc5
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/plugin-webpack-filter/rollup.config.mjs (3)
7-7: OK to rename; minor noteCONTEXT_DIR looks fine. If you adopt the suggestions below (remove context and switch preserveModulesRoot to 'src'), this constant may become unnecessary.
20-20: Prefer a stable preserveModulesRootUsing an absolute path can be sensitive to platform-specific path normalization. Most setups use the source folder name.
Option A (simple, common):
- preserveModulesRoot: CONTEXT_DIR, + preserveModulesRoot: 'src',Option B (if you want to keep absolute but ensure POSIX matching):
-const CONTEXT_DIR = path.join(process.cwd(), './src'); +const CONTEXT_DIR = path.resolve('src'); +const CONTEXT_POSIX = CONTEXT_DIR.split(path.sep).join('/'); ... - preserveModulesRoot: CONTEXT_DIR, + preserveModulesRoot: CONTEXT_POSIX,Please confirm which approach you prefer. If you pick Option A and remove the context property per above, CONTEXT_DIR can be dropped entirely.
Also applies to: 46-46
23-23: Make externals robust to monorepos/PNPM and avoid path-couplingexternal: /node_modules/ ties behavior to install location and can miss symlinked deps. Prefer externalizing bare imports + node: builtins.
- external: /node_modules/, + external: (id) => + id.startsWith('node:') || + (!id.startsWith('.') && !path.isAbsolute(id)),Applies to both outputs (Lines 23 and 49). This is the common pattern across Rollup repos and works well with monorepos.
Also applies to: 49-49
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
packages/plugin-webpack-filter/rollup.config.mjs(2 hunks)packages/rollup-plugin/package.json(3 hunks)packages/rollup-plugin/rollup.config.mjs(1 hunks)packages/rollup-plugin/src/index.ts(1 hunks)packages/rollup-plugin/tsconfig.base.json(1 hunks)packages/rollup-plugin/tsconfig.cjs.json(1 hunks)packages/rollup-plugin/tsconfig.esm.json(1 hunks)packages/rollup-plugin/tsconfig.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/rollup-plugin/src/index.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/rollup-plugin/tsconfig.cjs.json
- packages/rollup-plugin/tsconfig.esm.json
- packages/rollup-plugin/tsconfig.json
- packages/rollup-plugin/tsconfig.base.json
- packages/rollup-plugin/rollup.config.mjs
- packages/rollup-plugin/package.json
🔇 Additional comments (2)
packages/plugin-webpack-filter/rollup.config.mjs (2)
54-54: LGTM: default CommonJS interop for ESM buildUsing commonjsPlugin() with defaults for the ESM bundle is fine and keeps the config simple.
28-30: Asymmetry in CommonJS plugin config; also summary mismatchCJS config still sets defaultIsModuleExports: 'auto' (Lines 28–30), while ESM uses plugin defaults (Line 54). If that’s intentional, all good—just confirm. If the PR intent was to simplify both, consider unifying:
- commonjsPlugin({ - defaultIsModuleExports: 'auto', - }), + commonjsPlugin(),If you do need the option for CJS due to specific dependencies, add a brief inline comment explaining why to help future maintainers.
Also applies to: 54-54
Likely an incorrect or invalid review comment.
| export default defineConfig([ | ||
| { | ||
| context: CONTEXT, | ||
| context: CONTEXT_DIR, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Remove Rollup “context” — it’s not a directory path
Rollup’s config option context sets the top-level this binding during bundling; passing a filesystem path here is misleading and not what the option is for. It’s safe to drop it for both outputs.
Apply:
- context: CONTEXT_DIR,
+ // context not needed; use default module semanticsTo verify repo-wide, search for any other misuse:
Also applies to: 38-38
🏁 Script executed:
#!/bin/bash
fd -t f "rollup.config.*" | xargs -I{} rg -n --no-heading "^\s*context\s*:" {}Length of output: 221
🏁 Script executed:
#!/bin/bash
# Search all Rollup config files for any context usages, including file names
fd -t f "rollup.config.*" | xargs -I{} rg -n "^\s*context\s*:" {}Length of output: 208
🏁 Script executed:
#!/bin/bash
# List all occurrences of `context:` in Rollup config files with file paths
rg -nH '^\s*context\s*:' -g 'rollup.config.*'Length of output: 425
Drop misused Rollup context option across plugin configs
Rollup’s context option sets the top-level this binding during bundling; passing a filesystem path here is misleading. We can safely remove it (or replace with a comment) in all outputs:
• packages/rollup-plugin/rollup.config.mjs (line 13)
• packages/plugin-webpack-filter/rollup.config.mjs (lines 12, 38)
• packages/plugin-webpack-validate/rollup.config.mjs (lines 13, 38)
Apply the same diff in each file:
- context: CONTEXT_DIR,
+ // context not needed; use default module semantics
- context: CONTEXT,
+ // context not needed; use default module semanticsCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/plugin-webpack-filter/rollup.config.mjs at line 12, the Rollup
`context` option is incorrectly set to a filesystem path, which is misleading
because `context` should define the top-level `this` binding. Remove the
`context: CONTEXT_DIR,` line entirely from the config to fix this misuse. Also,
apply the same removal to the specified lines in the other mentioned files for
consistency.
Summary by CodeRabbit
New Features
Chores