Conversation
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
sfc-gh-bnisco
added
the
change:chore
Graphite App
✅ PR preview is ready!
|
sfc-gh-bnisco
added
the
impact:internal
Graphite App
📉 Significant bundle size change detected
Please verify that this change is expected. |
📉 Significant wheel size change detectedThe wheel file size has decreased by 1.55% (threshold: 0.25%)
Please verify that this change is expected. |
sfc-gh-bnisco
force-pushed
the
03-12-chore_upgrade_vite_v7_-_v8
branch
2 times, most recently
from
c7f00f8 to
0ffa2f7
Compare
sfc-gh-bnisco
added
the
ai-review
There was a problem hiding this comment.
Pull request overview
Upgrades the frontend toolchain from Vite 7 → Vite 8 (and Vitest 4.0.x → 4.1.x) across the monorepo, applying required migration changes (Rolldown config renames, CJS interop shims) and updating build output handling and snapshots to keep Streamlit’s build/dev/test flows working.
Changes:
- Bump Vite/Vitest (and related deps) across frontend workspaces and refresh
yarn.lock. - Migrate Vite configs from
build.rollupOptions→build.rolldownOptions, and adjust app asset URL handling for Rolldown output. - Add targeted runtime/interop shims: BaseUI theme import workaround,
react-json-view/react-plotly.jscompat modules,react-uidalias shim, and a DeckGL/Luma canvas resize guard.
Reviewed changes
Copilot reviewed 24 out of 46 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/yarn.lock | Lockfile refresh for Vite 8 / Vitest 4.1 dependency graph (Rolldown, lightningcss, etc.). |
| frontend/package.json | Bump root dev deps for Vitest + coverage. |
| frontend/utils/package.json | Bump workspace dev deps (vite/vitest). |
| frontend/app/package.json | Bump app dev deps (vite, vitest, plugin-react-swc). |
| frontend/app/vite.config.ts | Switch to rolldownOptions, add react-uid alias, adjust asset routing and renderBuiltUrl for fonts. |
| frontend/app/src/util/reactUidCompat.ts | New Vite-8 interop shim for react-uid (used via Vite alias). |
| frontend/lib/package.json | Bump lib dev deps (vite/vitest/plugin-react-swc). |
| frontend/lib/vite.config.ts | Switch to build.rolldownOptions for lib build. |
| frontend/lib/src/theme/createBaseUiTheme.ts | BaseUI import pathing adjusted for Vite 8 interop behavior. |
| frontend/lib/src/util/reactJsonViewCompat.ts | New compat wrapper to normalize CJS default export shape for react-json-view. |
| frontend/lib/src/util/reactPlotlyCompat.ts | New compat wrapper to normalize CJS default export shape for react-plotly.js. |
| frontend/lib/src/components/elements/Json/Json.tsx | Consume reactJsonViewCompat for runtime-stable JSON rendering. |
| frontend/lib/src/components/elements/Json/useJsonTooltip.ts | Import OnSelectProps via compat module. |
| frontend/lib/src/components/widgets/DataFrame/columns/cells/JsonViewer.tsx | Use reactJsonViewCompat in DataFrame JSON viewer. |
| frontend/lib/src/components/elements/PlotlyChart/PlotlyChart.tsx | Use reactPlotlyCompat for Plotly component + types. |
| frontend/lib/src/components/elements/PlotlyChart/PlotlyChart.test.tsx | Import Plotly types from compat module. |
| frontend/lib/src/components/elements/DeckGlJsonChart/patchLumaCanvasContext.ts | New prototype patch to guard Luma resize crash when device.limits is temporarily unavailable. |
| frontend/lib/src/components/elements/DeckGlJsonChart/DeckGlJsonChart.tsx | Apply the Luma canvas context patch via side-effect import. |
| frontend/connection/package.json | Bump connection workspace dev deps (vite/vitest). |
| frontend/connection/vite.config.ts | Switch to build.rolldownOptions. |
| frontend/component-lib/package.json | Bump component-lib dev deps (vite/vitest). |
| frontend/component-lib/vite.config.ts | Switch to build.rolldownOptions. |
| frontend/component-v2-lib/package.json | Bump component-v2-lib dev deps (vite/vitest). |
| frontend/eslint-plugin-streamlit-custom/package.json | Bump vitest deps for eslint plugin workspace. |
| frontend/knip.json | Ignore knip “files” issue for the Vite-alias-only compat file. |
| e2e_playwright/snapshots/linux/custom_light_sidebar_theme_test/custom_dark_sidebar_theme_no_dark_configs[chromium].png | Updated snapshot reflecting new render/build output. |
Comments suppressed due to low confidence (1)
frontend/app/vite.config.ts:221
assetInfo.namesandassetInfo.originalFileNamesare treated as optional (you already fall back to[]inassetNames), but this block calls.includes(...)onassetInfo.names/assetInfo.originalFileNamesdirectly. If either isundefinedunder Rolldown, the build will throw at config-eval time. Use the already-created fallback arrays (or optional chaining) for these.includeschecks.
const assetNames = assetInfo.names || []
const hasAssetExtension = (extensions: string[]): boolean =>
assetNames.some(name =>
extensions.some(extension => name.endsWith(extension))
)
// For CSS files, place them in the /static/css/ directory
if (hasAssetExtension([".css"])) {
// If OMIT_HASH_FROM_MAIN_FILES is set, we don't want to include the
// hash in the filename of the entry file at the minimum. There could
// be other files with the same name that cause a conflict, which would
// increment the entry file to index2.css, etc. This ensures the entry
// file is named index.css in this case.
if (
assetInfo.names.includes("index.css") &&
assetInfo.originalFileNames.includes("index.html")
) {
You can also share your feedback on Copilot code review. Take the survey.
github-actions
bot
removed
the
ai-review
SummaryThis PR upgrades the frontend build tooling from Vite 7 (Rollup-based) to Vite 8 (Rolldown-based) along with a Vitest 4.0.x → 4.1.x bump. Key changes include:
The cited performance benefits are significant: 72% faster Code QualityAll three reviewers agreed the code quality is high and the changes are well-scoped for a migration PR. Consensus strengths:
Consensus observations (non-blocking):
Test CoverageReviewers largely agreed that test coverage is adequate for a migration PR but noted a gap in dedicated unit tests for the new compat helpers:
Backwards CompatibilityUnanimous agreement: No breaking API changes. This is an internal build tooling upgrade with no changes to Streamlit's public Python or TypeScript API. The 21 updated E2E theme snapshots indicate pixel-level CSS rendering differences that are cosmetic and unlikely to affect users. The Security & RiskUnanimous agreement: Low risk. No changes to security-sensitive areas (auth, sessions, WebSocket handling, CORS, CSP, file upload/download, XSRF). No new Residual risk areas (consensus):
External test recommendationUnanimous recommendation: Yes — external testing is warranted.
AccessibilityUnanimous agreement: No accessibility impact. Changes are limited to build tooling and module interop — no UI components, ARIA attributes, focus management, or semantic HTML were modified. RecommendationsThese are non-blocking improvements, consolidated and prioritized from all three reviews:
VerdictAPPROVED: The Vite 7 → 8 migration is well-executed with thorough compatibility shims, excellent documentation, defensive coding practices, and significant build performance improvements. All three reviewers approved. The recommendations above are non-blocking improvements that can be addressed in this PR or as follow-ups. This is a consolidated AI review by 📋 Review by `gemini-3.1-pro`SummaryThis PR upgrades the frontend workspace dependencies from Vite 7 to Vite 8 and Vitest 4.0.x to 4.1.x, applying necessary migration fixes to accommodate Vite 8's new Rolldown bundler. Code Quality
Test Coverage
Backwards Compatibility
Security & Risk
External test recommendation
Accessibility
Recommendations
VerdictAPPROVED: The Vite 8 upgrade is implemented cleanly with excellent defensive coding and documentation around the temporary compatibility shims. This is an automated AI review by 📋 Review by `gpt-5.3-codex-high`SummaryThis PR upgrades the frontend toolchain from Vite 7 to Vite 8 (plus Vitest 4.1), migrates Vite config keys to Rolldown equivalents, and introduces targeted interop guards for CommonJS-heavy dependencies ( Code QualityThe changes are cohesive and well-scoped for a migration PR, with strong inline documentation and clear removal criteria in temporary compatibility shims. I did not find blocking correctness defects in the reviewed code paths. Notable maintainability observations:
Test CoverageCoverage is partially adequate for migration confidence, but there is a gap in targeted regression tests for the new compat helpers:
Backwards CompatibilityNo public Streamlit Python API changes are introduced, so end-user app code should remain compatible. The main compatibility risk is frontend runtime/build behavior under varied hosting conditions due to asset URL and module interop changes (not API surface changes). Security & RiskNo direct high-risk security patterns were introduced (no new auth/session logic, no new endpoint handling, no unsafe eval/exec patterns, no new external service calls).
External test recommendation
AccessibilityNo direct accessibility-impacting UI interaction changes were introduced (no new controls, roles, focus management, or ARIA behavior changes). Snapshot churn appears visual/build-output related rather than semantic DOM/a11y behavior changes. Recommendations
VerdictAPPROVED: The migration is well-structured and merge-ready, with no blocking defects found; the main residual risk is hosting-topology-dependent asset/interop behavior, which should be further hardened with targeted follow-up tests. This is an automated AI review by 📋 Review by `opus-4.6-thinking`SummaryThis PR upgrades the frontend build tooling from Vite 7 (Rollup-based) to Vite 8 (Rolldown-based), along with a Vitest 4.0.x → 4.1.x bump. The key changes are:
The performance benefits cited are significant: 72% faster Code QualityStrengths:
Minor observations:
Test Coverage
Backwards Compatibility
Security & Risk
External test recommendation
AccessibilityNo accessibility impact. The changes are limited to build tooling and module interop — no UI components, ARIA attributes, focus management, or semantic HTML were modified. The theme snapshot changes are pixel-level CSS differences that do not affect accessible names, roles, or keyboard behavior. Recommendations
VerdictAPPROVED: Well-executed Vite 7 → 8 migration with thorough compatibility shims, clear documentation, good defensive coding practices, and significant build performance improvements. The recommendations above are non-blocking improvements. This is an automated AI review by |
|
@github-actions Follow-up on your review suggestions: I updated |
sfc-gh-bnisco
force-pushed
the
03-12-chore_upgrade_vite_v7_-_v8
branch
from
73186ec to
41237a5
Compare
Describe your changes
This PR upgrades the frontend workspace from Vite 7 to Vite 8 (and Vitest 4.0.x to 4.1.x), and applies the migration fixes needed for Streamlit’s build/dev/test flows.
One of the biggest benefits for this is perf.
make frontend-fastis now 72% faster (50s -> 14s)vite:^7.3.1->^8.0.0@vitejs/plugin-react-swc:^4.2.3->^4.3.0vitest:^4.0.18->^4.1.0@vitest/coverage-v8:^4.0.18->^4.1.0build.rollupOptions->build.rolldownOptionsinapp,lib,connection, andcomponent-libassetInfo.names-based checksrenderBuiltUrlhandling for CSS font URLs so emitted font paths stay stable (../media/...)react-uidalias to local compat modulereact-json-viewandreact-plotly.jsto normalize nested default exportsCanvasContext.getMaxDrawingBufferSize()to avoid crashes whendevice.limitsis temporarily unavailableknipignore config for newly introduced compat filePer the Vite migration guidance, Vite 8 introduces Rolldown-based internals and changes CommonJS interop behavior. This branch applies the required config renames and adds targeted compatibility fixes for dependencies that surfaced runtime/import shape issues after the upgrade.
Reference: Vite Migration from v7
Screenshot or video (only for visual changes)
GitHub Issue Link (if applicable)
Testing Plan
Contribution License Agreement
By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.