Reorganizes codebase to be more in line with SOPs using Vite#77
Reorganizes codebase to be more in line with SOPs using Vite#77ISCOzmurph wants to merge 2 commits intoPegaProx:mainfrom
Conversation
- Added support for serving a Vite-built single-page application (SPA) from the `web/dist/` directory. - Updated the main index route to prefer the Vite build output if available, falling back to the legacy `web/index.html` if not. - Introduced a new route for serving Vite-built assets. - Updated README to reflect the new build process and directory structure. - Deprecated the old build script in favor of the new npm-based build process.
There was a problem hiding this comment.
Pull request overview
This PR introduces a Vite-based React frontend build pipeline and updates the Flask backend to serve the built SPA from web/dist/ (with a fallback to a legacy entrypoint when the build output is missing).
Changes:
- Add Vite + Tailwind/PostCSS configuration and npm scripts for building the frontend into
web/dist/. - Update Flask routes to serve
web/dist/index.htmlandweb/dist/assets/*when present. - Update documentation and metadata (
README.md,version.json) to reflect the new build/deploy approach.
Reviewed changes
Copilot reviewed 12 out of 16 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| web/vite.config.js | Adds Vite config (React plugin, build output settings, dev proxy). |
| web/tailwind.config.js | Adds Tailwind theme/content configuration for the Vite app. |
| web/postcss.config.js | Adds PostCSS config for Tailwind + Autoprefixer. |
| web/package.json | Defines npm scripts and frontend dependencies for Vite/React build. |
| web/package-lock.json | Locks npm dependency tree for reproducible installs. |
| web/src/main.jsx | Adds React entrypoint mounting the app. |
| web/src/index.css | Adds Tailwind directives and global styles. |
| web/src/constants.js | Adds frontend constants (API URL, version, debug flag). |
| web/src/README.md | Documents the frontend source layout and build commands. |
| pegaprox_multi_cluster.py | Serves SPA index from web/dist/ when available and adds /assets/* route. |
| version.json | Updates update metadata and changelog to reflect the Vite build. |
| web/Dev/build.sh | Marks the legacy build script as deprecated in favor of Vite. |
| README.md | Documents the new frontend build and the web/dist/ output. |
| .gitignore | Ignores frontend build output and runtime-generated directories. |
Files not reviewed (1)
- web/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "web/dist/assets/", | ||
| "web/package.json", |
There was a problem hiding this comment.
update_files removed web/index.html and does not include web/package-lock.json, but the docs instruct npm ci and the backend falls back to web/index.html when web/dist/index.html is missing. This can leave installs/updaters without a working frontend unless dist/ is already built. Consider adding web/index.html (for fallback) and web/package-lock.json (for npm ci) to the update list, or adjust the docs/update mechanism accordingly.
| "web/dist/assets/", | |
| "web/package.json", | |
| "web/dist/assets/", | |
| "web/index.html", | |
| "web/package.json", | |
| "web/package-lock.json", |
| if not os.path.isdir(assets_dir): | ||
| return Response('', status=404, mimetype='application/javascript') |
There was a problem hiding this comment.
If web/dist/assets is missing, this returns a 404 with a hardcoded application/javascript MIME type. Since Vite assets include CSS (and potentially other types), this can trigger misleading browser MIME errors. Consider choosing the MIME type from the requested extension (similar to serve_static) or just abort(404).
| assets_dir = os.path.join(SPA_DIST, 'assets') | ||
| if not os.path.isdir(assets_dir): | ||
| return Response('', status=404, mimetype='application/javascript') | ||
| return send_from_directory(assets_dir, filename) |
There was a problem hiding this comment.
serve_spa_assets does not check whether the requested asset file exists before calling send_from_directory(). For missing hashed assets, Flask will return its default HTML 404 page, which can cause “Refused to apply style/script due to MIME type” in the browser. Consider adding an os.path.isfile() check and returning an empty 404 with the correct content-type (similar to serve_static).
| loadingScreen.style.opacity = '0'; | ||
| loadingScreen.style.transition = 'opacity 0.3s ease'; | ||
| setTimeout(() => loadingScreen.remove(), 300); |
There was a problem hiding this comment.
The fadeout sets opacity before setting the transition, which usually prevents the animation from running. Set transition first and then change opacity on the next frame (or toggle a CSS class) so the fade reliably animates.
| loadingScreen.style.opacity = '0'; | |
| loadingScreen.style.transition = 'opacity 0.3s ease'; | |
| setTimeout(() => loadingScreen.remove(), 300); | |
| loadingScreen.style.transition = 'opacity 0.3s ease'; | |
| window.requestAnimationFrame(() => { | |
| loadingScreen.style.opacity = '0'; | |
| setTimeout(() => loadingScreen.remove(), 300); | |
| }); |
| "xterm": "^5.3.0", | ||
| "xterm-addon-fit": "^0.8.0" |
There was a problem hiding this comment.
xterm and xterm-addon-fit are published as deprecated packages (see warnings in the lockfile). If feasible, consider migrating to the maintained scoped packages (@xterm/xterm, @xterm/addon-fit) to avoid relying on deprecated dependencies.
| "xterm": "^5.3.0", | |
| "xterm-addon-fit": "^0.8.0" | |
| "@xterm/xterm": "^5.3.0", | |
| "@xterm/addon-fit": "^0.8.0" |
| if os.path.isfile(os.path.join(SPA_DIST, 'index.html')): | ||
| return send_from_directory(SPA_DIST, 'index.html') | ||
| return send_from_directory(WEB_DIR, 'index.html') |
There was a problem hiding this comment.
The fallback path in _serve_spa_index() serves web/index.html when web/dist/index.html is absent, but web/index.html currently bootstraps via /src/main.jsx (Vite dev entry) and the Flask app doesn’t expose a /src/* route. That means the fallback will render a blank page in production if dist/ hasn’t been built. Either point the fallback at an actually self-contained legacy file (e.g., the precompiled single-file UI) or add a dev-only route to serve web/src/* when using this fallback.
| "name": "pegaprox-web", | ||
| "version": "0.6.6", | ||
| "dependencies": { | ||
| "@novnc/novnc": "^1.4.0", |
There was a problem hiding this comment.
package-lock.json lists @novnc/novnc as a direct dependency, but package.json does not. This will cause npm ci to fail with a lockfile/package.json mismatch. Either add @novnc/novnc to dependencies or regenerate the lockfile from the intended package.json (preferred if you’re only loading noVNC via CDN/static files).
| "@novnc/novnc": "^1.4.0", |
| "web/dist/assets/", | ||
| "web/package.json", | ||
| "web/src/", |
There was a problem hiding this comment.
version.json update_files now includes directory entries (e.g. web/dist/assets/, web/src/). The in-app updater downloads each entry via requests.get(...).text and writes it as a single file, so directory entries will be skipped/404 and won’t update the frontend correctly. Replace directory entries with concrete file paths, or change the updater to support recursive/binary downloads (e.g., download and extract a release artifact).
| "web/dist/assets/", | |
| "web/package.json", | |
| "web/src/", | |
| "web/package.json", |
|
Copilot is flagging a lot of things that are pre-existing as I only did a lift-and-shift. One of the great advantages of this reorganization is these issues are surfaced. |
- Added "web/index.html" and "web/package-lock.json" to the list of tracked assets in version.json to ensure they are included in the build process.
web/dist/directory.web/index.htmlif not.