feat(start-plugin-core): support Rsbuild preview SSR middleware#7372
Open
elecmonkey wants to merge 2 commits intoTanStack:mainfrom
Open
feat(start-plugin-core): support Rsbuild preview SSR middleware#7372elecmonkey wants to merge 2 commits intoTanStack:mainfrom
elecmonkey wants to merge 2 commits intoTanStack:mainfrom
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR extends Rsbuild integration with preview mode support by refactoring dev-specific SSR middleware into a unified server-middleware layer, updating configuration schemas, adding E2E tests for preview mode, and optimizing React dependency resolution. ChangesRsbuild Preview Mode Support
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 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 unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
Contributor
Bundle Size Benchmarks
Current gzip tracks all emitted client JS chunks. Initial gzip tracks only the entry/import graph. Trend sparkline is historical current gzip ending with this PR measurement; lower is better. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/start-plugin-core/src/rsbuild/schema.ts`:
- Around line 11-13: The rsbuild schema currently only accepts
installServerMiddleware which breaks configs using the old
installDevServerMiddleware key; update the rsbuild zod object to accept both
installServerMiddleware and installDevServerMiddleware as optional booleans,
then normalize to a single canonical property (prefer installServerMiddleware
when both present) by adding a transform/preprocess so downstream code uses only
installServerMiddleware; mark installDevServerMiddleware as deprecated in the
schema/comments/types so it remains supported for one release.
In `@packages/start-plugin-core/src/rsbuild/server-middleware.ts`:
- Around line 185-193: The HTML response in the error path of
server-middleware.ts currently injects the raw exception message (e.message)
into the <pre>, causing reflected XSS; update the error rendering inside the new
Response to never output unescaped user-controlled text by either replacing the
displayed message with a generic string (e.g., "Internal server error") or by
HTML-escaping the value first (add a small helper like escapeHtml and call it
where e or e.message is interpolated). Ensure you reference the same Response
construction site and the exception variable e so the template uses the
escaped/generic text instead of e.message directly.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 42518d46-d3b2-436c-8ce5-3d554a2f3e68
📒 Files selected for processing (8)
e2e/react-start/basic/package.jsone2e/react-start/basic/playwright.config.tse2e/react-start/custom-server-rsbuild/rsbuild.config.tspackages/react-start/src/plugin/rsbuild.tspackages/start-plugin-core/src/rsbuild/dev-server.tspackages/start-plugin-core/src/rsbuild/plugin.tspackages/start-plugin-core/src/rsbuild/schema.tspackages/start-plugin-core/src/rsbuild/server-middleware.ts
💤 Files with no reviewable changes (1)
- packages/start-plugin-core/src/rsbuild/dev-server.ts
Comment on lines
11
to
13
| rsbuild: z | ||
| .object({ installDevServerMiddleware: z.boolean().optional() }) | ||
| .object({ installServerMiddleware: z.boolean().optional() }) | ||
| .optional(), |
Contributor
There was a problem hiding this comment.
Keep the old option as a deprecated alias for one release.
This rename is currently a breaking change. Existing configs that still set rsbuild.installDevServerMiddleware will silently re-enable the middleware after upgrade, because only installServerMiddleware is recognized now. Please accept both keys for now and prefer the new one when both are present.
🤖 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/start-plugin-core/src/rsbuild/schema.ts` around lines 11 - 13, The
rsbuild schema currently only accepts installServerMiddleware which breaks
configs using the old installDevServerMiddleware key; update the rsbuild zod
object to accept both installServerMiddleware and installDevServerMiddleware as
optional booleans, then normalize to a single canonical property (prefer
installServerMiddleware when both present) by adding a transform/preprocess so
downstream code uses only installServerMiddleware; mark
installDevServerMiddleware as deprecated in the schema/comments/types so it
remains supported for one release.
Comment on lines
+185
to
+193
| new Response( | ||
| `<!DOCTYPE html> | ||
| <html lang="en"> | ||
| <head><meta charset="UTF-8" /><title>Error</title></head> | ||
| <body> | ||
| <h1>Internal Server Error</h1> | ||
| <pre>${e instanceof Error ? e.message : String(e)}</pre> | ||
| </body> | ||
| </html>`, |
Contributor
There was a problem hiding this comment.
Escape the reflected error text before returning HTML.
The <pre> currently embeds e.message verbatim. If the thrown message includes request-controlled text, this turns SSR failures into reflected HTML injection/XSS. Please HTML-escape the value or replace it with a generic message.
Possible fix
+function escapeHtml(value: string): string {
+ return value.replace(/[&<>"']/g, (char) => {
+ switch (char) {
+ case '&':
+ return '&'
+ case '<':
+ return '<'
+ case '>':
+ return '>'
+ case '"':
+ return '"'
+ case "'":
+ return '''
+ default:
+ return char
+ }
+ })
+}- <pre>${e instanceof Error ? e.message : String(e)}</pre>
+ <pre>${escapeHtml(e instanceof Error ? e.message : String(e))}</pre>📝 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.
Suggested change
| new Response( | |
| `<!DOCTYPE html> | |
| <html lang="en"> | |
| <head><meta charset="UTF-8" /><title>Error</title></head> | |
| <body> | |
| <h1>Internal Server Error</h1> | |
| <pre>${e instanceof Error ? e.message : String(e)}</pre> | |
| </body> | |
| </html>`, | |
| function escapeHtml(value: string): string { | |
| return value.replace(/[&<>"']/g, (char) => { | |
| switch (char) { | |
| case '&': | |
| return '&' | |
| case '<': | |
| return '<' | |
| case '>': | |
| return '>' | |
| case '"': | |
| return '"' | |
| case "'": | |
| return ''' | |
| default: | |
| return char | |
| } | |
| }) | |
| } | |
| new Response( | |
| `<!DOCTYPE html> | |
| <html lang="en"> | |
| <head><meta charset="UTF-8" /><title>Error</title></head> | |
| <body> | |
| <h1>Internal Server Error</h1> | |
| <pre>${escapeHtml(e instanceof Error ? e.message : String(e))}</pre> | |
| </body> | |
| </html>`, |
🤖 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/start-plugin-core/src/rsbuild/server-middleware.ts` around lines 185
- 193, The HTML response in the error path of server-middleware.ts currently
injects the raw exception message (e.message) into the <pre>, causing reflected
XSS; update the error rendering inside the new Response to never output
unescaped user-controlled text by either replacing the displayed message with a
generic string (e.g., "Internal server error") or by HTML-escaping the value
first (add a small helper like escapeHtml and call it where e or e.message is
interpolated). Ensure you reference the same Response construction site and the
exception variable e so the template uses the escaped/generic text instead of
e.message directly.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Support Rsbuild preview SSR middleware.
Previously, because the output generated by
rsbuild buildincluded server-side code for SSR,rsbuild previewtreated it as static assets, which caused the preview to malfunction. The middleware code shared between dev mode and preview mode has now been extracted.Details
This updates the Rsbuild integration so Start’s server middleware is no longer dev-only.
Because the option now controls both dev and preview middleware installation, the Rsbuild option has been renamed:
to:
Summary by CodeRabbit
New Features
Improvements