Update docs#86
Conversation
There was a problem hiding this comment.
Pull request overview
Updates project documentation to reflect the frontend migration from Create React App conventions to Vite + Jest, aligning dev URLs and test commands with the current toolchain.
Changes:
- Update frontend dev server URLs from
localhost:3000to Vite’s defaultlocalhost:5173. - Update frontend config/test documentation to use Vite/Jest conventions (e.g.,
import.meta.env, simplified coverage commands). - Update README tech stack to React 19 + Vite.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| docs/SETUP.md | Updates dev server URL, frontend config snippet, and Jest command examples. |
| docs/DEVELOPMENT.md | Simplifies frontend coverage command to npm test. |
| README.md | Updates frontend tech stack line to React 19 + Vite. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Edit `frontend/src/constant.ts`: | ||
|
|
||
| ```typescript | ||
| export const API_BASE_URL = process.env.REACT_APP_API_URL || 'http://localhost:8080'; | ||
| export const CLASSICAL_API = `${API_BASE_URL}/api/classical`; | ||
| export const MODAL_API = `${API_BASE_URL}/api/modal`; | ||
| export const BASE_URL: string = import.meta.env.VITE_API_URL || 'http://localhost:8080/'; | ||
| export const LOGIC: string = 'classical'; |
There was a problem hiding this comment.
The “API Endpoint Configuration” snippet suggests using import.meta.env.VITE_API_URL via BASE_URL, but the current frontend code does not read any VITE_API_URL value and BASE_URL is not referenced anywhere (API calls in frontend/src/service/actions.ts use relative fetch('/logic/...')). This makes the setup instructions misleading. Either update the docs to describe the actual mechanism (Vite dev-server proxy / same-origin paths), or adjust the app to consistently use BASE_URL for API requests (and document where to set VITE_API_URL, e.g. .env).
| ``` | ||
|
|
||
| Access at: http://localhost:3000 (frontend will proxy API calls to backend) | ||
| Access at: http://localhost:5173 (frontend will proxy API calls to backend) |
There was a problem hiding this comment.
This line states that the frontend dev server will proxy API calls to the backend, but frontend/vite.config.ts currently has no server.proxy configuration and frontend requests are relative (e.g. fetch('/logic/...')). As written, running the frontend on :5173 will try to hit :5173 for API endpoints and fail. Please either add Vite proxy setup instructions here (or update the wording to reflect the actual networking/CORS approach).
| ```powershell | ||
| npm test -- --coverage | ||
| npm test | ||
| ``` |
There was a problem hiding this comment.
In the Frontend Tests section, “Run all tests” and “Run with coverage” now show the exact same npm test command. To avoid redundancy/confusion, consider collapsing these into a single instruction (and note that the npm test script includes coverage by default).
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
* Initial plan * Collapse redundant frontend test commands into single instruction Co-authored-by: dan323 <32780500+dan323@users.noreply.github.com> Agent-Logs-Url: https://github.com/dan323/natural-deduction/sessions/c75973e4-b1cd-4677-92a2-fb1ef3782a06 --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: dan323 <32780500+dan323@users.noreply.github.com>
|
|
Summary
Updated documentation to reflect the migration from Create React App to Vite + Jest:
process.env.REACT_APP_API_URLwithimport.meta.env.VITE_API_URL, and fixed Jest CLI syntax for running specific tests.-- --coverageflag (coverage is always collected perjest.config.ts).Opened by multi-repo-tasks via Claude Code.