Conversation
- Changed project setup from Create React App to Vite for improved performance. - Updated dependencies including @types/node and added new devDependencies for Vite and SWC. - Refactored test files to mock window.location and localStorage for better isolation. - Introduced a redirect function in LocalLogin component to handle navigation. - Updated WebSocketProvider to construct WebSocket URLs dynamically based on the environment. - Enhanced UrlInput component to clear input conditionally based on validation results. - Added new test cases to ensure proper functionality of the refactored components.
…ontrol, prior to refactor around cache-control
…point calls with upstream-dev
…ests and refactor components to use it
…M and external db flows
There was a problem hiding this comment.
Pull request overview
This pull request migrates the frontend build system from Create React App (react-scripts) to Vite, along with updates to Node.js tooling and Jest. The migration aims to provide faster development with Hot Module Reload (HMR) and modern build tooling.
Changes:
- Migration from react-scripts to Vite for frontend build and development server
- Introduction of docker-compose.dev.yml for dedicated development environment with port 3011
- Implementation of locationUtils abstraction layer for testable window.location usage
- Addition of cache control middleware for optimized static asset caching with Vite's hashed filenames
- Update to Jest 30 with SWC transformer, new jest.setup.ts, and removal of direct window.location mocking
- Documentation updates describing dual workflow: Docker with static build vs Vite dev server with HMR
Reviewed changes
Copilot reviewed 33 out of 38 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| client/vite.config.ts | New Vite configuration with proxy rules for API/WebSocket and dev server setup |
| client/index.html | New Vite-specific HTML entry point |
| client/package.json | Removed react-scripts, added Vite and SWC dependencies, updated scripts |
| client/jest.config.cjs | New Jest configuration using SWC transformer and jsdom environment |
| client/jest.setup.ts | New test setup with location mocking helpers and polyfills |
| client/src/utils/location.ts | New utility for testable window.location access |
| client/src/providers/WebSocketProvider.tsx | Updated to use locationUtils and unified /ws path for WebSocket connections |
| client/src/App.tsx | Added axios interceptor to strip conditional headers in dev mode, uses locationUtils |
| client/src/components/*/tsx | Updated components to use locationUtils instead of direct window.location access |
| docker-compose.dev.yml | New development compose file with port 3011 mapping and Node --watch mode |
| scripts/*.sh | Updated start scripts to support dev compose file selection |
| server/server.js | Added cache control middleware for Vite's hashed assets |
| docs/DEVELOPMENT.md | Updated with Vite dev server workflow instructions |
Comments suppressed due to low confidence (1)
docs/DEVELOPMENT.md:228
- The documentation instructs users to run
./stop.shto stop the dev environment (lines 208 and 227), but stop.sh doesn't handle docker-compose.dev.yml. Users should instead rundocker compose -f docker-compose.dev.yml downto properly stop dev containers, or the stop.sh script needs to be updated to handle the dev compose file.
Consider either:
- Updating the documentation to use the correct stop command for dev:
docker compose -f docker-compose.dev.yml down - Creating a dedicated
./scripts/stop-dev.shscript - Noting this limitation in the documentation
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Updated TypeScript version in client/package.json and package.json to 5.9.3. - Deleted unused index.html file from client/public. - Removed test-location.mjs as it was no longer needed. - Adjusted Vite configuration by removing the rewrite rule for WebSocket paths.
dialmaster
left a comment
dialmaster
left a comment
There was a problem hiding this comment.
Previously when running a dev instance via ./scripts/start-dev.sh the app was accessible via the browser at http://localhost:3087/ and with these changes it now is at http://localhost:3011/
I'd prefer if we keep things fully functionally the same, this confused me a bit since I have a local dev reverse proxy setup via cloudflared and when I ran your branch it didn't work.
This should fix that issue:
--- a/docker-compose.dev.yml
+++ b/docker-compose.dev.yml
@@ -20,6 +20,7 @@ services:
# Map 3011:3011 in dev so Vite proxy can reach backend at localhost:3011
ports:
- "3011:3011"
+ - "3087:3011"
And another small thing:
I was running an older Node locally, and your Vite changes change the Node requirement for local dev, I saw this:
You are using Node.js 20.11.1. Vite requires Node.js version 20.19+ or 22.12+. Please upgrade your Node.js version.
I think we just need to update docs/DEVELOPMENT.md to change Node.js 18+ to Node.js 20.19+
No issue in production builds since FROM node:20-slim AS base pulls in v20.20.0
… documentation for Node version.
…v to use this same endpoint. Updated documentation to reflect.
Vite migration and bump of node/jest.
Had to modify the dev build a bit with its own compose.
Let me know your thoughts.