refactor(global): migrate rollup build to tsup #25
Conversation
|
Warning Rate limit exceeded@SavelevMatthew has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 5 minutes and 25 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
WalkthroughThe pull request introduces significant updates across multiple packages in the Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (5)
packages/configs/package.json (1)
Line range hint
1-38: LGTM! The package.json changes align with the tsup migrationThe changes effectively transition the build system from Rollup to tsup while maintaining proper package structure and dependencies. The use of .mjs files and modern export fields is a good practice.
Consider documenting the migration process in the repository's documentation to help other teams performing similar migrations.
packages/npm/package.json (1)
23-29: LGTM! Exports field is properly configured.The exports field correctly defines:
- Types path for TypeScript support
- CommonJS (require) and ESM (import) entry points
This dual-format publishing strategy ensures maximum compatibility while supporting modern ESM imports.
packages/configs/tsup.mjs (2)
5-5: Consider supporting more TypeScript extensionsThe
ALLOWED_EXTENSIONSarray is currently limited to.tsfiles. Consider adding support for other common TypeScript extensions.-const ALLOWED_EXTENSIONS = ['.ts'] +const ALLOWED_EXTENSIONS = ['.ts', '.tsx', '.mts', '.cts']
21-32: Consider exposing more tsup configuration optionsThe build pipeline configuration is good but could be more flexible.
-export function createBuildPipeline(exports) { +export function createBuildPipeline(exports, options = {}) { return defineConfig({ entry: _getInputs(exports), clean: true, dts: true, sourcemap: true, format: ['cjs', 'esm'], target: 'node16', minify: true, splitting: false, + ...options, // Allow overriding defaults }) }packages/github/package.json (1)
Line range hint
1-1: Well-structured build system migrationThe migration from rollup to tsup has been implemented systematically with:
- Centralized build configuration in a shared package
- Consistent export field updates across package.json files
- Modern module resolution patterns
Consider documenting the new build system setup in the repository's README or a dedicated documentation file to help future contributors understand the build process.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (19)
packages/commit-analyzer/package.json(3 hunks)packages/commit-analyzer/tsup.config.mjs(1 hunks)packages/configs/package.json(1 hunks)packages/configs/rollup.js(0 hunks)packages/configs/tsconfig.json(2 hunks)packages/configs/tsup.mjs(1 hunks)packages/git/package.json(3 hunks)packages/git/rollup.config.mjs(0 hunks)packages/git/tsup.config.mjs(1 hunks)packages/github/package.json(3 hunks)packages/github/rollup.config.mjs(0 hunks)packages/github/tsup.config.mjs(1 hunks)packages/mono-pub/package.json(3 hunks)packages/mono-pub/rollup.config.mjs(0 hunks)packages/mono-pub/tsconfig.json(1 hunks)packages/mono-pub/tsup.config.mjs(1 hunks)packages/npm/package.json(3 hunks)packages/npm/rollup.config.mjs(0 hunks)packages/npm/tsup.config.mjs(1 hunks)
💤 Files with no reviewable changes (5)
- packages/git/rollup.config.mjs
- packages/github/rollup.config.mjs
- packages/mono-pub/rollup.config.mjs
- packages/npm/rollup.config.mjs
- packages/configs/rollup.js
✅ Files skipped from review due to trivial changes (5)
- packages/commit-analyzer/tsup.config.mjs
- packages/mono-pub/tsup.config.mjs
- packages/git/tsup.config.mjs
- packages/npm/tsup.config.mjs
- packages/github/tsup.config.mjs
🔇 Additional comments (14)
packages/configs/package.json (2)
11-12: Verify the existence of new configuration files
The migration to .mjs files aligns with modern ES modules practices. Let's verify that these new files exist in the repository.
✅ Verification successful
Both configuration files exist as expected
The new configuration files eslint.config.mjs and tsup.mjs are present in the packages/configs directory, which aligns with the package.json file entries.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the new configuration files exist
# Expected: Both files should exist in the packages/configs directory
fd -t f "eslint.config.mjs|tsup.mjs" packages/configs/
Length of output: 117
21-22: Verify tsup version constraints and latest stable release
The package uses tsup v8.3.5 as a dev dependency and ^8 as a peer dependency. Let's verify:
- If v8.3.5 is the latest stable version
- If there are any known issues with major version 8
Also applies to: 30-30
✅ Verification successful
Dependencies are up-to-date and properly versioned
The package uses the latest stable version of tsup (v8.3.5) and the version constraints are appropriate:
- devDependencies: ^8.3.5 (specific version for development)
- peerDependencies: ^8 (flexible for consumers)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check latest tsup version and recent releases
# Expected: v8.3.5 should be reasonably up-to-date
# Check latest version on npm
echo "Latest tsup version:"
curl -s https://registry.npmjs.org/tsup/latest | jq '.version'
# Check recent releases for any critical issues
echo "Recent releases:"
curl -s https://api.github.com/repos/egoist/tsup/releases?per_page=5 | jq '.[].name'
Length of output: 298
packages/configs/tsconfig.json (1)
5-6: Verify Node.js version compatibility with ES2022 and NodeNext.
The upgrade to ES2022 and NodeNext module resolution is a good modernization step. However, we should ensure all deployment environments support these features.
Let's check the Node.js version requirements:
Also applies to: 15-15
✅ Verification successful
ES2022 and NodeNext are compatible with the project's dependencies
Looking at the package.json files, we can see that:
- The project uses TypeScript 5.x (
typescript: "^5.0.4"and"^5.7.2") - Modern build tools like tsup 8.x are used
- No explicit Node.js version constraints are defined in any package.json
- The tooling choices (TypeScript 5, tsup 8) implicitly suggest a modern Node.js environment
Since ES2022 features are fully supported in Node.js 16+ and NodeNext module resolution was introduced in TypeScript 4.7+, these compiler options are safe to use with the current project setup.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check package.json files for Node.js engine requirements
# Look for engine requirements in package.json files
fd package.json | xargs cat | jq -r 'select(.engines != null) | .engines.node'
Length of output: 80
Script:
#!/bin/bash
# Let's try a different approach to find package.json files and read them one by one
fd package.json -x sh -c 'echo "=== $1 ==="; cat "$1"' sh {} | grep -A 1 '"engines"'
# Also check for any README or documentation that might mention Node.js version
fd -e md -x grep -l -i "node" {} | xargs cat | grep -i "node.*version\|version.*node"
Length of output: 176
Script:
#!/bin/bash
# Let's try one more approach to check package.json files
fd package.json -x cat {}
# Also check for any .nvmrc or .node-version files that might specify Node version
fd -e nvmrc -e node-version -x cat {}
Length of output: 8822
packages/npm/package.json (2)
38-38: Verify tsup version consistency across packages.
The tsup version should be consistent across all packages in the monorepo.
Let's check tsup versions across all package.json files:
✅ Verification successful
tsup version is consistent across all packages
All package.json files in the monorepo use the same tsup version ^8.3.5. No version mismatches found.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check tsup version consistency
# Find all package.json files and extract tsup version
fd package.json | xargs cat | jq -r 'select(.devDependencies.tsup != null) | .devDependencies.tsup'
Length of output: 143
12-12: Verify tsup configuration file exists.
The build command has been simplified to just tsup, but we need to ensure a proper tsup configuration file exists.
Let's check for the tsup config file:
✅ Verification successful
Let me check the content of the tsup configuration file to ensure it's properly configured.
Tsup configuration is properly set up ✅
The tsup configuration file exists and is correctly configured using a shared build pipeline from @mono-pub/configs/tsup that processes the exports field from package.json.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for tsup configuration
# Look for tsup config files
fd -g "tsup.config.*" packages/npm/
Length of output: 66
Script:
#!/bin/bash
# Check the content of tsup config file
cat packages/npm/tsup.config.mjs
Length of output: 273
packages/commit-analyzer/package.json (2)
23-29: LGTM! Export field configuration follows Node.js best practices
The new exports field correctly defines conditional exports for different module systems (CommonJS and ESM) and TypeScript types.
38-38: Verify tsup version compatibility
The tsup version is pinned to ^8.3.5. Let's verify this version's compatibility with the project's Node.js target.
✅ Verification successful
Let me verify the Node.js version requirement of the project to ensure compatibility.
tsup version ^8.3.5 is consistently used and compatible
The version is used consistently across all packages in the monorepo, and tsup 8.3.5 requires Node.js >=18 which is a reasonable requirement for a modern TypeScript build tool. No compatibility issues found.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check tsup's package.json for Node.js engine requirements
curl -s https://registry.npmjs.org/tsup/8.3.5 | jq -r '.engines.node'
# Check if this version is used consistently across packages
rg '"tsup":\s*"\^8\.3\.5"' -g 'package.json'
Length of output: 431
Script:
#!/bin/bash
# Check engines field in package.json files
rg '"engines"' -A 3 -g 'package.json'
# Also check root package.json
cat package.json | jq '.engines'
Length of output: 75
packages/github/package.json (2)
23-29: LGTM! Export configuration matches repository-wide pattern
The exports field configuration is consistent with other packages in the monorepo.
39-39: Consistent tsup version across packages
The tsup version matches other packages, maintaining consistency in the monorepo.
packages/git/package.json (2)
35-37: LGTM! Export paths follow recommended structure
The updated export paths follow the recommended structure:
- Proper separation of ESM (.mjs) and CJS (.js) outputs
- Organized utils under a dedicated directory
- Maintains TypeScript type definitions
Also applies to: 40-42
12-12: Verify tsup configuration file existence
The build script has been updated to use tsup, but we should ensure a corresponding configuration file exists.
✅ Verification successful
Let me verify the contents of the tsup configuration file to ensure it's properly configured.
Let me check the shared build pipeline configuration to ensure it's properly set up.
Let me try to locate the correct path for the shared configuration file.
✓ tsup configuration is properly set up
The build configuration is correctly implemented with:
- A valid tsup.config.mjs file in the git package
- Proper shared configuration in @mono-pub/configs that:
- Generates both CommonJS and ESM outputs
- Includes type definitions and sourcemaps
- Targets Node.js 16
- Uses proper minification and cleaning
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for tsup config file existence
fd -e mjs -e js -e ts "tsup.config" packages/git/
Length of output: 78
Script:
#!/bin/bash
# Check contents of tsup config file
cat packages/git/tsup.config.mjs
Length of output: 274
Script:
#!/bin/bash
# Check the shared tsup build pipeline configuration
cat packages/configs/src/tsup.ts
Length of output: 95
Script:
#!/bin/bash
# Find the tsup configuration file in the configs package
fd -e ts -e mjs -e js "tsup" packages/configs/
Length of output: 72
Script:
#!/bin/bash
# Check the shared tsup configuration
cat packages/configs/tsup.mjs
Length of output: 1012
packages/mono-pub/package.json (3)
64-64: LGTM! Dependencies are aligned
The tsup version (8.3.5) is consistent with the git package and is the latest stable version.
46-48: Verify no breaking changes in export paths
The export paths have been updated to follow the new structure. Let's verify there are no breaking changes for existing consumers.
Also applies to: 51-53
✅ Verification successful
Let me gather more information to verify potential breaking changes by checking for other import patterns.
No breaking changes detected in export paths
The verification shows that:
- All current imports are using the package name 'mono-pub' without file extensions
- The previous package.json had identical export paths in the same structure
- No imports were found using explicit .cjs extensions
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for import statements using the old paths
rg -t js -t ts "require\(['\"]mono-pub[/\w]*\.cjs['\"]" || echo "No old CJS imports found"
Length of output: 120
Script:
#!/bin/bash
# Check for any imports of mono-pub in the codebase
rg -t js -t ts "(?:import|require).*['\"]mono-pub['\"]" -A 2
# Check package.json history to see previous export paths
git show HEAD~1:packages/mono-pub/package.json | grep -A 5 '"exports":'
Length of output: 3031
27-27: Verify consistent build configuration across packages
As this is the main package, we should ensure the build configuration is consistent with other packages in the monorepo.
✅ Verification successful
Let me analyze the tsup configuration files to ensure they are consistent and properly configured for all packages.
Let me verify the shared build pipeline configuration and exports to ensure everything is properly configured across all packages.
Let me verify the shared tsup configuration and ensure version consistency across packages before making the final determination.
Build configuration is consistent across packages
The verification shows:
- All packages use the same build script (
tsup) - Each package has identical tsup configuration using the shared
@mono-pub/configs/tsup - The shared configuration properly handles all necessary outputs (CJS, ESM, types)
- All packages use the same tsup version (^8.3.5)
- Export paths are consistently configured across packages
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for consistent build scripts and tsup configs across packages
echo "Checking build scripts consistency:"
fd package.json packages/ -x jq -r 'select(.scripts.build != null) | "\(.name): \(.scripts.build)"'
echo -e "\nChecking tsup configs:"
fd "tsup.config" packages/
Length of output: 545
Script:
#!/bin/bash
# Check tsup configurations across packages for consistency
echo "Checking tsup configurations:"
for config in $(fd "tsup.config" packages/); do
echo -e "\n=== $config ==="
cat "$config"
done
Length of output: 2204
Script:
#!/bin/bash
# Check the shared build pipeline configuration
echo "Checking shared build pipeline configuration:"
fd "tsup.ts" -e ts -e js -e mjs packages/configs/
echo -e "\nChecking exports configuration in package.json files:"
for pkg in $(fd package.json packages/); do
echo -e "\n=== $pkg ==="
jq '.exports' "$pkg"
done
Length of output: 2333
Script:
#!/bin/bash
# Check the shared tsup configuration file
echo "Checking shared tsup configuration:"
cat packages/configs/tsup.mjs
echo -e "\nChecking if all packages use the same tsup version:"
for pkg in $(fd package.json packages/); do
echo -e "\n=== $pkg ==="
jq '.dependencies.tsup, .devDependencies.tsup' "$pkg"
done
Length of output: 2597
Summary by CodeRabbit
New Features
tsupacross multiple packages.Bug Fixes
Documentation
package.jsonfiles to reflect changes in build tools and module exports.Chores
tsupacross various packages.