salvage: orphaned wip from concurrent agent (do not merge)#7791
salvage: orphaned wip from concurrent agent (do not merge)#7791lalalune wants to merge 1 commit into
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Claude encountered an error after 1s —— View job I'll analyze this and get back to you. |
|
Closing per maintainer decision: this PR was an automated salvage commit from a concurrent agent and was explicitly marked 'do not merge' in the title. Real changes should be cherry-picked by the owning agent into a properly-scoped PR rather than landed wholesale. |
| }, | ||
| react({ include: /\.(jsx|tsx|mdx)$/ }), | ||
| tailwindcss(), | ||
| visualizer({ | ||
| filename: "dist/stats.html", | ||
| gzipSize: true, | ||
| brotliSize: false, |
There was a problem hiding this comment.
dist/stats.html will be publicly accessible after deploy
rollup-plugin-visualizer writes dist/stats.html to the build output directory. Since that directory is deployed as a static asset tree, anyone can browse to https://elizacloud.ai/stats.html and see the full bundle treemap — exposing internal package names, file paths, relative sizes, and the complete dependency graph. The same issue exists in packages/homepage/vite.config.ts and packages/os-homepage/vite.config.ts. Consider adding dist/stats.html to the deploy ignore list, moving the output outside dist/ (e.g. tmp/stats.html), or gating the plugin on mode !== "production".
| // anywhere). The dynamic-import callsite still triggers preload via | ||
| // Vite's __vitePreload helper at navigation time. | ||
| modulePreload: { | ||
| polyfill: false, | ||
| resolveDependencies: (_filename, deps) => | ||
| deps.filter( | ||
| (dep) => | ||
| /vendor-react|rolldown-runtime/.test(dep) || | ||
| dep.endsWith(".css"), |
There was a problem hiding this comment.
modulePreload filter is tied to internal Vite chunk-name conventions
The regex /vendor-react|rolldown-runtime/ matches Vite/Rolldown's current internal naming for the React vendor chunk and the module loader runtime. If the bundler ever renames these chunks (e.g. Rolldown graduates from beta and changes defaults, or the manual-chunks configuration evolves), the filter silently matches nothing and the runtime is no longer preloaded — causing a measurable regression in Time-to-Interactive on first load. Consider asserting in a build-time check that at least one rolldown-runtime and one vendor-react chunk was actually emitted, so a naming drift is caught at build time rather than silently degrading production performance.
Relates to
Risks
Background
What does this PR do?
What kind of change is this?
Documentation changes needed?
Testing
Where should a reviewer start?
Detailed testing steps
Greptile Summary
This PR refactors the
cloud-frontend,homepage, andos-homepagepackages: replacing ~90 hardcoded hex brand colors (#FF5800,#0B35F1) with CSS custom properties (var(--brand-orange),var(--brand-blue)), and extracts theCheckoutPage/CheckoutResult/ProductDetailcomponents fromos-homepage/App.tsxinto their own files with lazy-loading. Build tooling is also updated across all three packages.cloud-frontendare replaced with CSS custom properties, enabling centralized theme control.React.lazy/Suspense, reducing the initial homepage bundle size.package.jsoninstead of a hardcoded regex;rollup-plugin-visualizeris added to all three frontend packages;cloud-frontendgains amodulePreloadfilter to avoid eagerly preloading heavy wallet/chart chunks on the landing page.Confidence Score: 3/5
The CSS-variable and code-splitting changes are safe; the stats.html output being deployed publicly needs to be addressed before shipping.
The vast majority of cloud-frontend changes are mechanical hex-to-CSS-variable substitutions with no logic impact. The os-homepage refactoring cleanly separates checkout and product-detail into lazy-loaded modules. The one issue worth resolving before merge is that all three packages now write dist/stats.html into their production build output, making internal bundle composition visible to anyone who visits /stats.html on the deployed sites.
All three vite.config.ts files (cloud-frontend, homepage, os-homepage) need the visualizer output relocated or excluded from deployment. packages/homepage/src/App.tsx RouteFallback color inversion is worth a second look.
Security Review
rollup-plugin-visualizeris configured withfilename: "dist/stats.html"inpackages/cloud-frontend/vite.config.ts,packages/homepage/vite.config.ts, andpackages/os-homepage/vite.config.ts. This file is written into the production build output and will be served as a public static asset, exposing the full internal bundle composition (file paths, package names, dependency graph, chunk sizes) to anyone who navigates to/stats.html.Important Files Changed
Comments Outside Diff (1)
packages/homepage/src/App.tsx, line 16-28 (link)The
RouteFallbackbackground was changed fromBRAND_COLORS.blacktoBRAND_COLORS.orangeand thecolorfromBRAND_COLORS.whitetoBRAND_COLORS.black. Every lazy route navigation now flashes a bright-orange full-screen background before the chunk loads. On fast connections this can appear as an intrusive orange flash; on slow connections users stare at an orange screen. All other loading spinners in the codebase use a dark background — this is the only one with an inverted scheme. Confirm this inversion is intentional.Reviews (1): Last reviewed commit: "salvage: orphaned wip from concurrent ag..." | Re-trigger Greptile