chore: migrate Rslib to Rstack CLI#8170
Conversation
📝 WalkthroughWalkthroughThe repository migrates package build commands from 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Checkov (3.3.8)packages/core/package.jsonTraceback (most recent call last): packages/create-rsbuild/package.jsonTraceback (most recent call last): packages/plugin-babel/package.jsonTraceback (most recent call last):
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: 1
🧹 Nitpick comments (1)
packages/core/rstack.config.ts (1)
42-63: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHoist the
jiticheck out of theregexpMaploop.The
/jiti/.test(request)branch (lines 51-54) doesn't usename/testfrom the loop, so it's redundantly re-evaluated on every iteration instead of once. It also reads as if tied to the current entry, which can mislead future edits.♻️ Suggested refactor
({ request }, callback) => { const entries = Object.entries(regexpMap); - if (request) { + if (request) { + if (/jiti/.test(request)) { + callback(undefined, '../compiled/jiti/lib/jiti.mjs'); + return; + } for (const [name, test] of entries) { if (request === name) { throw new Error( `"${name}" is not allowed to be imported, use "requireCompiledPackage" instead.`, ); } - if (/jiti/.test(request)) { - callback(undefined, '../compiled/jiti/lib/jiti.mjs'); - return; - } if (test.test(request)) { callback(undefined, `../compiled/${name}/index.js`); return; } } } callback(); },🤖 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 `@packages/core/rstack.config.ts` around lines 42 - 63, Move the `/jiti/.test(request)` handling out of the `for (const [name, test] of entries)` loop in the request resolver, placing it once before iterating `regexpMap`. Preserve its callback target and early return, while leaving the `name`/`test` matching and disallowed-import logic inside the loop.
🤖 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 `@packages/core/rstack.config.ts`:
- Around line 86-94: Update the explanatory comment above the placeholder
replacement plugin to replace stale “Rslib” and “rslib” references with the
current Rstack/Rsbuild-based rs lib tooling, while preserving the existing
explanation of the __webpack_require__.* workaround and post-bundle replacement
behavior.
---
Nitpick comments:
In `@packages/core/rstack.config.ts`:
- Around line 42-63: Move the `/jiti/.test(request)` handling out of the `for
(const [name, test] of entries)` loop in the request resolver, placing it once
before iterating `regexpMap`. Preserve its callback target and early return,
while leaving the `name`/`test` matching and disallowed-import logic inside the
loop.
🪄 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 Plus
Run ID: 04701940-4d89-44c2-99da-0440d48ff53a
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (39)
package.jsonpackages/core/package.jsonpackages/core/rslib.config.tspackages/core/rstack.config.tspackages/create-rsbuild/package.jsonpackages/plugin-babel/package.jsonpackages/plugin-babel/rslib.config.tspackages/plugin-babel/rstack.config.tspackages/plugin-less/package.jsonpackages/plugin-less/rslib.config.tspackages/plugin-less/rstack.config.tspackages/plugin-preact/package.jsonpackages/plugin-preact/rslib.config.tspackages/plugin-preact/rstack.config.tspackages/plugin-react/package.jsonpackages/plugin-react/rslib.config.tspackages/plugin-react/rstack.config.tspackages/plugin-sass/package.jsonpackages/plugin-sass/rslib.config.tspackages/plugin-sass/rstack.config.tspackages/plugin-solid/package.jsonpackages/plugin-solid/rslib.config.tspackages/plugin-solid/rstack.config.tspackages/plugin-svelte/package.jsonpackages/plugin-svelte/rslib.config.tspackages/plugin-svelte/rstack.config.tspackages/plugin-svgr/package.jsonpackages/plugin-svgr/rslib.config.tspackages/plugin-svgr/rstack.config.tspackages/plugin-tailwindcss/package.jsonpackages/plugin-tailwindcss/rslib.config.tspackages/plugin-tailwindcss/rstack.config.tspackages/plugin-vue/package.jsonpackages/plugin-vue/rslib.config.tspackages/plugin-vue/rstack.config.tspnpm-workspace.yamlscripts/config/lib.tsscripts/config/package.jsonscripts/config/test.ts
💤 Files with no reviewable changes (13)
- packages/plugin-preact/rslib.config.ts
- packages/plugin-less/rslib.config.ts
- packages/plugin-svelte/rslib.config.ts
- packages/plugin-vue/rslib.config.ts
- packages/plugin-solid/rslib.config.ts
- packages/plugin-babel/rslib.config.ts
- packages/plugin-sass/rslib.config.ts
- packages/plugin-react/rslib.config.ts
- packages/plugin-svgr/rslib.config.ts
- package.json
- packages/plugin-tailwindcss/rslib.config.ts
- packages/core/rslib.config.ts
- pnpm-workspace.yaml
| // Rslib currently doesn't provide a way to preserve `__webpack_require__.*` | ||
| // references in the emitted bundle. | ||
| // | ||
| // To work around this, we use "magic" placeholder identifiers during authoring | ||
| // (e.g. `RSPACK_INTERCEPT_MODULE_EXECUTION`) so rslib | ||
| // won't try to resolve/transform `__webpack_require__.*` at build time. | ||
| // | ||
| // After bundling, this plugin performs a plain string replacement to swap | ||
| // those placeholders back to the actual Rspack runtime globals. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Stale "Rslib" reference in comment after migration.
This comment still says "Rslib currently doesn't provide..." and "won't try to resolve/transform __webpack_require__.*...rslib" even though the file has migrated off Rslib to the Rstack/Rsbuild-based rs lib pipeline. Update the wording to reflect the current tooling to avoid confusing future maintainers about which tool this workaround targets.
🤖 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 `@packages/core/rstack.config.ts` around lines 86 - 94, Update the explanatory
comment above the placeholder replacement plugin to replace stale “Rslib” and
“rslib” references with the current Rstack/Rsbuild-based rs lib tooling, while
preserving the existing explanation of the __webpack_require__.* workaround and
post-bundle replacement behavior.
1 participant
Summary
This PR migrates package library builds from standalone Rslib configs and commands to Rstack CLI.
It consolidates library and test definitions in each package's
rstack.config.ts, adds concise shared config exports, and removes direct Rslib and Rstest adapter dependencies.Generated library artifacts remain byte-for-byte unchanged.