Migrate to vite#933
Open
mshriver wants to merge 1 commit into
Open
Conversation
Contributor
Reviewer's GuideMigrate the frontend from Create React App/Jest to a Vite/Vitest-based toolchain, updating build/test scripts, lint configuration, and test code to use Vitest while removing obsolete CRA artifacts. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
Fixed security issues:
-
The ESLint config now imports
eslint-plugin-cypressfrom'eslint-plugin-cypress'but still usespluginCypress.configs.recommended, which is only exposed by the/flatentry point; consider switching back toeslint-plugin-cypress/flator updating the config usage to match the non-flat plugin API. -
In
vite.config.js, thejsxInJsplugin matches files via/src\/.*\.js$/, which will not work on Windows paths and may miss files when Vite uses absolute paths; consider using a more robust check likeid.endsWith('.js') && id.includes('/src/')orpath.sep-aware logic.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The ESLint config now imports `eslint-plugin-cypress` from `'eslint-plugin-cypress'` but still uses `pluginCypress.configs.recommended`, which is only exposed by the `/flat` entry point; consider switching back to `eslint-plugin-cypress/flat` or updating the config usage to match the non-flat plugin API.
- In `vite.config.js`, the `jsxInJs` plugin matches files via `/src\/.*\.js$/`, which will not work on Windows paths and may miss files when Vite uses absolute paths; consider using a more robust check like `id.endsWith('.js') && id.includes('/src/')` or `path.sep`-aware logic.
## Individual Comments
### Comment 1
<location path="frontend/vite.config.js" line_range="1-2" />
<code_context>
+import { defineConfig } from 'vitest/config';
+import { transformWithOxc } from 'vite';
+import react from '@vitejs/plugin-react';
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Vite does not export `transformWithOxc`; this plugin will likely fail at runtime.
The `jsxInJs` plugin imports `transformWithOxc` from `vite`, but this symbol is not part of Vite’s public API, so the import will fail and prevent the dev server/build from starting. To handle `.js` files with JSX, use Vite’s `transformWithEsbuild` or rely on the React plugin’s JSX support (e.g., via appropriate `esbuild` options) instead of a non-existent helper.
</issue_to_address>
### Comment 2
<location path="frontend/vite.config.js" line_range="5-12" />
<code_context>
+ name: 'jsx-in-js',
+ enforce: 'pre',
+ async transform(code, id) {
+ if (!/src\/.*\.js$/.test(id)) return null;
+ return transformWithOxc(code, id, { lang: 'jsx' });
+ },
</code_context>
<issue_to_address>
**suggestion (bug_risk):** The JSX-in-JS filter assumes POSIX paths and may skip files on non-Unix environments.
The plugin currently matches only POSIX-style paths (`src/...`) and will miss Windows paths with backslashes (e.g. `src\components\x.js`). For cross-platform support, normalize `id` to use `/` before testing, or base the check on `path.sep`/`endsWith('.js')` rather than a hardcoded `/src/` regex.
```suggestion
const jsxInJs = () => ({
name: 'jsx-in-js',
enforce: 'pre',
async transform(code, id) {
const normalizedId = id.replace(/\\/g, '/');
if (!/src\/.*\.js$/.test(normalizedId)) return null;
return transformWithOxc(code, normalizedId, { lang: 'jsx' });
},
});
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+1
to
+2
| import { defineConfig } from 'vitest/config'; | ||
| import { transformWithOxc } from 'vite'; |
Contributor
There was a problem hiding this comment.
issue (bug_risk): Vite does not export transformWithOxc; this plugin will likely fail at runtime.
The jsxInJs plugin imports transformWithOxc from vite, but this symbol is not part of Vite’s public API, so the import will fail and prevent the dev server/build from starting. To handle .js files with JSX, use Vite’s transformWithEsbuild or rely on the React plugin’s JSX support (e.g., via appropriate esbuild options) instead of a non-existent helper.
Comment on lines
+5
to
+12
| const jsxInJs = () => ({ | ||
| name: 'jsx-in-js', | ||
| enforce: 'pre', | ||
| async transform(code, id) { | ||
| if (!/src\/.*\.js$/.test(id)) return null; | ||
| return transformWithOxc(code, id, { lang: 'jsx' }); | ||
| }, | ||
| }); |
Contributor
There was a problem hiding this comment.
suggestion (bug_risk): The JSX-in-JS filter assumes POSIX paths and may skip files on non-Unix environments.
The plugin currently matches only POSIX-style paths (src/...) and will miss Windows paths with backslashes (e.g. src\components\x.js). For cross-platform support, normalize id to use / before testing, or base the check on path.sep/endsWith('.js') rather than a hardcoded /src/ regex.
Suggested change
| const jsxInJs = () => ({ | |
| name: 'jsx-in-js', | |
| enforce: 'pre', | |
| async transform(code, id) { | |
| if (!/src\/.*\.js$/.test(id)) return null; | |
| return transformWithOxc(code, id, { lang: 'jsx' }); | |
| }, | |
| }); | |
| const jsxInJs = () => ({ | |
| name: 'jsx-in-js', | |
| enforce: 'pre', | |
| async transform(code, id) { | |
| const normalizedId = id.replace(/\\/g, '/'); | |
| if (!/src\/.*\.js$/.test(normalizedId)) return null; | |
| return transformWithOxc(code, normalizedId, { lang: 'jsx' }); | |
| }, | |
| }); |
Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Migrate the frontend from create-react-app/Jest tooling to a Vite + Vitest stack, updating build and test scripts, configs, and dependencies while cleaning up legacy CRA artifacts and aligning project metadata with Ibutsu.
New Features:
Enhancements:
Build:
Documentation:
Tests:
Chores: