Conversation
ogzhanolguncu
requested review from
chronark,
mcstepp and
perkinsjr
as code owners
|
📝 WalkthroughWalkthroughThis update refactors the rate limits dashboard to shift all data fetching from server-side to client-side using TRPC queries. Several page and component props are simplified to use only IDs, with components now responsible for their own data loading and skeleton states. New TRPC procedures and skeleton components are introduced, and obsolete server-side helpers are removed. Changes
Sequence Diagram(s)sequenceDiagram
participant Page
participant Navbar
participant Table
participant TRPC
participant DB
Page->>Navbar: Render with namespaceId
Navbar->>TRPC: queryDetails(namespaceId, includeOverrides?)
TRPC->>DB: Fetch namespace, ratelimitNamespaces (and overrides)
DB-->>TRPC: Data
TRPC-->>Navbar: Data
Navbar-->>Page: Rendered Navbar
Page->>Table: Render with namespaceId
Table->>TRPC: queryDetails(namespaceId, includeOverrides: true)
TRPC->>DB: Fetch overrides
DB-->>TRPC: Overrides data
TRPC-->>Table: Overrides data
loop For each override
Table->>LastUsedCell: Render with namespaceId, identifier
LastUsedCell->>TRPC: queryRatelimitLastUsed(namespaceId, identifier)
TRPC->>DB: Fetch last used timestamp
DB-->>TRPC: Timestamp
TRPC-->>LastUsedCell: Timestamp
end
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
⏰ Context from checks skipped due to timeout of 90000ms (3)
🔇 Additional comments (15)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Thank you for following the naming conventions for pull request titles! 🙏 |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 25
🔭 Outside diff range comments (1)
apps/dashboard/app/(app)/ratelimits/[namespaceId]/overrides/overrides-table.tsx (1)
38-48: Minor perf:getRowClassNamerecreated every renderSince the return value is static, memoise once outside the component to avoid a new fn on every render (micro-optimisation, but free).
-const getRowClassName = () => { +const getRowClassName = (() => { const style = STATUS_STYLES.default; return cn( style.base, style.hover, "group rounded-md", "focus:outline-none focus:ring-1 focus:ring-opacity-40", style.focusRing, ); -}; +})();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (19)
apps/dashboard/app/(app)/ratelimits/[namespaceId]/_overview/components/table/utils/get-row-class.ts(1 hunks)apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/page.tsx(1 hunks)apps/dashboard/app/(app)/ratelimits/[namespaceId]/namespace-navbar.tsx(2 hunks)apps/dashboard/app/(app)/ratelimits/[namespaceId]/namespace.actions.ts(0 hunks)apps/dashboard/app/(app)/ratelimits/[namespaceId]/overrides/last-used-cell.tsx(1 hunks)apps/dashboard/app/(app)/ratelimits/[namespaceId]/overrides/overrides-table.tsx(4 hunks)apps/dashboard/app/(app)/ratelimits/[namespaceId]/overrides/page.tsx(1 hunks)apps/dashboard/app/(app)/ratelimits/[namespaceId]/page.tsx(1 hunks)apps/dashboard/app/(app)/ratelimits/[namespaceId]/settings/components/settings-client.tsx(5 hunks)apps/dashboard/app/(app)/ratelimits/[namespaceId]/settings/components/skeleton.tsx(1 hunks)apps/dashboard/app/(app)/ratelimits/[namespaceId]/settings/page.tsx(1 hunks)apps/dashboard/app/(app)/ratelimits/_components/namespace-card.tsx(2 hunks)apps/dashboard/app/(app)/ratelimits/_components/ratelimit-client.tsx(2 hunks)apps/dashboard/app/(app)/ratelimits/_components/skeletons.tsx(1 hunks)apps/dashboard/app/(app)/ratelimits/page.tsx(1 hunks)apps/dashboard/components/stats-card/index.tsx(1 hunks)apps/dashboard/lib/trpc/routers/index.ts(2 hunks)apps/dashboard/lib/trpc/routers/ratelimit/query-last-used-times.ts(1 hunks)apps/dashboard/lib/trpc/routers/ratelimit/query-namespace-details/index.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/dashboard/app/(app)/ratelimits/[namespaceId]/namespace.actions.ts
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: Test Packages / Test ./packages/cache
- GitHub Check: Test Packages / Test ./packages/api
- GitHub Check: Test Packages / Test ./packages/nextjs
- GitHub Check: Test Packages / Test ./packages/hono
- GitHub Check: Test Packages / Test ./internal/resend
- GitHub Check: Test Packages / Test ./internal/clickhouse
- GitHub Check: Test Packages / Test ./internal/encryption
- GitHub Check: autofix
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (6)
apps/dashboard/app/(app)/ratelimits/_components/skeletons.tsx (1)
25-27: ```shell
#!/bin/bash
set -efile="apps/dashboard/app/(app)/ratelimits/_components/skeletons.tsx"
echo "Inspecting file: $file"
echoecho "=== Imports ==="
rg --line-number "^import.*Clock" "$file" || true
rg --line-number "^import.*ProgressBar" "$file" || true
echoecho "=== Lines 1-80 ==="
sed -n '1,80p' "$file"</details> <details> <summary>apps/dashboard/app/(app)/ratelimits/[namespaceId]/_overview/components/table/utils/get-row-class.ts (1)</summary> `22-24`: **Looks good – style tweak only** Border additions are self-contained and don’t affect logic. ✅ </details> <details> <summary>apps/dashboard/lib/trpc/routers/index.ts (1)</summary> `70-75`: **Router wiring checks out** New procedures are correctly imported and exposed at `ratelimit.namespace.queryDetails` and `ratelimit.namespace.queryRatelimitLastUsed`. No conflicts detected with existing paths. Just ensure corresponding client hooks (`utils.ratelimit.namespace.queryDetails`) are regenerated after the change. Also applies to: 210-213 </details> <details> <summary>apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/page.tsx (1)</summary> `6-17`: **Same destructuring nitpick** Inline–destructuring keeps the signature short and consistent with the other pages. ```diff -export default function RatelimitLogsPage({ - params: { namespaceId }, -}: { - params: { namespaceId: string }; -}) { +export default function RatelimitLogsPage({ + params: { namespaceId }, +}: { + params: { namespaceId: string }; +}) {Likely an incorrect or invalid review comment.
apps/dashboard/lib/trpc/routers/ratelimit/query-last-used-times.ts (1)
11-15: PotentialNumberoverflowClickHouse stores timestamps in milliseconds; current values already exceed
2^31. Usingz.number()risks precision loss pastNumber.MAX_SAFE_INTEGER. Return an ISO string orbigintinstead.-const getLastUsedOutput = z.object({ - identifier: z.string(), - lastUsed: z.number().nullable(), +const getLastUsedOutput = z.object({ + identifier: z.string(), + lastUsed: z.string().nullable(), // ISO date string });Likely an incorrect or invalid review comment.
apps/dashboard/app/(app)/ratelimits/[namespaceId]/overrides/page.tsx (1)
6-21: No functional concerns – neat reduction in propsThe component is now a lightweight client wrapper that simply forwards
namespaceId, which fits the new TRPC-driven data-loading approach. Nothing blocking here.
apps/dashboard/lib/trpc/routers/ratelimit/query-namespace-details/index.ts
Show resolved
Hide resolved
apps/dashboard/lib/trpc/routers/ratelimit/query-namespace-details/index.ts
Show resolved
Hide resolved
apps/dashboard/lib/trpc/routers/ratelimit/query-namespace-details/index.ts
Outdated
Show resolved
Hide resolved
apps/dashboard/lib/trpc/routers/ratelimit/query-namespace-details/index.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
apps/dashboard/lib/trpc/routers/ratelimit/query-namespace-details/index.ts (3)
8-8: TheincludeOverridesflag naming issue persists.The
includeOverridesflag still controls both override data and workspace functionality, which creates confusion for API consumers about what data will be included in the response.
78-102: Unbounded query still risks performance issues.The database query fetches all
ratelimitNamespaceswithout limits, which could cause memory and performance problems for workspaces with many namespaces or overrides.
111-111: Linear search inefficiency remains unaddressed.The code still uses
workspace.ratelimitNamespaces.find()which performs a linear search in memory instead of leveraging database indexing.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
apps/dashboard/app/(app)/ratelimits/page.tsx(1 hunks)apps/dashboard/lib/trpc/routers/ratelimit/query-namespace-details/index.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: Test Packages / Test ./packages/api
- GitHub Check: Test Packages / Test ./packages/nextjs
- GitHub Check: Test Packages / Test ./packages/rbac
- GitHub Check: Test Packages / Test ./internal/clickhouse
- GitHub Check: autofix
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (4)
apps/dashboard/lib/trpc/routers/ratelimit/query-namespace-details/index.ts (1)
35-41: TRPC procedure structure looks solid.The procedure properly uses authentication middleware, workspace context, and rate limiting. The input/output validation with Zod schemas provides good type safety.
apps/dashboard/app/(app)/ratelimits/page.tsx (3)
1-1: Successful conversion to client component.The "use client" directive properly marks this as a client component, aligning with the PR's goal of moving data fetching from RSC to TRPC.
6-13: Clean and focused component structure.The simplified component structure is well-organized, with clear separation of concerns between navigation and the main ratelimit functionality.
10-10: Verify RatelimitClient handles data fetching internally.Since this component no longer passes any props to
RatelimitClient, ensure that the client component properly handles its own data fetching and loading states via TRPC queries.#!/bin/bash # Description: Verify RatelimitClient component handles its own data fetching # Expected: Find TRPC hooks or queries within RatelimitClient component rg -A 5 -B 5 "trpc\.|useQuery|useMutation" apps/dashboard/app/\(app\)/ratelimits/_components/ratelimit-client.tsx
apps/dashboard/lib/trpc/routers/ratelimit/query-namespace-details/index.ts
Show resolved
Hide resolved
2 participants
What does this PR do?
Move ratelimit RSC to tRPC and add some prefetches to make things faster.
Type of change
How should this be tested?
Checklist
Required
pnpm buildpnpm fmtconsole.logsgit pull origin mainAppreciated
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Chores