Migrate React frontend from Create React App to Vite#291
Conversation
vmuralictr
left a comment
vmuralictr
left a comment
There was a problem hiding this comment.
Thanks for working on this! Vite migration is a great improvement over CRA — faster dev server and HMR are definitely worth it. However, there are a few issues that need to be addressed before this can be merged.
-
Test runner missing
react-scripts test has been removed and the test script is no longer in package.json. After this merge, npm test will fail and the CI pipeline will break. Please add vitest or configure Jest separately to keep the test suite working. -
FAQ data loss
faq_data.js has been reduced from the full dataset down to 2 placeholder items. This looks unintentional — it would wipe all FAQ content in production. Please restore the original data. -
package-lock.json missing
package-lock.json should be committed alongside package.json. Without it, npm ci in CI will fail and builds won't be reproducible across environments. -
Environment variable breaking change
REACT_APP_API_URL has been renamed to VITE_REACT_APP_API_URL in SearchBar.jsx. Anyone with an existing .env file will need to update it. Please update .env.example to reflect this change so contributors know what to update. -
Script rename breaks docs
npm run start has been replaced by npm run dev — docs/Installation.md still references npm run start which will now fail. Please either keep "start": "vite" as an alias or update the docs as part of this PR. -
Proxy misconfiguration in vite.config.js
The proxy is set to /api → http://localhost:5000, but the Flask backend serves all routes under /sdt. This means all API calls will fail in development. Please update to:
proxy: {
'/sdt': 'http://localhost:5000'
}
7. Merge conflicts
This PR currently has merge conflicts that need to be resolved before it can be reviewed further. Please rebase your branch on the latest master and resolve the conflicts:
git fetch upstream
git rebase upstream/master
git push --force-with-lease origin vite-migration
Overall this is a solid migration effort — once these issues are fixed it should be good to go!
|
Hi @Mansi2007275, just following up on the review comments left last week. Could you please take a look when you get a chance? Happy to help if you have any questions on the feedback! |
|
Hi @vmuralictr, I've addressed all the review comments — added the test runner (vitest), restored the FAQ data, committed package-lock.json, updated env variables to use the VITE prefix, kept npm start as an alias, fixed the proxy config to use /sdt, resolved the merge conflicts, and also cleaned up some duplicate code that came up during the conflict resolution. Ready for another look whenever you get a chance!" |
Mansi2007275
force-pushed
the
vite-migration
branch
from
0bb95da to
8843195
Compare
vmuralictr
left a comment
vmuralictr
left a comment
There was a problem hiding this comment.
Tested this locally — the Vite migration works and the app loads correctly. A few things need to be cleaned up before merging:
- Update index.html title and description from placeholder values
- Remove commented-out dead code in SearchBar.jsx and main.jsx
- Delete App.js (empty placeholder) and Accordion.js (deprecated stub)
- Fix package.json name (sdt-veryfinal) and remove CRA-specific eslintConfig
- Remove the dead /api proxy comment in vite.config.js
| ``` | ||
| Ensure `REACT_APP_API_URL` points to your backend URL (e.g., `http://localhost:5000`). | ||
|
|
||
| <!-- Ensure `REACT_APP_API_URL` points to your backend URL (e.g., `http://localhost:5000`). --> |
There was a problem hiding this comment.
The old REACT_APP_API_URL line is commented out as an HTML comment (). It's better to just remove it entirely rather than leaving it as a visible comment in the markdown — readers may find it confusing.
| <meta name="viewport" content="width=device-width, initial-scale=1.0" /> | ||
| <meta name="theme-color" content="#000000" /> | ||
| <meta name="description" content="React + Vite App" /> | ||
| <title>React App</title> |
There was a problem hiding this comment.
The title is React App and description is React + Vite App. These should be updated to match the project:
<title>Software Discovery Tool</title>
<meta name="description" content="Discover Open Source Packages for Z architecture" />| ) | ||
| } | ||
|
|
||
| export default App No newline at end of file |
There was a problem hiding this comment.
This file only contains a placeholder comment and export {}. It should be deleted — App.jsx handles everything. Keeping an empty App.js alongside App.jsx is misleading.
|
|
||
| // If you want to start measuring performance in your app, pass a function | ||
| // to log results (for example: reportWebVitals(console.log)) | ||
| // or send to an analytics endpoint. Learn more: https://bit.ly/CRA-vitals |
There was a problem hiding this comment.
The old CRA-style code is fully commented out. Please remove these lines — the working Vite code below it is sufficient.
| // setOsList(data); | ||
| // }); | ||
| // }; | ||
|
|
There was a problem hiding this comment.
This is a duplicate of the active fetchOSList function defined above, with debug console.log statements included. Please remove this entire commented-out block before merging.
| const searchTerm = params ? params.value : value; | ||
|
|
||
|
|
||
| // const apiUrl = `${import.meta.env.VITE_REACT_APP_API_URL}/searchPackages?search_term=${value}&exact_match=${exact}&search_bit_flag=${searchBitFlag}${osFilters}`; |
There was a problem hiding this comment.
Remove the commented-out apiUrl variable and debug comment. The active code below already handles this correctly.
| port: 3000, | ||
| // proxy: { | ||
| // '/api': 'http://localhost:5000' | ||
| // } |
There was a problem hiding this comment.
The /api proxy is commented out — that's fine since the project uses /sdt. Consider removing the dead commented-out proxy block to keep the config clean.
|
Hi @vmuralictr, all review comments have been addressed:
|
Mansi2007275
force-pushed
the
vite-migration
branch
from
9108b5a to
bd933fc
Compare
|
@vmuralictr sir please merge my pr if there is no issue ............if there is any issue you can share i will resolve it please ... |
muraliveesambattu
left a comment
muraliveesambattu
left a comment
There was a problem hiding this comment.
This PR has two critical blockers that must be fixed before merging:
-
.gitignore corrupted with PowerShell syntax
The file contains a Windows PowerShell heredoc command instead of valid gitignore patterns. Critical entries like *.pyc and src/config/supported_distros.py (which contains sensitive DB config) are no longer ignored. A git add . could accidentally expose credentials. -
vite.config.js proxy vs absolute URL mismatch
The proxy routes /sdt but VITE_REACT_APP_API_URL is set to an absolute URL, so the proxy is never triggered.
Other issues noted in inline comments:
- Both dev and start scripts run the same command — redundant
- package-lock.json should be regenerated via npm install, not manually edited
- npm start in docs should be updated to npm run dev
| .env | ||
| build/ | ||
| dist/ | ||
| "@ | Out-File -FilePath .gitignore -Encoding utf8 No newline at end of file |
There was a problem hiding this comment.
This file now contains a Windows PowerShell heredoc command (@"..."@ | Out-File...) instead of valid gitignore patterns. This appears to have been generated by running a PowerShell script on Windows and accidentally committing the command itself as file content.
As a result:
- @" and "@ | Out-File -FilePath .gitignore -Encoding utf8 are treated as literal gitignore patterns (matching nothing)
- *.pyc is no longer ignored — Python bytecode files will be tracked
- src/config/supported_distros.py is no longer ignored — this is a generated file containing sensitive DB config that must stay out of the repo
Please replace the entire file content with proper gitignore patterns and ensure *.pyc and src/config/supported_distros.py are included.
| "react-app/jest" | ||
| ] | ||
| "dev": "vite", | ||
| "start": "vite", |
There was a problem hiding this comment.
Both dev and start run the same command (vite). This is redundant. Vite's convention is npm run dev — consider removing start to avoid confusion, or keep only dev and update the docs accordingly.
|
@vmuralictr @pleia2 please review this PR if no issue please merge it |
|
Hi @vmuralictr, I've addressed the remaining feedback: Removed the duplicate start script in package.json All previous review items have also been verified locally — FAQ data restored, test runner (vitest) working, dead code removed, App.js/Accordion.js deleted, .gitignore fixed, proxy config correct. |
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
vmuralictr
left a comment
vmuralictr
left a comment
There was a problem hiding this comment.
Thanks for addressing the feedback! The critical blockers are all fixed — vitest is wired up, .gitignore is clean, dead code removed, App.js deleted, proxy looks correct.
A few small things to clean up when you get a chance (not blocking):
.env.example: remove the# // REACT_APP_API_URLline and align theVITE_REACT_APP_API_URLvalue with what Installation.md says (/sdtvshttp://localhost:5000)SearchBar.jsx:const BASE_URLis unindented at column 0 — just a style nit- Several files are missing trailing newlines
Approving — nice work on the migration!
Signed-off-by: Mansi2007275 <mansi@example.com>
…cate code, update env vars, fix proxy config, update docs Signed-off-by: Mansi2007275 <mansi@example.com>
Signed-off-by: Mansi2007275 <mansi@example.com>
Signed-off-by: Mansi2007275 <mansi@example.com>
Signed-off-by: Mansi2007275 <mansi@example.com>
Mansi2007275
force-pushed
the
vite-migration
branch
from
306f1f4 to
b81ddd1
Compare
|
Hi @vmuralictr @muraliveesambattu, |
3 participants
This PR migrates the
react-frontendfrom Create React App (CRA) to Vite as part of issue #287.The migration modernizes the frontend tooling setup, improves development performance, reduces dependency overhead, and removes reliance on the deprecated
react-scriptsecosystem.Changes Implemented
### Vite Migration
vite.config.jsconfigurationFrontend Entry Migration
src/index.jswithsrc/main.jsxindex.htmlfrompublic/to the React frontend root directoryReact Component Updates
.jsxcomponent structure compatibility for VitePackage & Script Updates
Updated
package.jsonscripts:start→devRemoved CRA-specific dependencies
Added Vite and React plugin dependencies
Validation & Testing
Installed dependencies successfully
Verified frontend launches correctly with:
Confirmed existing pages/components render properly after migration
Checked FAQ and search functionality after migration changes
Verified no breaking issues in frontend startup flow
## Additional Notes
node_moduleswas intentionally excluded from the PRpackage-lock.jsonchanges were omitted to keep the PR cleaner and easier to review## Outcome
This migration improves:
It also removes dependency on the deprecated Create React App tooling ecosystem.