-
Notifications
You must be signed in to change notification settings - Fork 169
Update vite.config.ts #1086
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Update vite.config.ts #1086
Conversation
| }, | ||
| }, | ||
| "/api/apps/chatgpt/sandbox-proxy": { | ||
| target: "http://localhost:6274", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this doesn't look right?
WalkthroughVite's proxy configuration in the development server was reconfigured to handle sandbox proxies with explicit route targeting. The unified 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
client/vite.config.ts (2)
67-83: Solid approach to bypass HMR injection.The
selfHandleResponsemechanism correctly prevents Vite from transforming sandbox responses. However, consider two refinements:
- Add error handling for the pipe operation to prevent silent failures:
configure: (proxy) => { proxy.on("proxyRes", (proxyRes, req, res) => { res.writeHead(proxyRes.statusCode || 200, proxyRes.headers); - proxyRes.pipe(res); + proxyRes.pipe(res).on("error", (err) => { + console.error("Sandbox proxy pipe error:", err); + if (!res.headersSent) res.writeHead(502); + res.end(); + }); }); },
- Verify WebSocket requirements for sandbox proxies—the general
/apiproxy enablesws: true, but these routes omit it.Does the sandbox proxy architecture require WebSocket support, or is standard HTTP sufficient?
84-94: Extract the duplicated configuration into a helper.The sandbox proxy configuration is duplicated between this route and
/api/mcp/sandbox-proxy. Reduce maintenance burden by extracting the shared logic:// Define before the proxy configuration const createSandboxProxyConfig = () => ({ target: "http://localhost:6274", changeOrigin: true, selfHandleResponse: true, configure: (proxy) => { proxy.on("proxyRes", (proxyRes, req, res) => { res.writeHead(proxyRes.statusCode || 200, proxyRes.headers); proxyRes.pipe(res).on("error", (err) => { console.error("Sandbox proxy pipe error:", err); if (!res.headersSent) res.writeHead(502); res.end(); }); }); }, });Then use it in the proxy configuration:
-"/api/apps/chatgpt/sandbox-proxy": { - target: "http://localhost:6274", - changeOrigin: true, - selfHandleResponse: true, - configure: (proxy) => { - proxy.on("proxyRes", (proxyRes, req, res) => { - res.writeHead(proxyRes.statusCode || 200, proxyRes.headers); - proxyRes.pipe(res); - }); - }, -}, +"/api/apps/chatgpt/sandbox-proxy": createSandboxProxyConfig(),Apply the same pattern to
/api/mcp/sandbox-proxy.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
client/vite.config.ts(1 hunks)
🧰 Additional context used
🔍 Remote MCP Context7
Let me search for more specific information about selfHandleResponse and the HTML transformation issue in Vite:Based on my research, I now have sufficient context to provide a comprehensive summary for reviewing this PR. Let me compile the findings:
Summary of Additional Context for PR Review
Vite Proxy Configuration Overview
If a request matches any of the configured proxy rules, the request won't be transformed by Vite. This is a key principle being leveraged in the PR to prevent unwanted transformations on sandbox proxy responses.
Understanding selfHandleResponse
When selfHandleResponse is set to true, none of the webOutgoing passes are called and it's your responsibility to appropriately return the response by listening and acting on the proxyRes event. This is critical for the sandbox proxy implementation—it allows direct control over response handling while bypassing Vite's standard response transformation pipeline.
The selfHandleResponse: true option with a proxyRes handler enables copying proxy response headers and piping the response directly to the client.
Vite HTML Transformation Context
The PR addresses preventing Vite's HTML transformation for sandbox proxy routes. This is important because wrapper apps that render user content in iframes may require custom WebSocket events to coordinate with the Vite backend and IDE, and the default Vite HMR client injection can interfere with this.
HMR Injection Concerns
The Vite team has acknowledged requests to provide a flag or configuration to prevent HMR client injection. The PR's approach of using per-route sandbox proxies with selfHandleResponse effectively bypasses Vite's HTML transformation middleware for these specific paths, preventing unwanted HMR script injection in sandbox iframes.
Additional Proxy Configuration Features
The PR restores the general /api proxy with standard Vite proxy options including http-proxy-3 under the hood, supporting target, changeOrigin, secure, and ws settings for WebSocket proxying.
---, [::web_search::]
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (1)
client/vite.config.ts (1)
95-101: General proxy configuration is sound.The placement after specific sandbox routes ensures correct routing precedence, and
ws: trueproperly enables WebSocket support for standard API calls. Usingsecure: falseforlocalhost:6274is appropriate in development.
b2b8ced to
51756ac
Compare
Note
Adds self-handled passthrough proxies for sandbox endpoints to prevent Vite HMR from modifying HTML, and splits out a general
/apiproxy.client/vite.config.ts):"/api/mcp/sandbox-proxy"and"/api/apps/chatgpt/sandbox-proxy"withselfHandleResponse: trueand directproxyRespiping to bypass Vite HTML transformation/HMR injections."/api"proxy withchangeOrigin,secure: false, andws: true./apiproxy handlers in favor of route-specific handling.Written by Cursor Bugbot for commit b2b8ced. This will update automatically on new commits. Configure here.