feat: add sub-directory deployment support and base path configuration #238
Conversation
- Introduced APP_BASE_PATH in .env.example for deployment path configuration. - Updated server/index.js to handle dynamic base path for routes, WebSocket connections, and static assets. - Modified React components to utilize BASE_URL for asset paths and router basename. - Enhanced WebSocket connection handling to respect the base path. - Adjusted API utility functions to prepend BASE_URL to relative paths.
WalkthroughAdds configurable APP_BASE_PATH support: server mounts routes under a router at that base path and injects it into served HTML/manifest; client reads window.APP_BASE_PATH (BASE_URL/ROUTER_BASENAME) to prefix API calls, WebSocket URLs, router basename, and asset imports. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant Server
participant expressApp as expressApp
participant router as Router (mounted)
participant Injector as HTML/Manifest Injector
Browser->>Server: GET /myapp/ (or /myapp/index.html)
Server->>expressApp: Accept request
expressApp->>router: Forward to router mounted at APP_BASE_PATH (/myapp)
router->>Injector: Request index/manifest
Injector->>Injector: Rewrite asset URLs, inject window.__APP_BASE_PATH__ = '/myapp'
Injector-->>Browser: Serve HTML/manifest with injected base path
Browser->>Browser: Initialize React app, read window.__APP_BASE_PATH__
Browser->>Server: API request to `${BASE_URL}/user` (via authenticatedFetch)
sequenceDiagram
participant App as React App
participant API as api.js
participant WS as websocket.js
participant Server as Backend (HTTP/WS)
App->>API: Read ROUTER_BASENAME / BASE_URL
App->>App: Router basename={ROUTER_BASENAME}
App->>API: authenticatedFetch('/status')
API->>API: Prepend BASE_URL -> `${BASE_URL}/status`
API->>Server: HTTP request to `${BASE_URL}/status`
App->>WS: useWebSocket()
WS->>Server: Connect to `${BASE_URL}/ws` (with token if required)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 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: 0
🧹 Nitpick comments (4)
server/index.js (3)
166-178: Base-path aware app/router setup is solid; consider central base-path normalizationSplitting
expressAppfrom therouterand mounting the router atAPP_BASE_PATHwith shared middleware onexpressAppis a good way to support sub-directory deployments. UsingexpressApp.locals.wssis also the correct choice soreq.app.locals.wsscontinues to work from within routes.You compute a trimmed base path (
APP_BASE_PATH.endsWith('/') ? ...) in several places (manifest, HTML, WebSocket, SPA index). To avoid drift and edge cases (e.g. values without a leading slash, or accidental//), consider a small helper like:function normalizeBasePath(raw = '/') { if (!raw) return ''; let p = raw.trim(); if (!p.startsWith('/')) p = `/${p}`; if (p !== '/' && p.endsWith('/')) p = p.slice(0, -1); return p === '/' ? '' : p; }and then reuse
const basePath = normalizeBasePath(APP_BASE_PATH);everywhere, including theexpressApp.use(APP_BASE_PATH, router)call (which could then use the normalized form).Also applies to: 221-225, 1531-1533
274-292: HTML/manifest base-path injection works; tighten script injection and index.html handlingThe manifest and HTML rewriting plus the SPA catch-all give you consistent base-path behavior in production. A couple of small robustness tweaks:
- Escape
APP_BASE_PATHsafely in the injected scriptInstead of interpolating directly into JavaScript:
const configScript = `<script>window.__APP_BASE_PATH__='${basePath}';</script>`;prefer a JSON-safe form:
const configScript = `<script>window.__APP_BASE_PATH__=${JSON.stringify(basePath)};</script>`;This avoids breakage if
APP_BASE_PATHever contains quotes or other special characters.
- Ensure
/index.htmlalso benefits from injectionRight now the SPA handler is on
app.get('*')and reads../dist/index.html, but/index.htmlunder the base path can still be served directly byexpress.static('../dist')without your rewrites. If users or tooling hit/your-base/index.htmldirectly, asset URLs may not be base-prefixed.You can avoid that by, for example:
- Adding an explicit
app.get('/index.html', ...)handler (inside the router) that delegates to the same SPA logic, before theexpress.static('../dist', ...)middleware; or- Setting
index: falseon thepublicstatic as well and relying solely on the SPA handler for all HTML.Also applies to: 295-319, 1488-1528
754-762: WebSocket path routing matches APP_BASE_PATH; optionally normalize trailing slashesDeriving
shellPathandchatPathfromAPP_BASE_PATHso they become/<base>/shelland/<base>/wsis consistent with the rest of the base-path design and should work as long as the reverse proxy preserves the prefix.As a small hardening improvement, you could normalize
pathnameto strip trailing slashes before comparison so clients connecting to/ws/or/shell/still work:const urlObj = new URL(url, 'http://localhost'); let pathname = urlObj.pathname.replace(/\/+$/, '') || '/';and then compare against normalized
shellPath/chatPath..env.example (1)
43-51: APP_BASE_PATH documentation is clear; consider clarifying normalization rulesThe deployment section makes it obvious how to set
APP_BASE_PATHfor root vs sub-directory installs. To reduce misconfiguration, you might add a brief note that:
- The value should start with
/(e.g./claudeui, notclaudeui), and- A trailing slash is optional and normalized internally.
This would align expectations with the server’s base-path handling.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.env.example(1 hunks)server/index.js(5 hunks)src/App.jsx(2 hunks)src/components/ClaudeLogo.jsx(1 hunks)src/components/CursorLogo.jsx(1 hunks)src/utils/api.js(4 hunks)src/utils/websocket.js(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
src/App.jsx (1)
src/utils/api.js (2)
ROUTER_BASENAME(7-7)ROUTER_BASENAME(7-7)
src/utils/api.js (3)
server/index.js (3)
token(203-204)url(202-202)url(747-747)src/contexts/AuthContext.jsx (1)
token(27-27)server/middleware/auth.js (1)
token(40-40)
src/utils/websocket.js (2)
src/utils/api.js (3)
BASE_URL(5-5)BASE_URL(5-5)token(11-11)server/index.js (1)
token(203-204)
src/components/ClaudeLogo.jsx (1)
src/utils/api.js (2)
BASE_URL(5-5)BASE_URL(5-5)
src/components/CursorLogo.jsx (1)
src/utils/api.js (2)
BASE_URL(5-5)BASE_URL(5-5)
server/index.js (2)
server/routes/user.js (1)
router(9-9)server/routes/agent.js (1)
router(13-13)
🔇 Additional comments (11)
src/App.jsx (1)
39-39: Router basename wiring correctly ties client routing to the base pathUsing
ROUTER_BASENAMEfor theBrowserRouterbasename, derived fromwindow.__APP_BASE_PATH__, is the right way to keep client-side URLs aligned withAPP_BASE_PATHwhile still allowing simplenavigate('/session/:id')calls.Since
ROUTER_BASENAMEisBASE_URL || '/', please verify:
- Deep links to
/session/:idwork both when deploying at root and under a sub-path.- In development (no
__APP_BASE_PATH__injection),basename="/"behaves as expected with your existing routes.Also applies to: 955-960
src/components/ClaudeLogo.jsx (1)
1-5: ClaudeLogo now respects BASE_URL for sub-path deploymentsUsing
${BASE_URL}/icons/claude-ai-icon.svgensures the icon resolves correctly both at the root and under a configured base path, without changing the component’s public API.src/components/CursorLogo.jsx (1)
1-5: CursorLogo BASE_URL usage is consistent and base-path awareMirroring ClaudeLogo, building the src as
${BASE_URL}/icons/cursor.svgkeeps the cursor icon working under both root and sub-directory deployments without altering the component’s interface.src/utils/websocket.js (3)
2-2: LGTM: BASE_URL import is correct.The import properly retrieves the BASE_URL constant for constructing base-path-aware WebSocket URLs.
30-34: LGTM: Platform mode WebSocket URL correctly incorporates BASE_URL.The WebSocket URL construction properly prepends the base path while maintaining protocol detection and proxy compatibility.
36-44: LGTM: OSS mode WebSocket connection properly handles BASE_URL and authentication.The implementation correctly:
- Guards against missing tokens with early return
- Incorporates BASE_URL into the WebSocket path
- URL-encodes the token parameter
- Maintains consistency with server-side token extraction logic
src/utils/api.js (5)
3-7: LGTM: BASE_URL and ROUTER_BASENAME exports are well-implemented.The implementation correctly:
- Reads the server-injected base path from window global
- Provides appropriate fallbacks for root path deployment
- Exports both variants for different use cases (API calls vs routing)
17-20: LGTM: FormData Content-Type handling is correct.Properly removes the Content-Type header for FormData requests to allow the browser to set the correct multipart boundary automatically.
22-29: LGTM: Header merging and Authorization logic is well-structured.The implementation correctly:
- Merges default headers with provided headers
- Adds Authorization header after merge, ensuring token-based auth takes precedence
- Only applies Authorization in OSS mode when token exists
37-40: LGTM: Fetch call correctly uses computed URL and headers.The fetch invocation properly applies the base-path-aware URL and merged headers object.
31-35: BASE_URL trailing slash concern is already addressed.The server-side code normalizes
APP_BASE_PATHbefore injecting it aswindow.__APP_BASE_PATH__(server/index.js:1504–1509). The normalization removes any trailing slash, ensuring thatBASE_URLon the client never contains a trailing slash. Therefore, the URL construction in lines 31–35 is safe and will not produce double slashes. The suggested client-side normalization in the review comment is unnecessary.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/Shell.jsx (1)
61-74:skipAuthis undefined here and will break non‑platform WebSocket connectionsIn the non‑platform branch, this line:
const queryParams = (token && !skipAuth) ? `?token=${encodeURIComponent(token)}` : '';references
skipAuth, which is not defined anywhere in this file or in the surrounding scope. At runtime this will throw aReferenceErrorthe first time the non‑platform path is executed, preventing the shell from connecting.Unless
skipAuthis intentionally provided as a global elsewhere (which is brittle), this should either be removed from the condition or properly sourced (e.g., from props/env or an import).If the intent is simply “append token if present”, a minimal safe fix is:
- const queryParams = (token && !skipAuth) ? `?token=${encodeURIComponent(token)}` : ''; - wsUrl = `${protocol}//${window.location.host}${BASE_URL}/shell${queryParams}`; + const queryParams = token ? `?token=${encodeURIComponent(token)}` : ''; + wsUrl = `${protocol}//${window.location.host}${BASE_URL}/shell${queryParams}`;This keeps behavior compatible with the server’s
tokenhandling (tokenfrom query params orAuthorizationheader) while avoiding the undefined variable. Based on relevant code snippets, this matches how the server currently retrieves the token.
🧹 Nitpick comments (2)
src/components/Shell.jsx (2)
63-63: Using BASE_URL in the platform WebSocket URL fits the base‑path designThe platform branch now builds:
wsUrl = `${protocol}//${window.location.host}${BASE_URL}/shell`;which makes the shell endpoint respect the configured base path, consistent with the rest of the app’s routing. This aligns with the PR goal of sub‑directory deployments.
One small future‑proofing idea (optional): if
APP_BASE_PATHcan be configured with or without a trailing slash, consider normalizingBASE_URLonce insrc/utils/api.jsto avoid accidental double slashes in URLs.
302-302: Silent clipboard error handling is acceptable but could log in devSwallowing errors from
navigator.clipboard.readText()is reasonable to avoid noisy UX when permissions are denied, but the completely emptycatchmakes debugging harder if something else goes wrong.Optionally, you could log only in development, e.g.:
- }).catch(() => { }); + }).catch((err) => { + if (import.meta.env.DEV) { + console.warn('[Shell] Clipboard paste failed:', err); + } + });This keeps production quiet while still giving you visibility during development.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/Shell.jsx(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/components/Shell.jsx (4)
src/utils/api.js (3)
BASE_URL(5-5)BASE_URL(5-5)token(11-11)server/index.js (1)
token(203-204)src/contexts/AuthContext.jsx (1)
token(27-27)server/middleware/auth.js (1)
token(40-40)
🔇 Additional comments (1)
src/components/Shell.jsx (1)
8-8: BASE_URL import for WebSocket path looks appropriateImporting
BASE_URLhere keeps the shell WebSocket endpoint aligned with the app’s configured base path; this is consistent with howBASE_URLis exposed fromsrc/utils/api.js. No changes needed.
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.