fix: bundle size optimization#2849
Conversation
Main upstream
|
@tomaszantas is attempting to deploy a commit to the langfuse Team on Vercel. A member of the Team first needs to authorize it. |
|
@claude review |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Clean bundle-size optimization — dynamic imports for three.js and recharts are correctly wired, image hints are appropriate, and the removals fall back to Next.js's default of 75. Worth a visual spot-check on the preview, but no correctness concerns.
Extended reasoning...
Overview
The PR makes three independent, mechanical optimizations across 15 files: (1) splits Three.js into a dynamic chunk by extracting the real implementation into NotFoundAnimationImpl.tsx and replacing the no-op Promise.resolve(Component) pattern with a true import(), (2) splits recharts into MetricsCharts.tsx and dynamically imports both charts into Metrics.tsx (used only on /wrapped), and (3) drops quality={100}/unoptimized props on homepage images so Next.js's default compression and optimization pipeline runs. Seven PNG binary assets are also recompressed.
Security risks
None. All changes are client-side bundling, image optimization props, and static binary swaps. No touching of auth, crypto, permissions, data handling, API surfaces, or user input. This is a docs/marketing site, not production backend.
Level of scrutiny
Low. Each logical change is a well-documented Next.js pattern: dynamic() with ssr: false for client-only heavy deps, named re-exports via .then((m) => ({ default: m.X })), StaticImageData imports auto-supplying width/height to <Image>, and sizes hints for responsive optimization. The Promise.resolve(Component) → real import() conversion is a correct bug fix — the prior code was a known anti-pattern that defeated code-splitting.
Other factors
Bug hunter found no issues. Greptile reviewed with 5/5 confidence; its two P2 notes (duplicated CANVAS_STYLES constant between NotFoundAnimation.tsx and NotFoundAnimationImpl.tsx; no loading skeleton on the dynamic charts) are nits that don't affect correctness. The quality=75 default is the Next.js / web-perf industry norm and the payload deltas (~52% total homepage reduction, ~23% on /docs) are substantial and measured. Since image quality is inherently subjective, a quick visual check on the Vercel preview is advisable before merging, but that is a UX judgment call rather than a code-correctness issue.
| // Dynamic import keeps recharts out of every page's initial chunk graph. | ||
| // The charts only render on /wrapped, so there's no reason to ship ~100 KB | ||
| // brotli of recharts on /, /docs/*, /pricing, etc. | ||
| const ConsumptionChart = dynamic( | ||
| () => import("./MetricsCharts").then((m) => ({ default: m.ConsumptionChart })), | ||
| { ssr: false } | ||
| ); | ||
| const DownloadsChart = dynamic( | ||
| () => import("./MetricsCharts").then((m) => ({ default: m.DownloadsChart })), | ||
| { ssr: false } | ||
| ); | ||
|
|
There was a problem hiding this comment.
Why would these otherwise show up in the main bundle? It looks like components/wrapped/Metrics.tsx is only imported from content/marketing/wrapped.mdx. Do content routes hoist all their imports to the main content page?
Improve bundle size:
Homepage
//docsNote:
?dpl=<id>cache-busting suffix on every internal URL. Preview-environment artifact; disappears when staging merges tolangfuse.com.Disclaimer: Experimental PR review
Greptile Summary
This PR delivers three independent bundle-size improvements: correct dynamic splitting of Three.js (404 page) and recharts (
/wrapped), removal ofquality={100}overrides so Next.js's default 75% JPEG compression applies on homepage images, and lossless/near-lossless compression of static PNGs. The net result is a ~52% reduction in total homepage transfer size and ~23% reduction on/docs, per the benchmark table in the description. All changes are mechanically sound — the previousPromise.resolve(Component)anti-pattern (which prevented real code-splitting) is correctly replaced with a true dynamicimport().Confidence Score: 5/5
Safe to merge — all remaining findings are minor style suggestions with no correctness impact.
No P0 or P1 issues found. The bundle-split logic is correct, image handling is valid (StaticImageData sources auto-supply dimensions), and the binary image replacements are transparent swaps. The two P2 comments (duplicated constant, missing loading skeleton) are cleanup suggestions that do not affect runtime correctness.
No files require special attention.
Important Files Changed
Promise.resolve(Component)trick did NOT code-split; the real dynamicimport("./NotFoundAnimationImpl")now does.CANVAS_STYLESconstant is duplicated between this file and NotFoundAnimation.tsx — minor redundancy.ssr: falseon the parent dynamic call means charts are client-only which is appropriate for a /wrapped page.tool.visualvalues areStaticImageDataimports, so Next.js Image infers width/height automatically even without explicit props;sizesis correctly added andunoptimizedremoved.quality={100}overrides so Next.js default quality (75) applies, reducing image payload with negligible visual impact at these sizes.quality={100}removal pattern as FeatureTabs.tsx; change is safe and correct.Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[Browser requests page] --> B{Page route} B --> |"/ (homepage)"| C[Initial JS bundle] B --> |"/wrapped"| C B --> |"/404"| C C --> D[AllTheTools — static images\nNext.js optimized via sizes hint] C --> E[FeatureTabs / TabContent\nquality default 75 instead of 100] C --> F{Dynamic import\non demand} F --> |"404 page renders"| G["NotFoundAnimationImpl chunk\n(Three.js + @react-three/*)"] F --> |"/wrapped page renders"| H["MetricsCharts chunk\n(recharts ~100 KB br)"] G --> I[Three.js canvas mounted\nssr: false] H --> J[ConsumptionChart rendered\nssr: false] H --> K[DownloadsChart rendered\nssr: false] style G fill:#f9a,stroke:#c66 style H fill:#f9a,stroke:#c66 style D fill:#9f9,stroke:#696 style E fill:#9f9,stroke:#696Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "fix: optimized images (#106)" | Re-trigger Greptile