fix: upgrade Vite to v8 + enable React Compiler#5657
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
bb19535 to
5cff30a
Compare
5cff30a to
75b4115
Compare
Greptile SummaryThis PR upgrades Vite from v6 to v8, enables the React Compiler via
Confidence Score: 4/5Safe to merge after confirming no wallet-SDK dependency reads Buffer as a global during module initialisation, and verifying the isLoading addition in StakeForm is intentional. The store refactor and form-watch migration are thorough and consistent across the codebase. The only non-trivial risks are: (1) the Buffer polyfill is now set after all module side-effects execute, which could silently break a dependency that accesses the global early; and (2) isLoading is now forwarded to VaultForm from StakeForm without explanation, changing button behaviour in a PR ostensibly about tooling. Neither is a definitive bug, but both deserve a quick confirmation before shipping to production. apps/evm/src/index.tsx (Buffer global ordering), apps/evm/src/containers/VenusVaultModal/StakeForm/index.tsx (isLoading addition) Important Files Changed
Reviews (1): Last reviewed commit: "feat: upgrade Vite to v8 and enable Reac..." | Re-trigger Greptile |
| const fromAmountTokensFieldValue = useWatch({ control: form.control, name: 'fromAmountTokens' }); | ||
| const fromAmountTokens = new BigNumber(fromAmountTokensFieldValue || 0); | ||
|
|
||
| const { stake } = useStakeInVault(); | ||
| const { stake, isLoading } = useStakeInVault(); | ||
|
|
There was a problem hiding this comment.
Unrelated behavioral change bundled into infrastructure PR
isLoading is now destructured from useStakeInVault() and forwarded to VaultForm. This is a functional change — if VaultForm uses isLoading to disable or show a spinner on the submit button, this changes the button's behaviour during staking. This appears unrelated to the Vite v8 / React Compiler migration and is not reflected in the PR description. Was this intentional? Is extracting isLoading from useStakeInVault and passing it to VaultForm intentional? It changes stake-button behaviour and seems unrelated to the Vite/React Compiler upgrade.
Greptile SummaryThis PR upgrades Vite from v6 to v8 (switching the bundler core to Rolldown) and enables the React Compiler for production builds. Alongside the tooling change, it removes the
Confidence Score: 4/5Safe to merge — the migrations are mechanical and complete, no regressions found in the store or form-watch patterns. The store selector migration and form.watch to useWatch migration are done consistently across all 55 files with no leftover old-pattern calls. Two non-blocking observations in vite.config.mts: the reactCompilerPreset filter is applied by mutating the preset object after creation, and jsxImportSource is now global instead of opt-in per file. apps/evm/vite.config.mts — React Compiler filter mutation and global jsxImportSource change are worth a second look. Important Files Changed
Reviews (2): Last reviewed commit: "feat: upgrade Vite to v8 and enable Reac..." | Re-trigger Greptile |
| const isTest = mode === 'test'; | ||
| const reactCompiler = isTest | ||
| ? undefined | ||
| : reactCompilerPreset({ | ||
| compilationMode: 'infer', | ||
| panicThreshold: 'none', | ||
| }); | ||
|
|
||
| if (reactCompiler) { | ||
| reactCompiler.rolldown.filter ??= {}; | ||
| reactCompiler.rolldown.filter.id = { | ||
| include: ['**/apps/evm/src/**'], |
There was a problem hiding this comment.
Mutating
reactCompiler preset object after construction
reactCompilerPreset returns a preset object and this code immediately mutates its .rolldown.filter.id property to apply file-scope filtering. If the reactCompilerPreset API in a future patch release changes the shape of the returned object (e.g., renames .rolldown), this silent mutation would silently stop filtering (compiling all files including packages) rather than throw. Consider checking whether reactCompilerPreset accepts filter/include options directly as constructor arguments so the filter is part of the initial configuration rather than a post-hoc mutation.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| react({ | ||
| jsxImportSource: '@emotion/react', | ||
| }), | ||
| ...(reactCompiler |
There was a problem hiding this comment.
Global
jsxImportSource changes JSX runtime for all files
Setting jsxImportSource: '@emotion/react' on the React plugin applies Emotion's JSX runtime to every file in the build, even those that don't use the css prop. Previously, files that needed Emotion's jsx function opted in with the /** @jsxImportSource @emotion/react */ pragma (e.g. ActiveVotingProgress, ApprovalSteps). Those pragmas are now redundant but harmless. The behaviour change to be aware of is that every JSX element now goes through Emotion's jsx wrapper, adding a small runtime overhead that didn't exist before for the majority of components that never use the css prop.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Jira ticket(s)
VPD-1406
Changes