feat!: migrate from unbuild to rolldown#751
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR migrates the module-builder project from unbuild to rolldown. It removes the unbuild build config, adds rolldown.config.ts, updates package.json scripts and dependencies (including Node engine bump), and replaces the build command with a rolldown-based pipeline that infers entries, compiles runtime assets, emits .mjs bundles and DTS, and writes module metadata. Tests and snapshots were updated to reflect new export shapes, Vue type import paths, and builder metadata/versioning. Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/commands/build.ts`:
- Around line 54-56: The early returns in the stub-path (calls to buildStub)
skip the dist cleanup and type-shim generation; remove the immediate returns
after buildStub calls (e.g., the if (context.args.stub) { await buildStub(cwd,
distDir, moduleEntries); return } blocks) and instead let execution continue so
the existing dist cleanup logic and writeTypes(distDir, true) run; ensure every
place that calls buildStub (referenced by buildStub, context.args.stub, distDir,
writeTypes) does not short-circuit the flow and still triggers the cleanup and
writeTypes invocation (apply the same change to the other stub occurrences
mentioned).
- Around line 110-112: The current logic that builds moduleInput from
moduleEntries uses basename(filename) and drops the export subpath, causing
collisions (e.g., ./dist/utils/index.mjs → dist/index.mjs); update the mapping
in inferModuleEntries (and the other similar spots noted) so the entry name
preserves the export subpath by extracting and using the capture group
(match[1]) from the path pattern (e.g., from "./dist/<subpath>.mjs") when
present, falling back to basename(filename(e), extname(e)) only if match[1] is
undefined; apply the same change where module entries are collected so
functions/variables like moduleEntries, moduleInput, filename(), basename(),
extname(), and any regex match usage consistently use the preserved subpath as
the key.
- Around line 79-83: The runtime file discovery currently only searches for .ts
and .tsx via the glob call that sets runtimeTsFiles; update that glob pattern to
include .mts (e.g. '**/*.{ts,tsx,mts}') and also extend the ignore patterns to
exclude stories/tests for .mts (e.g. add .mts to '**/*.stories.{ts,tsx}' and
'**/*.{spec,test}.{ts,tsx}'), so runtime .mts sources are discovered and the
test/story exclusions still apply.
In `@test/build.spec.ts`:
- Line 139: The inline snapshot assertion on runtimeImport!.code.trim() contains
unnecessary escaped quotes (\"), triggering no-useless-escape; update the
snapshot string so it contains normal double quotes without backslashes (e.g.,
import { SharedTypeFromRuntime } from "./runtime/plugins/plugin.js";) so the
inline snapshot matches the actual trimmed code and removes the redundant
escapes in the expect(...).toMatchInlineSnapshot call.
🪄 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: 74208684-8aeb-4ba3-879b-a88e1f323d35
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
build.config.tspackage.jsonrolldown.config.tssrc/commands/build.tstest/__snapshots__/JsxComponent.d.tstest/__snapshots__/JsxComponent.jstest/__snapshots__/TestMe.vuetest/__snapshots__/TestMe.vue.d.tstest/__snapshots__/TestMeSetup.d.vue.tstest/__snapshots__/TestMeSetup.vuetest/__snapshots__/TestMeSetup.vue.d.tstest/build.spec.ts
💤 Files with no reviewable changes (1)
- build.config.ts
| const runtimeTsFiles = await glob('**/*.{ts,tsx}', { | ||
| cwd: srcRuntimeDir, | ||
| absolute: true, | ||
| ignore: ['**/*.stories.{ts,tsx}', '**/*.{spec,test}.{ts,tsx}'], | ||
| }) |
There was a problem hiding this comment.
Runtime discovery skips .mts files.
The runtime glob only includes .ts and .tsx, so src/runtime/**/*.mts sources are never built even though the rest of the command already supports .mts elsewhere.
💡 Suggested fix
- const runtimeTsFiles = await glob('**/*.{ts,tsx}', {
+ const runtimeTsFiles = await glob('**/*.{ts,tsx,mts}', {
cwd: srcRuntimeDir,
absolute: true,
- ignore: ['**/*.stories.{ts,tsx}', '**/*.{spec,test}.{ts,tsx}'],
+ ignore: ['**/*.stories.{ts,tsx,mts}', '**/*.{spec,test}.{ts,tsx,mts}'],
})📝 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.
| const runtimeTsFiles = await glob('**/*.{ts,tsx}', { | |
| cwd: srcRuntimeDir, | |
| absolute: true, | |
| ignore: ['**/*.stories.{ts,tsx}', '**/*.{spec,test}.{ts,tsx}'], | |
| }) | |
| const runtimeTsFiles = await glob('**/*.{ts,tsx,mts}', { | |
| cwd: srcRuntimeDir, | |
| absolute: true, | |
| ignore: ['**/*.stories.{ts,tsx,mts}', '**/*.{spec,test}.{ts,tsx,mts}'], | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/commands/build.ts` around lines 79 - 83, The runtime file discovery
currently only searches for .ts and .tsx via the glob call that sets
runtimeTsFiles; update that glob pattern to include .mts (e.g.
'**/*.{ts,tsx,mts}') and also extend the ignore patterns to exclude
stories/tests for .mts (e.g. add .mts to '**/*.stories.{ts,tsx}' and
'**/*.{spec,test}.{ts,tsx}'), so runtime .mts sources are discovered and the
test/story exclusions still apply.
| const moduleInput = Object.fromEntries( | ||
| moduleEntries.map(e => [filename(e) || basename(e, extname(e)), e]), | ||
| ) |
There was a problem hiding this comment.
Preserve the export subpath when deriving entry names.
inferModuleEntries() drops the ./dist/... portion and both build paths reconstruct names from basename(). An export like ./dist/utils/index.mjs will now emit dist/index.mjs, and two exports with the same leaf filename will collide. That breaks package.json#exports for nested subpaths.
💡 Suggested direction
-type ModuleEntry = string
+type ModuleEntry = { name: string, source: string }
-const moduleEntries = await inferModuleEntries(cwd)
+const moduleEntries = await inferModuleEntries(cwd)
const moduleInput = Object.fromEntries(
- moduleEntries.map(e => [filename(e) || basename(e, extname(e)), e]),
+ moduleEntries.map(e => [e.name, e.source]),
)
-for (const resolvedEntry of entries) {
- const entryName = filename(resolvedEntry) || basename(resolvedEntry, extname(resolvedEntry))
- const outBase = resolve(distDir, entryName)
+for (const entry of entries) {
+ const outBase = resolve(distDir, entry.name)and keep match[1] from ./dist/<subpath>.mjs as the name when collecting entries.
Also applies to: 246-258, 270-272
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/commands/build.ts` around lines 110 - 112, The current logic that builds
moduleInput from moduleEntries uses basename(filename) and drops the export
subpath, causing collisions (e.g., ./dist/utils/index.mjs → dist/index.mjs);
update the mapping in inferModuleEntries (and the other similar spots noted) so
the entry name preserves the export subpath by extracting and using the capture
group (match[1]) from the path pattern (e.g., from "./dist/<subpath>.mjs") when
present, falling back to basename(filename(e), extname(e)) only if match[1] is
undefined; apply the same change where module entries are collected so
functions/variables like moduleEntries, moduleInput, filename(), basename(),
extname(), and any regex match usage consistently use the preserved subpath as
the key.
commit: |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
DamianGlowala
changed the title
1 participant
🔗 Linked issue
resolves #726
📚 Description
This is an initial attempt (WIP) to migrate from unbuild to rolldown (both for building this module and userland ones), so that the modules ecosystem can start preparing for Nuxt 5.