fix: detect router basename for path-prefixed deployments#465
fix: detect router basename for path-prefixed deployments#465JohnGenri wants to merge 3 commits intositeboon:mainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a module-level helper Changes
Sequence Diagram(s)sequenceDiagram
participant App as App Component
participant Doc as document (DOM)
participant Window as window/global
participant Router as React Router
App->>Window: call detectRouterBasename()
detectRouterBasename->>Window: check window.__ROUTER_BASENAME__
alt override present
detectRouterBasename-->>App: return trimmed override
else override absent
detectRouterBasename->>Doc: read manifest href, module script src, icon hrefs
Doc-->>detectRouterBasename: list of candidate URLs
detectRouterBasename->>detectRouterBasename: normalize, pick longest valid path
detectRouterBasename-->>App: return inferred basename (or empty)
end
App->>Router: mount with basename (routerBasename)
Router-->>App: routes mounted using computed basename
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)
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: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/App.tsx`:
- Line 30: The URL resolution is using window.location.origin which mis-resolves
relative hints; update the code that sets pathname (the expression new
URL(candidate, window.location.origin).pathname) to resolve against
document.baseURI instead (new URL(candidate, document.baseURI).pathname) so
relative paths and deployed prefixes are preserved; modify the line that
computes pathname in App.tsx accordingly.
- Around line 22-34: The loop over candidatePaths currently returns on the first
successful match which can pick a shorter or empty basename; instead iterate all
candidatePaths, collect each normalized prefix from the regex match (use the
same regex and normalization: match[1] ? match[1].replace(/\/+$/, '') : ''),
skip empty prefixes when preferring non-empty, then after the loop return the
longest non-empty normalized prefix (or '' if none); update the code around
candidatePaths, pathname, match to compute and return that strongest detected
prefix rather than returning immediately on first match.
|
Validation note from the environment where this was reproduced:
So the fix is not theoretical; it resolves the concrete path-prefixed deployment failure that triggered issue #464. |
|
Addressed both CodeRabbit findings in a follow-up commit:
This makes basename detection more robust for path-prefixed deployments where some hints may resolve to root while others preserve the actual prefix. |
JohnGenri
force-pushed
the
fix/router-basename-prefix
branch
from
f2b6c35 to
3950c0e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/App.tsx`:
- Around line 21-31: The current basename detection in App.tsx (iterating
candidatePaths and using pathname.match) only recognizes favicon.svg/png; update
the regex used in the match (the expression assigned to match in the loop) to
also accept favicon with any extension and other icon filenames so the prefix is
detected for favicon.ico or other icon names (e.g. change the part
`favicon\.(?:svg|png)$` to something like `favicon(?:\.[^\/]+)?$` or otherwise
include a broader `icon`/favicon filename pattern) so detectedBasename is
correctly set for more icon filenames.
|
Validation note after refreshing this branch onto current
So this is still an active bug, and the PR remains intentionally scoped to the client-side router basename detection only. |
|
Addressed the remaining CodeRabbit edge case in
The branch remains intentionally scoped to runtime basename detection for issue |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/App.tsx (1)
40-41:⚠️ Potential issue | 🟠 MajorIgnore cross-origin asset hints before deriving the basename.
At Line 40, any absolute manifest/script/icon URL can influence the detected basename. If those assets are hosted on a CDN, this will infer the CDN path instead of the app’s path prefix and break routing on the current origin.
Proposed fix
- const pathname = new URL(candidate.value, document.baseURI || window.location.href).pathname; + const candidateUrl = new URL(candidate.value, document.baseURI || window.location.href); + if (candidateUrl.origin !== window.location.origin) { + continue; + } + const pathname = candidateUrl.pathname;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/App.tsx` around lines 40 - 41, The code is deriving a basename from candidate.value regardless of origin, causing CDN/absolute asset URLs to leak into the app basename; before using new URL(candidate.value, document.baseURI || window.location.href).pathname, construct the URL and compare url.origin to window.location.origin (or new URL(document.baseURI || window.location.href).origin), and only use url.pathname (then normalize via pathname.replace(/\/+$/, '')) if the origins match; if they differ, skip this candidate (or fall back to using window.location.pathname) so CDN/third‑party absolute URLs do not influence basename detection.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/App.tsx`:
- Around line 54-57: The current cleanup loop that strips trailing folder names
from match[1] (using segments = match[1].split('/').filter(Boolean) and the
while loop checking segments.pop()) can remove the only segment (e.g., "/icons")
and collapse the prefix to root; change the loop condition to avoid popping when
there's only one segment left (e.g., while (segments.length > 1 &&
['assets','static','icons','images'].includes(segments[segments.length - 1])))
so the basename is preserved for single-segment mounts; update any logic that
builds the final prefix from segments to rely on this adjusted condition.
---
Duplicate comments:
In `@src/App.tsx`:
- Around line 40-41: The code is deriving a basename from candidate.value
regardless of origin, causing CDN/absolute asset URLs to leak into the app
basename; before using new URL(candidate.value, document.baseURI ||
window.location.href).pathname, construct the URL and compare url.origin to
window.location.origin (or new URL(document.baseURI ||
window.location.href).origin), and only use url.pathname (then normalize via
pathname.replace(/\/+$/, '')) if the origins match; if they differ, skip this
candidate (or fall back to using window.location.pathname) so CDN/third‑party
absolute URLs do not influence basename detection.
1 participant
Summary
Fix blank-page routing failures when CloudCLI UI is deployed behind a reverse proxy under a path prefix such as
/ai.Closes #464.
Problem
src/App.tsxcurrently useswindow.__ROUTER_BASENAME__ || '', but that global is not initialized by the app.As a result, path-prefixed deployments can successfully rewrite HTML, assets, and API calls, while React Router still assumes it is mounted at
/.That breaks routed views after login and direct links like:
/ai/session/:sessionIdSolution
Keep the explicit
window.__ROUTER_BASENAME__override when present, but otherwise derive the basename from runtime URLs that already reflect the deployed prefix:This makes prefixed deployments work without requiring an additional server-side bootstrap script.
Validation
Built locally with Vite after the change and verified that the generated app bundle was updated.
Summary by CodeRabbit