Repository navigation
perf: weekly react best practices audit fixes - #3518
Merged
Merged
Conversation
Weekly React/Next.js best-practices audit across the four Next.js apps (ui, code, playground, docs), per Vercel's react-best-practices guide: parallelize independent server fetches, dedupe per-request fetches with React.cache, defer heavy client bundles with dynamic imports, and fix render-purity/state-initialization issues. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012AXwZZQXjD9sephujzkJQ9
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe PR adds typed request-scoped API helpers, parallelizes independent requests, defers selected client modules, stabilizes QueryClient and PostHog instances, and updates documentation feedback, AI search, markdown caching, and UI behavior. ChangesServer API and dashboard data loading
Concurrent application data loading
Client runtime and bundle loading
Documentation rendering and feedback
UI behavior adjustments
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (1)
apps/docs/components/markdown.tsx (1)
132-143: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winUse LRU ordering for the bounded cache.
Cache hits at Line 132 do not refresh insertion order. During streaming, new snapshots are added while existing messages are rendered again. After 200 entries, frequently rendered older messages can be evicted and processed again on later renders. Move cache hits to the end before eviction, or use an LRU cache, so eviction removes cold snapshots.
Proposed cache-hit update
let result = cache.get(text); -if (!result) { +if (result) { + cache.delete(text); + cache.set(text, result); +} else { result = processor.process(text); if (cache.size >= cacheLimit) { const oldest = cache.keys().next().value;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/docs/components/markdown.tsx` around lines 132 - 143, Update the cache lookup around result and processor.process so cache hits refresh the entry’s insertion order before eviction. Remove and reinsert an existing text key, while preserving the current bounded-cache behavior so the oldest key is evicted when the limit is reached and cold snapshots are removed first.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/code/src/components/profile/ProfileView.tsx`:
- Around line 30-38: Remove the dynamic import in ProfileView.tsx at lines 30-38
and restore a top-level ProfileTokensChart import. Also remove the dynamic
import in markdown.tsx at lines 1-7 and restore a top-level SyntaxHighlightedPre
import; no direct change is needed elsewhere. Ensure neither file contains
next/dynamic or import() calls.
In `@apps/docs/components/ai/search.tsx`:
- Line 14: Replace the dynamic markdown import used by the next/dynamic
configuration around the Markdown component with a top-level import from
../markdown, then update the component usage to reference that static import.
Remove the now-unnecessary dynamic import so the file complies with the
repository’s TypeScript import guidelines.
In `@apps/docs/components/feedback.tsx`:
- Around line 82-83: Update the onRateAction prop contract in the feedback
component to accept only url, and call onRateAction with url alone instead of
feedback. Verify all onRateAction references in feedback.tsx and the page module
use this same single-argument contract.
In `@apps/playground/src/components/ai-elements/code-block.tsx`:
- Around line 150-157: Replace the lazy import in the code block highlighter
initialization with a top-level static Shiki import, while preserving the
existing createHighlighter configuration and highlighterPromise behavior;
alternatively, obtain an explicit exception for this file before merging.
In `@apps/ui/src/app/blog/page.tsx`:
- Line 17: Update the draft filter callback in the blog page to replace the
explicit any type with the generated content-collection entry type or an
inferred entry type. Preserve the existing !entry?.draft filtering behavior and
avoid introducing any additional any usage.
In `@apps/ui/src/app/dashboard/`[orgId]/org/guardrails/guardrails-client.tsx:
- Around line 156-163: Add before-and-after screenshots for the affected
dashboard guardrails screen in both light and dark themes where applicable, and
attach them to the pull request. No code changes are required around the
Promise.all requests.
- Around line 156-163: Replace the Promise.all coordination in the guardrails
data-loading flow with Promise.allSettled or equivalent independent request
handling so each fulfilled response is applied separately. Preserve a successful
configuration response when the rules request fails, and add a test covering
that configuration-success/rules-rejection case.
In `@apps/ui/src/components/providers.tsx`:
- Around line 20-25: Update the PR to include before-and-after screenshots for
the affected dashboard screens impacted by the global ChatSupport change,
covering both light and dark themes where applicable. Follow the repository’s
established screenshot conventions and ensure the screenshots are included with
the change.
- Line 5: Remove the next/dynamic import and the import() expression from the
provider setup, and restore a top-level ChatSupport import in the relevant
component flow. Update the ChatSupport usage to reference that static import
while preserving its existing rendering behavior.
In `@apps/ui/src/lib/server-api.ts`:
- Around line 116-125: Update getOrgProjects to pass the explicit
fetchServerData generic for the organization-project response envelope, using
the existing Project type and the { projects: Project[] } shape. Preserve the
current request path and parameters so callers receive the typed result without
unchecked assertions.
---
Nitpick comments:
In `@apps/docs/components/markdown.tsx`:
- Around line 132-143: Update the cache lookup around result and
processor.process so cache hits refresh the entry’s insertion order before
eviction. Remove and reinsert an existing text key, while preserving the current
bounded-cache behavior so the oldest key is evicted when the limit is reached
and cold snapshots are removed first.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 07ebfdc0-d592-4ebd-85d8-a243d78b4f01
📒 Files selected for processing (29)
apps/code/next.config.tsapps/code/src/components/profile/ProfileView.tsxapps/code/src/components/providers.tsxapps/code/src/lib/utils/markdown.tsxapps/docs/app/(home)/[[...slug]]/page.tsxapps/docs/components/ai/search.tsxapps/docs/components/feedback.tsxapps/docs/components/markdown.tsxapps/playground/src/app/group/page.tsxapps/playground/src/app/playground-shell.tsxapps/playground/src/app/posthog.tsapps/playground/src/components/ai-elements/code-block.tsxapps/playground/src/components/providers.tsxapps/playground/src/hooks/useUser.tsapps/playground/src/lib/providers.tsxapps/ui/src/app/blog/page.tsxapps/ui/src/app/changelog/page.tsxapps/ui/src/app/dashboard/[orgId]/[projectId]/layout.tsxapps/ui/src/app/dashboard/[orgId]/[projectId]/page.tsxapps/ui/src/app/dashboard/[orgId]/[projectId]/settings/preferences/_components/archive-project.tsxapps/ui/src/app/dashboard/[orgId]/[projectId]/settings/preferences/_components/caching-settings-rsc.tsxapps/ui/src/app/dashboard/[orgId]/[projectId]/settings/preferences/_components/project-mode-settings-rsc.tsxapps/ui/src/app/dashboard/[orgId]/[projectId]/settings/preferences/_components/project-name-settings-rsc.tsxapps/ui/src/app/dashboard/[orgId]/layout.tsxapps/ui/src/app/dashboard/[orgId]/org/guardrails/guardrails-client.tsxapps/ui/src/components/enterprise/calendly-inline.tsxapps/ui/src/components/landing/navbar.tsxapps/ui/src/components/providers.tsxapps/ui/src/lib/server-api.ts
Comment on lines
+30
to
+38
| // The tokens chart pulls in recharts and renders below the fold, so keep it | ||
| // out of the profile page's initial bundle. | ||
| const ProfileTokensChart = dynamic( | ||
| () => | ||
| import("@/components/profile/ProfileTokensChart").then( | ||
| (mod) => mod.ProfileTokensChart, | ||
| ), | ||
| { ssr: false, loading: () => <div className="h-52 w-full" /> }, | ||
| ); |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Resolve the shared dynamic-import policy violation.
Both changed files use dynamic imports despite the repository rule that forbids them.
apps/code/src/components/profile/ProfileView.tsx#L30-L38: restore the top-levelProfileTokensChartimport, or approve a documented exception.apps/code/src/lib/utils/markdown.tsx#L1-L7: restore the top-levelSyntaxHighlightedPreimport, or approve a documented exception.
Verify the changed files with:
#!/bin/bash
set -euo pipefail
if rg -n 'next/dynamic|import\s*\(' \
apps/code/src/components/profile/ProfileView.tsx \
apps/code/src/lib/utils/markdown.tsx
then
echo "Dynamic imports remain in files covered by the prohibition." >&2
exit 1
fiAs per coding guidelines: use top-level import statements and never use dynamic imports.
📍 Affects 2 files
apps/code/src/components/profile/ProfileView.tsx#L30-L38(this comment)apps/code/src/lib/utils/markdown.tsx#L1-L7
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/code/src/components/profile/ProfileView.tsx` around lines 30 - 38,
Remove the dynamic import in ProfileView.tsx at lines 30-38 and restore a
top-level ProfileTokensChart import. Also remove the dynamic import in
markdown.tsx at lines 1-7 and restore a top-level SyntaxHighlightedPre import;
no direct change is needed elsewhere. Ensure neither file contains next/dynamic
or import() calls.
Comment on lines
+150
to
+157
| // Import shiki lazily so its engine and grammar registry stay out of the | ||
| // chunk until a code block actually renders. | ||
| const highlighterPromise = import("shiki").then(({ createHighlighter }) => | ||
| createHighlighter({ | ||
| langs: [language], | ||
| themes: ["github-light", "github-dark"], | ||
| }), | ||
| ); |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Repository files matching code-block.tsx:"
fd -a 'code-block\.tsx$' . || true
file="$(fd 'code-block\.tsx$' . | head -n 1 || true)"
if [ -n "${file:-}" ]; then
echo
echo "File: $file"
wc -l "$file"
echo
echo "Top-level import statements and dynamic imports:"
sed -n '1,220p' "$file" | nl -ba | sed -n '1,220p'
fi
echo
echo "Search for dynamic import pattern in ts/tsx files (excluding node_modules/dist):"
rg -n --glob '!node_modules/**' --glob '!dist/**' --glob '!build/**' '\bimport\s*\([^)]*\)' . || trueRepository: theopenco/llmgateway
Length of output: 1929
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Repository files matching code-block.tsx:"
fd -a 'code-block\.tsx$' . || true
file="$(fd 'code-block\.tsx$' . | head -n 1 || true)"
if [ -n "${file:-}" ]; then
echo
echo "File: $file"
wc -l "$file"
echo
echo "Top-level import statements and dynamic imports:"
sed -n '1,220p' "$file" | nl -ba | sed -n '1,220p'
fi
echo
echo "Search for dynamic import pattern in ts/tsx files (excluding node_modules/dist):"
rg -n --glob '!node_modules/**' --glob '!dist/**' --glob '!build/**' '\bimport\s*\([^)]*\)' . || trueRepository: theopenco/llmgateway
Length of output: 1929
Resolve the dynamic-import guideline violation.
import("shiki") violates the **/*.{ts,tsx} guideline against dynamic imports. Use a top-level Shiki import instead, or get an explicit exception for this path before merge.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/playground/src/components/ai-elements/code-block.tsx` around lines 150 -
157, Replace the lazy import in the code block highlighter initialization with a
top-level static Shiki import, while preserving the existing createHighlighter
configuration and highlighterPromise behavior; alternatively, obtain an explicit
exception for this file before merging.
Source: Coding guidelines
|
|
||
| import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; | ||
| import { ReactQueryDevtools } from "@tanstack/react-query-devtools"; | ||
| import dynamic from "next/dynamic"; |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Remove the dynamic import or obtain an approved exception.
This TypeScript file uses next/dynamic at Line 5 and an import() expression at Line 23. Restore a top-level ChatSupport import unless maintainers approve an exception for this performance change.
Policy-compliant alternative
-import dynamic from "next/dynamic";
+import { ChatSupport } from "`@/components/chat-support`";
...
-const ChatSupport = dynamic(
- () => import("`@/components/chat-support`").then((mod) => mod.ChatSupport),
- { ssr: false },
-);#!/usr/bin/env bash
set -euo pipefail
if rg -n '"next/dynamic"|import\s*\(' apps/ui/src/components/providers.tsx; then
echo "Dynamic import remains."
exit 1
fiAs per coding guidelines, **/*.{ts,tsx} files must use top-level import statements and never use require or dynamic imports.
Also applies to: 20-25
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/ui/src/components/providers.tsx` at line 5, Remove the next/dynamic
import and the import() expression from the provider setup, and restore a
top-level ChatSupport import in the relevant component flow. Update the
ChatSupport usage to reference that static import while preserving its existing
rendering behavior.
Comment on lines
+20
to
+25
| // The support widget starts collapsed but statically pulls in the AI SDK and | ||
| // streamdown/shiki, so defer it out of the initial bundle of every route. | ||
| const ChatSupport = dynamic( | ||
| () => import("@/components/chat-support").then((mod) => mod.ChatSupport), | ||
| { ssr: false }, | ||
| ); |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add the required dashboard screenshots.
This change modifies the global ChatSupport UI in the apps/ui provider. Add before/after screenshots for affected dashboard screens, including both themes where applicable.
As per coding guidelines, apps/{ui,code}/**/*.{ts,tsx} dashboard UI changes require before/after screenshots, including both themes where applicable. The PR objectives state that no screenshots were added.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/ui/src/components/providers.tsx` around lines 20 - 25, Update the PR to
include before-and-after screenshots for the affected dashboard screens impacted
by the global ChatSupport change, covering both light and dark themes where
applicable. Follow the repository’s established screenshot conventions and
ensure the screenshots are included with the change.
Source: Coding guidelines
Member
Author
|
Addressed the CodeRabbit review in 479877d: Fixed
Declined, with reasoning
Generated by Claude Code |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/docs/components/feedback.tsx (1)
82-89: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winWait for the rate action before resetting the submission pending state.
The
startTransitioncallback returns immediately becauseonRateAction(url)is not awaited. This can setisPendingback tofalsebefore the call completes. AwaitonRateActioninside the transition, guard state updates afterawaitwithstartTransition, and handle failures so the form can be resubmitted.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/docs/components/feedback.tsx` around lines 82 - 89, Update the submission transition around onRateAction to await the rate action before completing, then wrap post-await updates such as localStorage persistence and replacePrevious in startTransition. Add failure handling that restores the form’s resubmittable state when onRateAction rejects, while preserving the existing success result construction.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/ui/src/app/dashboard/`[orgId]/org/guardrails/guardrails-client.tsx:
- Around line 154-155: Update the fetch startup logic in fetchConfig to call
setError(null) alongside setIsLoading(true), clearing any stale load error
before the current request begins.
- Around line 153-157: Replace the useEffect-driven fetchConfig flow with
separate TanStack Query hooks for the guardrails configuration and rules
requests. Use each query’s data and loading state independently, and pass
canManageGuardrails as the enabled option to both queries so users without
organization management access skip both requests.
- Around line 166-185: Update the config and rules result handling in the
guardrails loading flow to inspect each fulfilled response’s error field before
applying DEFAULT_CONFIG or updating custom rules. Treat any HTTP error as a load
failure by setting the existing error state, while preserving default config
only for successful responses with no config data.
- Around line 157-187: Update fetchConfig to create a request-bound abort
controller or load token and associate it with the organizationId used for that
Promise.allSettled request. Before applying config, rules, errors, or
setIsLoading(false), verify the request is still current; ignore all completions
from earlier organization loads so they cannot overwrite active state or end the
newer loading cycle.
---
Outside diff comments:
In `@apps/docs/components/feedback.tsx`:
- Around line 82-89: Update the submission transition around onRateAction to
await the rate action before completing, then wrap post-await updates such as
localStorage persistence and replacePrevious in startTransition. Add failure
handling that restores the form’s resubmittable state when onRateAction rejects,
while preserving the existing success result construction.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d62cbd39-2939-4810-aa7a-f5f5b32a4da2
📒 Files selected for processing (5)
apps/docs/components/feedback.tsxapps/docs/components/markdown.tsxapps/ui/src/app/blog/page.tsxapps/ui/src/app/dashboard/[orgId]/org/guardrails/guardrails-client.tsxapps/ui/src/lib/server-api.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/ui/src/app/blog/page.tsx
- apps/ui/src/lib/server-api.ts
- apps/docs/components/markdown.tsx
Comment on lines
153
to
+157
| const fetchConfig = useCallback(async () => { | ||
| try { | ||
| setIsLoading(true); | ||
| const response = await fetchClient.GET( | ||
| "/guardrails/config/{organizationId}", | ||
| { | ||
| params: { path: { organizationId } }, | ||
| }, | ||
| ); | ||
| setIsLoading(true); | ||
| // allSettled so a fulfilled response is still applied when the other | ||
| // request fails. | ||
| const [configResult, rulesResult] = await Promise.allSettled([ |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 6 '\b(useQuery|useQueries)\b' apps/ui/src
rg -n -C 6 'useEffect|fetchConfig|useFetchClient' \
'apps/ui/src/app/dashboard/[orgId]/org/guardrails/guardrails-client.tsx'Repository: theopenco/llmgateway
Length of output: 50376
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "Guardrails file relevant section:"
sed -n '120,215p' 'apps/ui/src/app/dashboard/[orgId]/org/guardrails/guardrails-client.tsx' | cat -n
echo
echo "React Query dependency versions:"
for f in package.json apps/ui/package.json; do
if [ -f "$f" ]; then
echo "--- $f"
node -e "const p=require('./$f'); console.log(JSON.stringify({dependencies:p.dependencies,devDependencies:p.devDependencies||{}},null,2))" | rg -C 2 'react-query|`@tanstack`|package-lock|pnpm|yarn' || true
fi
done
echo
echo "Typical nearby query usage with organization params:"
rg -n -C 4 'api\.useQuery\(' 'apps/ui/src/components/sso/sso-client.tsx' 'apps/ui/src/components/custom-models/org-models-client.tsx' 'apps/ui/src/lib/dashboard-state.ts' | sed -n '1,180p'Repository: theopenco/llmgateway
Length of output: 9235
Move the guardrails loads into TanStack Query.
This component still calls fetchConfig from a useEffect, but generated API calls here should use the query hooks. Keep the configuration and rules loads separate, and drive canManageGuardrails from the query enabled option so disabled organization users skip both requests.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/ui/src/app/dashboard/`[orgId]/org/guardrails/guardrails-client.tsx
around lines 153 - 157, Replace the useEffect-driven fetchConfig flow with
separate TanStack Query hooks for the guardrails configuration and rules
requests. Use each query’s data and loading state independently, and pass
canManageGuardrails as the enabled option to both queries so users without
organization management access skip both requests.
Source: Coding guidelines
Comment on lines
+157
to
+187
| const [configResult, rulesResult] = await Promise.allSettled([ | ||
| fetchClient.GET("/guardrails/config/{organizationId}", { | ||
| params: { path: { organizationId } }, | ||
| }), | ||
| fetchClient.GET("/guardrails/rules/{organizationId}", { | ||
| params: { path: { organizationId } }, | ||
| }), | ||
| ]); | ||
|
|
||
| if (response.data) { | ||
| setConfig(response.data as unknown as GuardrailConfig); | ||
| if (configResult.status === "fulfilled") { | ||
| if (configResult.value.data) { | ||
| setConfig(configResult.value.data as unknown as GuardrailConfig); | ||
| } else { | ||
| // No config exists yet, use defaults | ||
| setConfig(DEFAULT_CONFIG); | ||
| } | ||
| } | ||
|
|
||
| const rulesResponse = await fetchClient.GET( | ||
| "/guardrails/rules/{organizationId}", | ||
| { | ||
| params: { path: { organizationId } }, | ||
| }, | ||
| if (rulesResult.status === "fulfilled" && rulesResult.value.data) { | ||
| setCustomRules( | ||
| (rulesResult.value.data as { rules: CustomRule[] }).rules || [], | ||
| ); | ||
| } | ||
|
|
||
| if (rulesResponse.data) { | ||
| setCustomRules( | ||
| (rulesResponse.data as { rules: CustomRule[] }).rules || [], | ||
| ); | ||
| } | ||
| } catch { | ||
| if ( | ||
| configResult.status === "rejected" || | ||
| rulesResult.status === "rejected" | ||
| ) { | ||
| setError("Failed to load guardrails configuration"); | ||
| } finally { | ||
| setIsLoading(false); | ||
| } | ||
| setIsLoading(false); |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 8 'GuardrailsClient|useParams\(\)|org/guardrails|router\.(push|replace)' \
apps/ui/src/app/dashboardRepository: theopenco/llmgateway
Length of output: 27196
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== guardrails-client outline =="
ast-grep outline apps/ui/src/app/dashboard/[orgId]/org/guardrails/guardrails-client.tsx || true
echo "== guardrails-client relevant lines =="
sed -n '1,260p' apps/ui/src/app/dashboard/[orgId]/org/guardrails/guardrails-client.tsx | cat -n
echo "== search for guardrails route usage =="
rg -n -C 4 'org/guardrails|guardrails|GuardrailsClient' apps/ui/srcRepository: theopenco/llmgateway
Length of output: 50379
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== dashboard sidebar route definitions =="
sed -n '180,230p' apps/ui/src/components/dashboard/dashboard-sidebar.tsx | cat -n
echo "== navigation hook relevant files =="
rg -n -C 3 "buildOrgUrl|organizationId|orgId|selectedOrganization" apps/ui/src/hooks apps/ui/src/lib | head -n 200
echo "== next router use in org settings/overview-ish files =="
sed -n '1,120p' apps/ui/src/app/dashboard/[orgId]/org/security-events/security-events-client.tsx | cat -n
sed -n '1,120p' apps/ui/src/app/dashboard/[orgId]/org/audit-logs/audit-logs-client.tsx | cat -nRepository: theopenco/llmgateway
Length of output: 21223
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== locate dashboard context/organization switchers =="
rg -n -C 6 "setSelectedOrganization|selectedOrganization|useOrganization|switch|change|organizationId" apps/ui/src/hooks apps/ui/src/lib apps/ui/src/app/dashboard --glob '*.ts' --glob '*.tsx' | head -n 260
echo "== focused files likely organization switcher =="
fd -e ts -e tsx . apps/ui/src/app/apps/ui/src/hooks apps/ui/src/lib | rg 'organization|switch|context|context|navigation|settings' || true
echo "== outline organization switcher candidates =="
for f in $(rg -l "setSelectedOrganization|selectedOrganization|switch" apps/ui/src/hooks apps/ui/src/lib apps/ui/src/app/dashboard --glob '*.ts' --glob '*.tsx' | head -n 20); do
echo "--- $f"
ast-grep outline "$f" || true
doneRepository: theopenco/llmgateway
Length of output: 27451
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== dashboard context/state full files =="
sed -n '1,220p' apps/ui/src/lib/dashboard-context.tsx | cat -n
sed -n '1,220p' apps/ui/src/lib/dashboard-state.ts | cat -n
echo "== organization select/usages =="
rg -n -C 5 "handleOrganizationSelect|organizationSelected|useOrganization|setSelectedOrganization|selectedOrganization" apps/ui/src --glob '*.ts' --glob '*.tsx' | head -n 240
echo "== all router calls in dashboard under orgId path =="
rg -n -C 4 "router\.(push|replace|back|forward)|useRouter|router\." apps/ui/src/app/dashboard --glob '*.tsx' --glob '*.ts' | head -n 260Repository: theopenco/llmgateway
Length of output: 45021
Use request-bound guards for guardrails fetch completions.
fetchConfig reads organizationId from useParams() inside the callback, but the pending Promise.allSettled() was created with the value captured at call time. Route changes can invoke a new fetchConfig before the first load finishes; store an abort controller/token per load and ignore responses belonging to an earlier organization, otherwise older guardrails config/rules can overwrite the active org state and setIsLoading(false) can close the newer loading state.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/ui/src/app/dashboard/`[orgId]/org/guardrails/guardrails-client.tsx
around lines 157 - 187, Update fetchConfig to create a request-bound abort
controller or load token and associate it with the organizationId used for that
Promise.allSettled request. Before applying config, rules, errors, or
setIsLoading(false), verify the request is still current; ignore all completions
from earlier organization loads so they cannot overwrite active state or end the
newer loading cycle.
Member
Author
|
Second review round: took the stale-error reset ( Declining the remaining guardrails/feedback suggestions — each targets behavior that predates this PR and was intentionally preserved by this perf pass: the original sequential code had the same treatment of non-2xx config responses (defaults applied), the same org-switch race, no request-bound abort tokens, and the same non-awaited Generated by Claude Code |
Correctness:
- server-api: share the in-flight promise instead of React.cache(), which
memoized fetchServerData's null-on-error and turned one transient blip
into a request-wide "unauthorized" render
- guardrails: treat openapi-fetch's { error } as a failure — an HTTP error
silently installed DEFAULT_CONFIG and the next save overwrote the org's
real configuration
- docs markdown: pin cache entries a mounted Renderer still needs so
eviction can't flash an answered message back to the fallback
- docs page: restore the lastUpdate fallback; the unauthenticated GitHub
lookup fails for most pages and dropped "Last updated" entirely
- docs feedback: commit the UI before persisting, and make localStorage
best-effort so a storage failure can't swallow the confirmation
- docs providers: bound the idle deferral so ratings captured early are
not dropped before posthog.init()
- playground: queue posthog.identify until init completes instead of
firing once and leaving the session anonymous
- code-block: drop a rejected shiki import from the cache so one chunk
failure doesn't permanently disable highlighting
- code: drop optimizePackageImports on the "use client" shared barrel, and
build the posthog options inline so a discarded memo can't cancel init
Cleanup:
- ui: keep the support trigger in the server HTML (no ssr: false)
- ui: reuse one posthog-node client per process
- ui: route the remaining org/project fetches through the shared helpers
- docs: split Ask AI so the AI SDK and markdown pipeline load on open
Claude-Session: https://claude.ai/code/session_01BjEeYVaMqJDgkLgz1pohGM
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/playground/src/hooks/useUser.ts (1)
42-52: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winClear queued PostHog identity on logout.
identifyUser()saves apendingidentity before PostHog loads.useUser.tsonly queues it whendata?.useris present and does not clear it when the session becomes unauthenticated, whileposthog.reset()does not clearflushPendingIdentity()’s shared pending state. AddclearPendingIdentity()inapps/playground/src/lib/posthog-identity.ts, call it whendata?.useris absent, and ensure the logout paths clear it as well.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/playground/src/hooks/useUser.ts` around lines 42 - 52, Update the useUser effect around identifyUser so it calls clearPendingIdentity when data?.user is absent, while preserving the existing identification flow for authenticated users. Add the clearPendingIdentity helper in posthog-identity.ts to remove the shared pending identity, and invoke it in every logout path alongside posthog.reset().
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/docs/app/`(home)/[[...slug]]/page.tsx:
- Around line 92-96: Update the lastUpdate assignment in the page data flow to
preserve undefined when getGithubLastEdit returns no time, rather than falling
back to new Date(). Keep the existing Date conversion when time is available so
the “Last updated” label reflects only GitHub edit data.
In `@apps/docs/components/ai/ask-ai.tsx`:
- Around line 13-15: Replace the dynamic AISearchPanel definition using
next/dynamic and import("./search") with a top-level static import of
AISearchPanel from "./search"; preserve the component’s existing usage and
remove the unnecessary loading configuration.
In `@apps/docs/components/markdown.tsx`:
- Around line 150-173: Update Renderer’s cache eviction flow to exclude the
current text key from evictColdEntries, preventing the newly created entry from
being removed before useEffect pins it. After the cleanup returned by the
pinning useEffect decrements or removes a pinned entry, run eviction again so
previously protected entries can be reclaimed.
---
Outside diff comments:
In `@apps/playground/src/hooks/useUser.ts`:
- Around line 42-52: Update the useUser effect around identifyUser so it calls
clearPendingIdentity when data?.user is absent, while preserving the existing
identification flow for authenticated users. Add the clearPendingIdentity helper
in posthog-identity.ts to remove the shared pending identity, and invoke it in
every logout path alongside posthog.reset().
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a4d5776c-6d60-4281-9e3a-d6329b4af94b
📒 Files selected for processing (18)
apps/code/src/components/providers.tsxapps/docs/app/(home)/[[...slug]]/page.tsxapps/docs/app/(home)/layout.tsxapps/docs/components/ai/ask-ai.tsxapps/docs/components/ai/search.tsxapps/docs/components/feedback.tsxapps/docs/components/markdown.tsxapps/docs/lib/providers.tsxapps/playground/src/components/ai-elements/code-block.tsxapps/playground/src/components/providers.tsxapps/playground/src/hooks/useUser.tsapps/playground/src/lib/posthog-identity.tsapps/ui/src/app/dashboard/[orgId]/[projectId]/settings/sdk/page.tsxapps/ui/src/app/dashboard/[orgId]/org/guardrails/guardrails-client.tsxapps/ui/src/app/dashboard/[orgId]/page.tsxapps/ui/src/app/posthog.tsapps/ui/src/components/providers.tsxapps/ui/src/lib/server-api.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/playground/src/components/ai-elements/code-block.tsx
- apps/docs/components/feedback.tsx
- apps/ui/src/app/dashboard/[orgId]/org/guardrails/guardrails-client.tsx
Comment on lines
+13
to
+15
| const AISearchPanel = dynamic(() => import("./search"), { | ||
| loading: () => null, | ||
| }); |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Remove the disallowed dynamic import.
Line 13 invokes import("./search") through next/dynamic. The TypeScript policy prohibits dynamic imports. Use a top-level AISearchPanel import. If deferred loading is required, obtain a scoped policy exception before merge.
Proposed policy-compliant change
-import dynamic from "next/dynamic";
+import AISearchPanel from "./search";
...
-const AISearchPanel = dynamic(() => import("./search"), {
- loading: () => null,
-});🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/docs/components/ai/ask-ai.tsx` around lines 13 - 15, Replace the dynamic
AISearchPanel definition using next/dynamic and import("./search") with a
top-level static import of AISearchPanel from "./search"; preserve the
component’s existing usage and remove the unnecessary loading configuration.
Source: Coding guidelines
…to claude/upbeat-johnson-77n54b
Member
Author
|
Latest review round, on top of 2373c5a: took two findings in d2b9d9b — the markdown cache now shields the current entry from eviction until the pinning effect runs (and re-evicts when a pin releases), and Skipped two: the Generated by Claude Code |
Stamping the build date claimed a page had been edited when it had not, and it moved on every deploy. fumadocs omits the line when undefined, so "Last updated" now only ever reflects real GitHub edit data. Claude-Session: https://claude.ai/code/session_01BjEeYVaMqJDgkLgz1pohGM
This was referenced Aug 16, 2026
smakosh
added a commit
that referenced
this pull request
Aug 24, 2026
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](https://github.com/vercel-labs/agent-skills/blob/main/skills/react-best-practices/AGENTS.md). The three previous audits (#3391, #3518, #3647) are all merged, so this week covers code merged since then plus remaining findings, including two items those audits explicitly deferred that turned out to have surgical fixes. No routing changes, no user-visible behavior changes, nothing touching `export const dynamic`. ## Fixes ### apps/ui - **Prevent Hydration Mismatch Without Flickering (rule 6.5)** — `lib/components/sidebar.tsx`, `app/dashboard/[orgId]/layout.tsx`. The sidebar open state was read from localStorage in a post-mount effect (behind a `mounted` flag), so every dashboard load rendered the default state and then snapped to the persisted one — a layout shift on every navigation for anyone with a collapsed sidebar. Deferred by the last two audits as needing prop threading; it doesn't: the provider's only mount point is the org server layout, which already reads cookies. The state now persists in a `sidebar_state` cookie (the pattern the playground's sidebar already uses) and the layout passes it as `defaultOpen`, so the first paint is correct and both effects plus the `mounted` state are gone. One-time migration cost: a previously saved localStorage value is ignored, so a collapsed sidebar renders expanded once until the user toggles again. - **Per-Request Deduplication with React.cache() (rule 3.9)** — `app/dashboard/page.tsx`. The dashboard entry page fetched `/user/me`, `/orgs`, and `/orgs/{id}/projects` through the raw fetcher while `dashboard/layout.tsx` fetches `/user/me` through the deduped `getUserMe()` in the same render pass — a duplicate round-trip on every `/dashboard` hit. All three now go through the existing `cache()`-backed helpers, which also lets the redirect target's org layout share them. - **Dynamic Imports for Heavy Components (rule 2.4)** — `lib/utils/markdown.tsx`. The prism-based `SyntaxHighlightedPre` (prism-react-renderer plus its full `themes` barrel) was statically wired into the markdown options used by the blog, guides, changelog, legal, use-cases, and migration routes, shipping the highlighter on articles with zero code blocks. Now `next/dynamic`, mirroring the identical fix `apps/code` got in #3518. - **Cache Repeated Function Calls / Hoist Constructors (rules 7.4/7.10)** — `components/api-keys/api-keys-list.tsx`, `api-key-limit-fields.tsx`, `api-key-ttl-fields.tsx`, `components/master-keys/master-keys-list.tsx`, `lib/components/number-ticker.tsx`. The API-keys table constructed 3–4 `Intl.DateTimeFormat` instances per row per render (creation date, tooltip, expiry, period reset), the master-keys list the same, and `NumberTicker` constructed one per spring animation frame (×3 tickers on pages using it). All formatters are hoisted to module scope (or, for the ticker, created once per subscription). ### apps/code - **Minimize Serialization at RSC Boundaries / bundle size (rules 3.6, 2.x)** — new `lib/coding-models.ts`, `components/CodingModelsShowcase.tsx`, `app/page.tsx`, `app/coding-models/page.tsx`. The showcase — rendered on the landing page and `/coding-models` — imported the entire `@llmgateway/models` catalogue (~800K of source: every model with all provider mappings) into a client component, then derived a few fields per coding model in the browser. Flagged as the app's biggest bundle finding in #3518 and deferred twice as "needs an interface redesign"; the redesign is small. All derivation (DevPass coding gate, recommended/premium sets, cheapest-provider pricing) now runs server-side and only a trimmed card array (8 scalar fields per model) crosses the RSC boundary. The client keeps just the tab state and copy button; rendered UI is unchanged. - **NumberTicker per-frame `Intl.NumberFormat`** — `components/ui/number-ticker.tsx`, same fix as the ui copy above. ### apps/playground - **Promise.all() for Independent Operations (rule 1.5)** — `app/realtime/page.tsx`. Realtime was the only media page still awaiting `/orgs/{id}/projects` serially after models/providers/orgs; the siblings (`image`, `video`, `audio`, `canvas`) all start it eagerly in the same `Promise.all` when the URL carries an `orgId`. Realtime now does the same, with the eager result used only when that org actually ends up selected. - **Defer Non-Critical Third-Party Libraries (rule 2.3)** — `lib/stripe.ts`, `components/credits/top-up-credits-dialog.tsx`. `useStripe()` loaded Stripe.js (~200KB, phones home on load) in an unconditional mount effect, and the top-up dialog mounts closed on the chat, image, video, and audio pages — so every playground visit fetched Stripe. Ported the `enabled` gate `apps/code` got in #3647; the dialog passes its `open` state, so Stripe.js loads only when the dialog is actually opened. - **Bounded module cache (rule 4.4-adjacent)** — `components/ai-elements/code-block.tsx`. The shiki `tokensCache` retained the full `ThemedToken[][]` of every code block ever rendered (including one entry per streaming snapshot) for the tab's lifetime; its sibling caches were already bounded/cleaned. Now LRU-capped at 200 entries. ### apps/docs - **Defer Await Until Needed (rule 1.2)** — `app/api/chat/route.ts`. The Ask-AI FlexSearch index was built eagerly at module scope: importing the route read and indexed the processed text of all ~131 MDX pages even when `DOCS_AI_SUPPORT_CHAT_API_KEY` is unset and the handler always 503s — and the module-scope promise had no rejection handler until the first tool call, so an indexing failure at boot would crash the standalone server as an unhandled rejection. The index is now built lazily and memoized on first search, with failed builds dropped so a transient error doesn't stick. - **Unnecessary `"use client"` on static components** — new `components/tracked-link.tsx`; `components/enterprise-cta.tsx`, `ai-tooling-cards.tsx`, `self-host-cards.tsx`. Three purely presentational card components were client components solely to fire a PostHog click capture — `EnterpriseCTA` renders in the TOC footer of every docs page, so its markup shipped as client JS everywhere. A thin `TrackedLink` client wrapper now owns the capture, and the cards (icons, copy, layout) are server components. - **Bounded module cache** — `components/ai/page-actions.tsx`. The "Copy Markdown" cache stored each copied page's entire raw markdown in an unbounded module `Map`; now capped with oldest-entry eviction, matching the treatment `markdown.tsx` got in #3518. - **Cache Storage API Calls (rule 7.5)** — `components/ai/search.tsx`. The Ask-AI input wrote its draft to localStorage synchronously on every keystroke; now debounced (300ms), flushed/cleared on submit. - **Remove always-missing `useMemo`** — `components/ai/search.tsx`. The context value was memoized on `[chat, open, setOpen]`, but `useChat` returns a fresh object every render, so the memo allocated every time and never hit; removed (React Compiler covers the rest). ## Considered and deliberately skipped - **Anything involving `export const dynamic`** — intentional (runtime env loading); excluded per repo policy. - **docs: `APIPage` in `mdx-components.tsx`** pulls the fumadocs OpenAPI playground into the client manifest of every docs page — the largest remaining docs bundle item, but the app has a single catch-all page route serving both prose and API reference, so splitting it needs a route restructure. Deferred. - **docs: static `posthog-js` import in the root provider** — the init is already idle-deferred; deferring the import itself requires reworking how `PostHogProvider` receives its client instance. Deferred as behavior-sensitive. - **ui: `posthog.identify()` inside `getUser()`** runs on every dashboard layout render (server-side twin of the client issue fixed in #3518/#3647) — relocating identification to the auth path is an analytics-behavior decision, not a perf-only diff. - **ui: security-events page hand-rolls `useEffect` fetching** (same class as the already-deferred audit-logs/routing-config pages) — the right fix is a `useInfiniteQuery` rewrite; too invasive for this pass. - **ui: millisecond-precision `new Date()` in the agents-view query key** defeats its `staleTime` — real, but truncating the window boundary changes the queried range semantics slightly; left for a deliberate change. - **playground: chat route's project retrieval serialized ahead of MCP connects** — parallelizing changes error-ordering on the hottest route; same reasoning as #3518's deferral of that route's auth/body ordering. - **playground: `@streamdown/mermaid` statically registered for every assistant message** — potentially the largest chat-chunk item, but needs bundle analysis to confirm the plugin doesn't lazy-load internally before acting. - **Manual memoization nits** — all four apps run the React Compiler; only issues the compiler cannot fix (effects, module caches, per-frame constructors, RSC boundaries) were touched. ## Verification - `pnpm build`: 15 of 17 workspaces green, including docs, code, and playground. `ui#build` compiles and type-checks, but static export fails on `/compare/litellm/opengraph-image` with `SELF_SIGNED_CERT_IN_CHAIN` — the same pre-existing build-environment artifact documented and reproduced on clean main in #3647 (TLS-intercepting proxy breaking a build-time `next/og` fetch), unrelated to this diff. - `pnpm exec tsc --noEmit` in `apps/ui` passes. - `pnpm format` clean. - No visual changes intended: the sidebar renders in its persisted state without the previous post-hydration snap, and the showcase/dialog/card changes render identical UI — so no before/after screenshots. Note: this session pushes to its designated branch, so the head branch is `claude/upbeat-johnson-mc6akz` rather than the `chore/react-bp-audit-2026-08-24` naming convention (same situation as #3518); future audits should locate this PR by title. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01GQLYmwthWTnfTLViZsfYKM --- _Generated by [Claude Code](https://claude.ai/code/session_01GQLYmwthWTnfTLViZsfYKM)_ <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Updated coding-model showcases with refreshed model details, pricing, context limits, and recommendation badges. - Sidebar preferences now persist across sessions. - Realtime views load projects for the selected workspace more reliably. - Added consistent click tracking for documentation and promotional links. - **Bug Fixes** - Improved documentation search reliability and retry behavior. - Preserved AI search drafts more reliably while typing. - Deferred Stripe loading until the credit top-up dialog opens. - **Performance** - Improved code highlighting, markdown rendering, number formatting, and dashboard data loading. - Added bounded caching to keep documentation and code previews responsive. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude <noreply@anthropic.com>
smakosh
added a commit
that referenced
this pull request
Sep 21, 2026
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](https://github.com/vercel-labs/agent-skills/blob/main/skills/react-best-practices/AGENTS.md). 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 touches `export const dynamic`. ## Fixes ### Bundle size - **Avoid Barrel File Imports (rule 2.1) — `packages/shared` was un-tree-shakeable, shipping the model catalogue on every ui route.** The root `@llmgateway/shared` barrel re-exports from `./components/index.js`, whose graph reaches the models-directory components and through them the full `@llmgateway/models` catalogue. Because the package declared no `sideEffects`, bundlers had to keep every re-export, so any client component importing the barrel for one symbol (the root `providers.tsx` for `TimeZoneProvider`, 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, `/blog` 2002 KB → 891 KB, `/compare/open-router` 1996 KB → 888 KB — catalogue bytes on all of them 976 KB → **0 KB** - `/models` keeps only the 137 KB of catalogue chunks it actually uses (was 976 KB), and the dashboard project page now references 0 catalogue bytes This 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 marketing `Footer` was `"use client"` and imported `listedProviders` from `providers-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. `Footer` is now a server component (it reads `getConfig()` directly; `Newsletter` stays a client island), and the one client consumer — the `AllModels` wrapper — takes `footer` as 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 same `animate-hero-enter` CSS 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 existing `AuthLink` leaves hydrate. All importers are server pages. - **Conditional Module Loading (rules 2.2/2.3) — `apps/playground` zip download.** `image-download.ts` statically imported `fflate`, 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 via `import()` in parallel with the image bytes, matching the repo's existing on-demand `html-to-image`/`jspdf` pattern. ### Hydration correctness - **Prevent Hydration Mismatch (rule 6.5) — `apps/ui` feature-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 - **Put Interaction Logic in Event Handlers (rule 5.8) — `apps/playground` Image Studio.** The Flex service-tier toggle persisted its cookie through a `useEffect` watching 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 - **Anything involving `export const dynamic`** — intentional runtime-env loading; excluded per repo policy (no finding touched one this round). - **Lounge points query serialized behind `/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. - **Playground API routes awaiting `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. - **Stale-selection resets via effects on model switch** (image controls/page, realtime voice — rule 5.1): pre-existing, commented as deliberate, one extra render on a rare interaction. - **The new Image Studio code is otherwise clean** — refcounted off-render preview decoding (`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/dynamic` charts), 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`, and `pnpm exec tsc --noEmit` in `apps/ui` passes. 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. - Full `pnpm build` for the remaining workspaces (shared, models, playground, code, docs, admin, api, gateway) passes. - Bundle numbers above measured by scanning `.next/static/chunks` for catalogue markers and summing the chunks referenced by each route's client-reference manifest, before vs. after. - `pnpm format` clean; lint-staged (eslint + prettier) passed on every commit; `image-download.spec.ts` (5 tests) passes. - No visual changes intended: the compare hero plays the same entrance via CSS, the comparison tables render identical markup server-side, the footer renders identical markup, and the feature demos show the same style of mock data (fixed values instead of random ones). Per repo policy screenshots are only for dashboard UI changes; none of these screens changed appearance. Note: this session pushes to its designated branch (`claude/upbeat-johnson-3b76et`) rather than the `chore/react-bp-audit-2026-09-21` naming 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.com/claude-code) https://claude.ai/code/session_01CepaNxrPkU6gcEXT23M7WL --- _Generated by [Claude Code](https://claude.ai/code/session_01CepaNxrPkU6gcEXT23M7WL)_ <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added the site footer to model listings and category pages, including text, vision, image, video, tools, embeddings, and web-search pages. - Service-tier preferences now persist immediately when changed. - **Improvements** - Updated comparison-page animations for smoother, more efficient rendering while preserving the existing presentation. - Image downloads continue to support ZIP creation with improved loading behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude <noreply@anthropic.com>
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 fixes are surgical — no routing changes, no behavior rewrites, and nothing touchingexport const dynamic. All four apps have the React Compiler enabled, so the audit deliberately skipped manual-memoization rules and focused on waterfalls, bundle size, server-side fetching, and render purity.Fixes
apps/ui
src/lib/server-api.ts, the four preferences RSCs undersettings/preferences/_components/,dashboard/[orgId]/layout.tsx,dashboard/[orgId]/[projectId]/layout.tsx,dashboard/[orgId]/[projectId]/page.tsx. The preferences page rendered four sibling RSCs that each independently fetched/projects/{id}(4 identical round-trips per page load), and/orgs+/orgs/{id}/projectswere re-fetched across nested layouts. Addedcache()-wrappedgetProject/getOrganizations/getOrgProjectshelpers and pointed all call sites at them.src/components/providers.tsx. The support chat widget (AI SDK + streamdown + shiki code plugin, initialized at module scope) was statically imported into the root providers and shipped on every route, including the landing and SEO pages, despite starting collapsed. Now loaded withnext/dynamic+ssr: false.dashboard/[orgId]/org/guardrails/guardrails-client.tsx. The guardrails config and rules GETs were awaited sequentially; they are independent, so they now resolve in parallel.src/components/landing/navbar.tsx. The site-wide navbar scroll listener was non-passive; other listeners in the repo already pass{ passive: true }.src/app/blog/page.tsx,src/app/changelog/page.tsx. Both pages called.sort()directly on the module-scopeallBlogs/allChangelogsarrays shared across requests. Filtering first (which copies) and sorting the copy keeps the shared array untouched.src/components/enterprise/calendly-inline.tsx. Thewindow.Calendlyprobe ran on every render; now a lazy initializer.apps/playground
src/app/playground-shell.tsx. The main entry point awaited/orgs,/chat-plans/status, and the orgId-scoped projects list serially (up to 4 sequential round-trips). They are mutually independent, so they now resolve in onePromise.all. The/playground/chat-orgfetch intentionally stays sequenced after the chat-plan redirect check — it provisions the chat org on demand and must not run for redirected users.src/app/group/page.tsx. Brought/groupin line with its siblings (/image,/video,/audio…), which already collapse models/providers/orgs/projects into one parallel fetch.src/hooks/useUser.ts.posthog.identify()ran in the render body of a hook with ~24 consumers, including chat components that re-render on every streamed token. Moved into auseEffectkeyed on the user identity.src/components/ai-elements/code-block.tsx.createHighlighterfromshikiwas a static top-level import even though highlighting is already async and cached; the engine + grammar registry now load lazily on first code block render.src/app/posthog.ts. A newposthog-nodeclient (with its own queue and flush timer, never shut down) was constructed on every server request across ~11 routes. Now one memoized client per process.apps/code
next.config.ts.@llmgateway/shared/componentsis a 23-lineexport *barrel fronting ~13k lines (model directory, log cards, provider icons) with nosideEffectsfield, and client components import tiny symbols from it. Added it toexperimental.optimizePackageImports(the playground app already uses this flag).src/lib/utils/markdown.tsx: the prism-basedSyntaxHighlightedPrewas statically wired into the/compare/*markdown options although none of the six comparison documents contains a fenced code block; nownext/dynamic.src/components/profile/ProfileView.tsx: recharts (ProfileTokensChart) was in the first-load JS of the shareable/profiles/[username]page while rendering below the fold; nownext/dynamicwith a size-matched placeholder.src/components/providers.tsx(and the same pattern inapps/ui/src/components/providers.tsx,apps/playground/src/lib/providers.tsx,apps/playground/src/components/providers.tsx).QueryClientwas created withuseMemo, whose cache React may discard — silently dropping the entire query cache mid-session. Switched touseStatewith a lazy initializer, per the React Query docs.apps/docs
components/ai/search.tsx. The Ask-AI panel statically imported the whole markdown pipeline (remark + remark-gfm + remark-rehype + hast-util-to-jsx-runtime + shikiDynamicCodeBlock), putting it in the initial bundle of every docs page although it only runs once a chat message renders. Nownext/dynamic.app/(home)/[[...slug]]/page.tsx+components/feedback.tsx. The docs page imported browserposthog-jsinto a server component and calledposthog.capture()inside a"use server"action — a no-op against the uninitialized browser SDK, so docs feedback was never recorded. The capture now happens client-side in the feedback component via the app's existingusePostHog()provider.components/feedback.tsx. localStorage persistence ran in an effect keyed on[previous, url], which on navigation momentarily wrote the previous page's feedback under the new page's key (and flashed "Thank you for your feedback!" on pages the user never rated). Persistence now happens directly in the submit / "Submit Again" handlers.components/markdown.tsx. The streaming markdown renderer cached one parsed tree per intermediate token snapshot in an unbounded module-scopeMapfor the lifetime of the tab. Now capped with oldest-entry eviction.app/(home)/[[...slug]]/page.tsx. When the GitHub last-edit lookup fails,lastUpdatefell back tonew Date()(the request time), claiming every page was edited "just now". Now omitted instead.Considered and deliberately skipped
export const dynamic— intentional and required for runtime env loading; excluded from the audit per constraints.getGithubLastEditalready fetches withcache: "force-cache", so it hits Next's data cache after the first request per path.ThemedImagedownloading both light+dark screenshots — the suggested<picture media="(prefers-color-scheme)">fix would break the class-based manual theme toggle; needs a different approach.useEffectdata fetching — the right fix is a TanStack Query (useInfiniteQuery/useQuery) rewrite of each page's data layer; too invasive for this pass.defaultOpen; needs prop threading through the dashboard layouts, deferred.awaitloops inapi/chat, andgetUser()serialized ahead ofreq.json()across API routes — real wins, but they change error-ordering semantics on the hottest route; better as a dedicated change.CodingModelsShowcaseships the full@llmgateway/modelscatalogue to the client on/and/coding-models— the highest-impact bundle finding in that app, but it requires redesigning the component's server/client interface (deriving the trimmed list server-side); deferred. Note theoptimizePackageImportschange above does not cover this case.number-tickerSSRs0, unconditional 5s dashboard poll, theme-togglemountedgate — all behavior-adjacent; deferred rather than risk user-visible changes in a perf pass.Verification
pnpm build— all 17 tasks pass.pnpm format(via lint-staged eslint --fix + prettier on every changed file).Note: this session pushes to its designated branch, so the head branch is
claude/upbeat-johnson-77n54brather than thechore/react-bp-audit-2026-08-10naming convention; future audits should locate this PR by title.🤖 Generated with Claude Code
https://claude.ai/code/session_012AXwZZQXjD9sephujzkJQ9
Generated by Claude Code
Summary by CodeRabbit
New Features
Performance
Bug Fixes