Repository navigation
perf: weekly react best-practices audit fixes - #4164
Merged
Merged
Conversation
The zip library is only needed inside the click-triggered downloadImagesAsZip path, so import it dynamically instead of shipping it in the image page's initial bundle. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CepaNxrPkU6gcEXT23M7WL
The package has no side-effectful modules, but without the declaration bundlers keep every barrel re-export, so one TimeZoneProvider import from the root barrel shipped the whole component library plus the model catalogue (~1MB of client JS) on every ui route. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CepaNxrPkU6gcEXT23M7WL
The footer was a client component importing providers-catalog, which uses the whole model catalogue to count active models per provider - an import tree-shaking cannot remove, shipped on every marketing page. The footer now reads getConfig() as a server component, and the AllModels client wrapper takes it as a slot from its server pages. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CepaNxrPkU6gcEXT23M7WL
HeroCompare shipped framer-motion on all seven /compare pages for a one-shot entrance; it now uses the landing hero's animate-hero-enter CSS utilities (with reduced-motion support) and renders server-side. The seven static comparison tables had "use client" with no hooks or handlers; without the directive only the AuthLink leaves hydrate. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CepaNxrPkU6gcEXT23M7WL
The SSR'd errors/performance monitoring demos generated their dataset with unseeded crypto randomness during render, so the prerendered HTML always hydration-mismatched and React 19 re-rendered the whole subtree. A seeded PRNG makes server and client renders identical. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CepaNxrPkU6gcEXT23M7WL
The Flex tier toggle persisted its cookie via an effect watching the state, which also wrote spuriously on mount. The write moved into the change handler. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CepaNxrPkU6gcEXT23M7WL
Contributor
|
Understand this PR’s impact Explore downstream dependencies and potential security impact with Blast Radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (26)
💤 Files with no reviewable changes (7)
Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review. WalkthroughThe pull request updates playground persistence and image downloads, adds server-provided footers to model pages, converts landing components to server components, replaces hero motion wrappers with CSS animations, seeds mock data generation, and marks the shared package as side-effect free. ChangesPlayground updates
Model page footer rendering
Landing component rendering
Deterministic mock data
Shared package metadata
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Refactor Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 21.05% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 19 functions across 18 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
🛠️ Fix failing CI checks 💡
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 |
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
2 participants
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.
Weekly React/Next.js best-practices audit of the four Next.js apps (
apps/ui,apps/code,apps/playground,apps/docs) against Vercel's react-best-practices guide. All prior audit rounds are merged on main, so this round covers code merged since #4058 plus one long-standing bundle problem on the marketing surface that turned out to be the dominant finding. No routing or user-visible behavior changes; nothing touchesexport const dynamic.Fixes
Bundle size
Avoid Barrel File Imports (rule 2.1) —
packages/sharedwas un-tree-shakeable, shipping the model catalogue on every ui route. The root@llmgateway/sharedbarrel re-exports from./components/index.js, whose graph reaches the models-directory components and through them the full@llmgateway/modelscatalogue. Because the package declared nosideEffects, bundlers had to keep every re-export, so any client component importing the barrel for one symbol (the rootproviders.tsxforTimeZoneProvider, the navbar, the FAQ…) dragged the whole component library plus the catalogue into its route. Declaring"sideEffects": false(the package has no side-effectful modules: no bare imports, no CSS, no global mutations) lets the bundler tree-shake the barrels. Measured on the built ui client-reference manifests, per-route referenced client JS (uncompressed):/2076 KB → 977 KB,/blog2002 KB → 891 KB,/compare/open-router1996 KB → 888 KB — catalogue bytes on all of them 976 KB → 0 KB/modelskeeps only the 137 KB of catalogue chunks it actually uses (was 976 KB), and the dashboard project page now references 0 catalogue bytesThis is a one-line change in
packages/shared(allowed as strictly required by the fix) and benefits every app that bundles the package.Client bundling of the model catalogue via the footer (rules 2.1/2.4) —
apps/ui. The marketingFooterwas"use client"and importedlistedProvidersfromproviders-catalog, which uses the whole catalogue to count active models per provider — a use tree-shaking cannot remove, so with the fix above alone the catalogue would have returned to all ~46 footer-bearing marketing routes.Footeris now a server component (it readsgetConfig()directly;Newsletterstays a client island), and the one client consumer — theAllModelswrapper — takesfooteras a slot filled by its server pages (standard RSC composition). The numbers above are measured with both fixes in place.Defer Non-Critical Third-Party Libraries (rule 2.3) — compare-page heroes,
apps/ui.HeroCompare(all seven/compare/*pages) shipped framer-motion in the initial bundle solely for a one-shot blur/slide hero entrance. It now uses the sameanimate-hero-enterCSS utilities the main landing hero was already migrated to (visually equivalent keyframes, plus the reduced-motion handling the JS variant lacked) and becomes a server component. No other component in the compare graph imports motion, so the library leaves those pages' first load entirely.Unnecessary client components — comparison tables,
apps/ui. The seven static feature-comparison tables (comparison*.tsx, ~270 lines of constant JSX each) were"use client"with zero hooks or handlers, so every/compare/*page shipped and hydrated the whole table. The directives are removed; the tables render as server components and only the existingAuthLinkleaves hydrate. All importers are server pages.Conditional Module Loading (rules 2.2/2.3) —
apps/playgroundzip download.image-download.tsstatically importedfflate, putting the zip library in the Image Studio's initial bundle although it is only needed in the click-triggered "download all" path. It now loads viaimport()in parallel with the image bytes, matching the repo's existing on-demandhtml-to-image/jspdfpattern.Hydration correctness
apps/uifeature-page demos.generateMockActivityData()built the demo dataset for the SSR'd errors-monitoring and performance-monitoring demos with unseeded crypto randomness on every render, so the statically generated HTML and the hydrating client always disagreed on every stat — a guaranteed mismatch, and React 19 re-renders the whole subtree. The generator now uses a seeded PRNG (mulberry32), so server and client produce identical data; the demos are also stable across re-renders now (random-in-render also violates the purity the React Compiler assumes).Interaction logic
apps/playgroundImage Studio. The Flex service-tier toggle persisted its cookie through auseEffectwatching the state (including a spurious write on every mount). The cookie write moved into the change handler; the effect is gone.Considered and deliberately skipped
export const dynamic— intentional runtime-env loading; excluded per repo policy (no finding touched one this round)./user/me(useLoungePoints,enabled: !!user— rules 1.5/4.3): a real one-round-trip waterfall for signed-in members, but the session cookie is httpOnly, so the clean fix threads a server-derived signed-in hint through a new context — auth-signal plumbing, not a surgical perf diff. Left for a dedicated change.getUser()before parsing/validating the body (rule 1.4): parallelizing saves single-digit milliseconds of body-parse time, and auth-before-parse has a mild unauthenticated-DoS rationale; not worth the churn.useGalleryImage), parallel server fetches, React-Compiler-covered render paths. Likewise the new provider OG card routes (static lookups hoisted,Promise.all'd data), the apps/code formatting unification (module-level formatters,next/dynamiccharts), and apps/docs (only content changed since the last audit; the AI search panel is already lazily imported).Verification
turbo run build --filter=ui:✓ Compiled successfully, andpnpm exec tsc --noEmitinapps/uipasses. The build's static-export stage fails in this sandbox on/compare/litellm/opengraph-image(SELF_SIGNED_CERT_IN_CHAIN— the TLS-intercepting proxy breaks that route's outbound fetch). This is environmental and pre-existing: the identical failure occurs on this sandbox before any of these changes, and was documented in the previous audit rounds.pnpm buildfor the remaining workspaces (shared, models, playground, code, docs, admin, api, gateway) passes..next/static/chunksfor catalogue markers and summing the chunks referenced by each route's client-reference manifest, before vs. after.pnpm formatclean; lint-staged (eslint + prettier) passed on every commit;image-download.spec.ts(5 tests) passes.Note: this session pushes to its designated branch (
claude/upbeat-johnson-3b76et) rather than thechore/react-bp-audit-2026-09-21naming convention — same situation as previous audit rounds (#3518/#3784/#3865/#3966); future audits should locate this PR by title.🤖 Generated with Claude Code
https://claude.ai/code/session_01CepaNxrPkU6gcEXT23M7WL
Generated by Claude Code
Summary by CodeRabbit
New Features
Improvements