chore(react/transform): migrate from esbuild to Rslib#2239
chore(react/transform): migrate from esbuild to Rslib#2239colinaaa wants to merge 4 commits intolynx-family:mainfrom
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
📝 WalkthroughWalkthroughReplace ad-hoc Node.js WASM build with an rslib/rsbuild configuration, add a buffer loader and rspack aliases, remove esbuild devDependency and legacy build script, adjust wasm import and build inputs, and update tests to assert raw warnings instead of using esbuild formatting. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (beta)
No actionable comments were generated in the recent review. 🎉 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 |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/react/transform/src/wasm.js`:
- Around line 7-9: The top-of-file build comment in wasm.js is stale: replace or
remove the line starting with "build with `./node_modules/.bin/esbuild ..." so
it no longer references the old esbuild command; either update it to the current
build instruction that uses "rslib build" or remove the comment entirely to
avoid confusion (the import line "import bytes from '#react_transform.wasm';"
and the file wasm.js are the anchors to locate the change).
🧹 Nitpick comments (2)
packages/react/transform/rslib.config.ts (2)
55-61: The WASM path traverses three directories up to the repo root — verify this holds in all build environments.
path.resolve(root, '../../../target/...')assumes the monorepo'starget/directory is exactly three levels abovepackages/react/transform. This is correct for the standard layout but could break if the package is ever moved or if cargo's target directory is overridden (e.g., viaCARGO_TARGET_DIR).Consider deriving the repo root dynamically (e.g., from a workspace root marker or
git rev-parse --show-toplevel) instead of hard-coding the relative depth, or at minimum add a comment documenting this assumption.
10-27: Optional: Consider adding explicit type annotation forapiparameter for consistency and clarity.The
satisfies RsbuildPluginconstraint provides structural type inference for theapiparameter, but explicit annotation makes the intent clearer in strictest mode. The codebase shows mixed patterns—some setup methods use explicitRsbuildPluginAPIannotation (e.g., in test files), while others rely on satisfies inference. Adding an explicit type would align with the strictest TypeScript configuration.💡 Suggested annotation
- setup(api) { + setup(api: Parameters<RsbuildPlugin['setup']>[0]) {Or use the exported
RsbuildPluginAPItype directly if available from@rslib/core.
Web Explorer#7674 Bundle Size — 383.74KiB (0%).df45887(current) vs 87682f6 main#7667(baseline) Bundle metrics
Bundle size by type
|
| Current #7674 |
Baseline #7667 |
|
|---|---|---|
252.83KiB |
252.83KiB |
|
95.85KiB |
95.85KiB |
|
35.06KiB |
35.06KiB |
Bundle analysis report Branch colinaaa:chore/migrate-esbuild-t... Project dashboard
Generated by RelativeCI Documentation Report issue
Merging this PR will degrade performance by 6.07%
Performance Changes
Comparing Footnotes
|
Summary by CodeRabbit
Checklist