fix: UI URL Basename#251
Conversation
Signed-off-by: nmattela <nicolas.bruno.mattelaer@vub.be>
✅ Deploy Preview for opencost-ui ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Pull request overview
Fixes serving the OpenCost UI under a non-root base path by aligning Vite’s build-time base URL and React Router’s basename with a single VITE_BASENAME/vite_basename setting.
Changes:
- Replace legacy basename env handling with
VITE_BASENAMEin Vite + React Router configs. - Update hard-coded
/logo.pngreferences to incorporate the configured base path. - Replace Docker build arg
ui_pathwithvite_basenameand update README accordingly.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
vite.config.ts |
Sets Vite base from VITE_BASENAME for asset URL generation under a base path. |
react-router.config.ts |
Sets React Router basename from VITE_BASENAME for correct routing under a base path. |
package.json |
Updates legacy build script to use VITE_BASENAME. |
app/components/legacy/Nav/SidebarNav.jsx |
Updates legacy sidebar logo path to incorporate basename. |
app/components/app-header.tsx |
Updates header logo path to incorporate basename. |
Dockerfile |
Introduces vite_basename build arg and wires it into build/runtime env and UI path. |
README.md |
Documents vite_basename build arg instead of ui_path. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ENV API_PORT=9003 | ||
| ENV API_SERVER=0.0.0.0 | ||
| ENV UI_PORT=9090 | ||
| ENV UI_PATH=${ui_path} | ||
| ENV UI_PATH=${vite_basename} | ||
|
|
There was a problem hiding this comment.
UI_PATH is set directly from vite_basename, but the nginx template concatenates ${UI_PATH}index.html for the SPA fallback. If the build arg is passed as /anything (as in the README), this becomes /anythingindex.html and deep links will 404. Either normalize UI_PATH to always end with / (e.g., strip trailing slashes then append one) or adjust the nginx template to insert the missing /.
There was a problem hiding this comment.
While it is true that the nginx template has the following in it:
location @ui_spa_fallback {
rewrite ^ ${UI_PATH}index.html last;
}
This block does not appeared in the rendered version of the template. So I'm not so sure what's going on. The reason why the README does not state that it should be normalized is because I simply took over what was written in the README before.
| To serve the web interface under a path other than the root (`/`), you need to build a custom image using the `vite_basename` build argument. | ||
| For example, you can clone this project and run: | ||
|
|
||
| ```sh | ||
| $ docker build --build-arg ui_path=/anything --tag opencost-ui:latest . | ||
| $ docker build --build-arg vite_basename=/anything --tag opencost-ui:latest . | ||
| ``` | ||
|
|
||
| This ensures that all static assets are served from the specified path. | ||
|
|
||
| Once the container is running, the UI will be accessible at `<domain>/{ui_path}`. | ||
| Once the container is running, the UI will be accessible at `<domain>/{vite_basename}`. |
There was a problem hiding this comment.
The docs show --build-arg vite_basename=/anything, but the nginx config logic requires UI_PATH to be normalized (at least for SPA fallback rewriting). Once UI_PATH/vite_basename handling is fixed, please clarify in this section whether vite_basename should be provided with/without a trailing slash so users don’t end up with broken deep links or double-slash asset paths.
There was a problem hiding this comment.
idem as response above
thomasvn
left a comment
thomasvn
left a comment
There was a problem hiding this comment.
Approving! I see a successful Netlify deploy in CI.
What does this PR change?
I noticed some problems with running opencost under a base path. The index.html would load, but it would try to load all assets under
/assets, and not/my_base_path/assets. This PR fixes thatDoes this PR relate to any other PRs?
No
How will this PR impact users?
The build argument
ui_pathhas been replaced byvite_basename, as the UI path you set up in the nginx conf should be in sync with the one set in Vite during build.Does this PR address any GitHub or Zendesk issues?
No
How was this PR tested?
Manual testing
Does this PR require changes to documentation?
The README has been edited to reflect the above mentioned deprecation of ui_path
Have you labeled this PR and its corresponding Issue as "next release" if it should be part of the next OpenCost release? If not, why not?
No opinion on this matter