fix #629 add vite base support#630
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ee7a99d3ca
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for configurable base paths and centralizes API and Socket.io handling. Key changes include the addition of an app-path utility for dynamic path resolution, an apiFetch wrapper in the query client to handle authorization and base path prefixing consistently, and a centralized socket singleton. Feedback focuses on improving the robustness of the base path resolution logic, reducing duplication of route definitions to ease maintenance, avoiding mutation of Headers objects in the authorization helper, and ensuring the Vite base configuration follows recommended formatting for asset resolution.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Doezer
force-pushed
the
feat/629/add-vite-base-support
branch
from
ee7a99d to
6f1dec0
Compare
…DB state - app-path.test.ts: cover all branches of normalizeBasePath, resolveBasePathFrom (dev/prod asset URLs, route matching, unmatched fallback, catch blocks) and withBasePathFrom - queryClient.test.ts: cover all withAuthorization branches including Headers mutation safety, array headers with/without existing auth, and plain-object headers with/without existing auth - AddGameModal.test.tsx: add IGDB-unconfigured state test; generalise setupFetch to accept an igdbConfigured flag Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Wire default getQueryFn into createTestQueryClient so queries that rely on the global default (e.g. userGames) actually fire in tests - Add tests covering: debounce timer (line 49), IGDB search with auth token (line 79), failed IGDB search (line 85), add-game mutation success path (lines 100-118, 135-136, 286), add-game mutation failure path (lines 110-112, 121-124), In Collection badge via userGames lookup (line 142 some() callback), Go to Settings button onClick (line 164), and form submit handler (line 129) - Promote toast mock to shared module-level spy (mockToast) so mutation callbacks can be asserted on - Extend setupFetch to distinguish GET vs POST /api/games and accept userGames + postGame response params Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
/deploy tag=vite-base-support |
|
|
@Doezer Had to do a small adjustment to get it to work. Here are CLAUDEs findings and what had to be changed: Build fails with non-default QUESTARR_BASE_PATH — Failed to resolve …src/main.tsx from index.html Trying this PR out on a deployment that serves Questarr under a subpath — the runtime side of the PR works correctly, but the production build itself fails before any assets are emitted. Repro QUESTARR_BASE_PATH=/my-subpath/ npm run build Root cause vite.config.ts normalizes QUESTARR_BASE_PATH=/my-subpath/ to base = "/my-subpath/". During HTML transform, Vite substitutes %BASE_URL% into the literal src string, producing: <script type="module" src="/my-subpath/src/main.tsx"></script>Vite's vite:build-html plugin then tries to resolve that as a source module to bundle. It does not strip the configured base off the src when resolving — it treats /my-subpath/src/main.tsx as an absolute path relative to the project root (client/), where no such file exists, and aborts. The %BASE_URL% substitution is intended for runtime public-asset URLs (links, images, etc.) — assets the browser fetches verbatim, where prepending the base is exactly what you want. For a <script type="module"> whose src points at a source file Vite needs to bundle, the prefix actively breaks build-time resolution. This is why the other %BASE_URL% usages in the same file (%BASE_URL%Questarr.svg, %BASE_URL%Questarr_icon.png) work fine — they're runtime fetches, never resolved as source modules at build time. Why the existing tests don't catch it Fix --- a/client/index.html
The %BASE_URL% references on the lines should stay — those are runtime asset URLs and benefit from the base prefix. Verification after the fix ✓ built in 1.57sBuilt dist/public/index.html correctly references /my-subpath/assets/index-.js. With this single-line patch on top of the PR, Questarr loads and operates cleanly behind a reverse proxy mounted at /my-subpath/: "GET /my-subpath/ HTTP/2.0" 200 |
2 participants
This pull request introduces robust support for deploying the app under a configurable base path, making the frontend work seamlessly in subdirectories or behind reverse proxies. It does so by centralizing base path logic in new helpers, updating routing and asset references, and ensuring all API and socket connections use the correct path. Additionally, it replaces direct use of the Socket.IO client with a shared singleton to avoid redundant connections and streamline event handling.
Base path support and routing:
app-path.tswith helpers to normalize, resolve, and prefix URLs with the app's base path, and updated all static asset and API references to use these helpers. This ensures assets, API requests, and navigation work correctly regardless of deployment location. [1] [2] [3] [4] [5] [6]App.tsxto use Wouter'sbaseprop, so client-side navigation is also base path aware. [1] [2] [3] [4]API and authentication requests:
apiFetchfunction that automatically applies the base path and authorization headers; refactored all fetch calls and the query client to use this. [1] [2] [3] [4] [5] [6] [7] [8]Socket.IO connection management:
getSockethelper that provides a shared singleton connection, using the correct socket path for the current base path. All Socket.IO usage now goes through this singleton, preventing redundant connections and ensuring consistent event handling. [1] [2] [3] [4] [5] [6] [7] [8]Testing: