feat: build @nuxt/module-builder using rolldown#730
feat: build @nuxt/module-builder using rolldown#730DamianGlowala wants to merge 14 commits intonuxt:mainfrom
@nuxt/module-builder using rolldown#730Conversation
Co-authored-by: Copilot <copilot@github.com>
commit: |
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ 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. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
📝 WalkthroughWalkthroughRemoved the Unbuild export from build.config.ts (no build config remains there). package.json scripts changed to use Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@rolldown.config.ts`:
- Around line 17-22: Add the missing trailing commas to satisfy the linter: in
the rolldown config object add a trailing comma after the dts() entry inside the
plugins array (plugins: [ dts(), ]) and add a trailing comma after the
entryFileNames property value inside the output object (output: {
entryFileNames: '[name].mjs', }), leaving the existing plugins, dts(), output
and entryFileNames identifiers unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6f0c2a97-c234-4eba-9e76-715a4720292e
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
build.config.tspackage.jsonrolldown.config.ts
💤 Files with no reviewable changes (1)
- build.config.ts
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
| input: Object.fromEntries( | ||
| globSync('src/**/*.ts').map(file => [ | ||
| path.relative('src', file.slice(0, file.length - path.extname(file).length)) | ||
| .split(path.sep) | ||
| .join('/'), | ||
| path.resolve(file), | ||
| ]), | ||
| ), |
There was a problem hiding this comment.
we should only need cli + index entries, right?
There was a problem hiding this comment.
Ideally only these two, but rolldown wasn't bundling dynamic imports for some reason.
This approach aligns with https://rolldown.rs/reference/InputOptions.input#in-depth
LMK if you're aware of a better solution.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
package.json (1)
58-59: Move build-only rolldown packages todevDependencies.
rolldownandrolldown-plugin-dtsare used exclusively inrolldown.config.tsand have no runtime imports. Keeping them independenciesunnecessarily increases the install surface for consumers.Proposed dependency move
"dependencies": { @@ - "rolldown": "^1.0.0-rc.18", - "rolldown-plugin-dts": "^0.23.2", "tinyglobby": "^0.2.16", @@ "devDependencies": { + "rolldown": "^1.0.0-rc.18", + "rolldown-plugin-dts": "^0.23.2",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` around lines 58 - 59, Move the build-only packages "rolldown" and "rolldown-plugin-dts" from dependencies into devDependencies in package.json; edit package.json to remove the "rolldown" and "rolldown-plugin-dts" entries under "dependencies" and add identical version entries under "devDependencies" (preserving the versions "^1.0.0-rc.18" and "^0.23.2"), then run an install to verify lockfile updates.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@package.json`:
- Around line 58-59: Move the build-only packages "rolldown" and
"rolldown-plugin-dts" from dependencies into devDependencies in package.json;
edit package.json to remove the "rolldown" and "rolldown-plugin-dts" entries
under "dependencies" and add identical version entries under "devDependencies"
(preserving the versions "^1.0.0-rc.18" and "^0.23.2"), then run an install to
verify lockfile updates.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d64bff2a-e743-424f-a48f-74b47a353ff0
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
.gitignorepackage.jsonrolldown.config.ts
✅ Files skipped from review due to trivial changes (1)
- .gitignore
🚧 Files skipped from review as they are similar to previous changes (1)
- rolldown.config.ts
📚 Description
This moves building the Nuxt module builder library itself from unbuild to rolldown. Building userland modules with it using rolldown is the next step.