fix: Format pagination numbers in log footers#3969
fix: Format pagination numbers in log footers#3969naaa760 wants to merge 21 commits intounkeyed:mainfrom naaa760:fix/log-footer
Conversation
naaa760
requested review from
chronark,
mcstepp,
ogzhanolguncu and
perkinsjr
as code owners
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
@naaa760 is attempting to deploy a commit to the Unkey Team on Vercel. A member of the Team first needs to authorize it. |
|
Warning Rate limit exceeded@naaa760 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 3 minutes and 0 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughAdds a new non-compact number formatter Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant KL as KeysList
participant R as TableRow
participant SC as SelectionControls
U->>R: Hover row
R-->>U: Reveal checkbox UI
U->>R: Click row checkbox
R->>KL: toggle key id in selectedKeys
KL->>KL: compute getSelectedKeysState(selectedKeys, keys)
KL->>SC: update selectedKeys, setter, aggregate state
SC-->>U: reflect header selection (all/mixed/none)
U->>R: Click row to navigate
R->>KL: set isNavigating
KL-->>R: show Loading indicator in identity/icon
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
|
Thank you for following the naming conventions for pull request titles! 🙏 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/components/table/logs-table.tsx (1)
159-177: Deduplicate severity mapping; tighten types
categorizeSeverityduplicatesgetStatusTypelogic and acceptsstring, hiding typos at compile time. Consolidate to a single source of truth and type it asLogOutcomeType.Apply:
-export const categorizeSeverity = ( - outcome: string -): keyof typeof STATUS_STYLES => { +export const categorizeSeverity = ( + outcome: LogOutcomeType +): keyof typeof STATUS_STYLES => { switch (outcome) { @@ - default: - return "success"; + default: + return "error"; } };Then remove
getStatusTypeand reusecategorizeSeveritywhere colors are computed:- STATUS_STYLES[getStatusType(outcomeInfo.type)].badge.selected + STATUS_STYLES[categorizeSeverity(outcomeInfo.type)].badge!.selectedapps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/keys-list.tsx (1)
337-337: Remove unsafe cast in rowClassNameCasting selectedKey to KeyDetails can hide null cases at compile time and risk runtime errors in helpers. Pass the nullable value and type helpers accordingly.
- rowClassName={(log) => getRowClassName(log, selectedKey as KeyDetails)} + rowClassName={(log) => getRowClassName(log, selectedKey)}If getRowClassName expects a non-null second arg, update its signature to accept KeyDetails | null.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (13)
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/components/table/logs-table.tsx(15 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/keys-list.tsx(11 hunks)apps/dashboard/app/(app)/apis/_components/api-list-client.tsx(3 hunks)apps/dashboard/app/(app)/apis/_components/api-list-grid.tsx(1 hunks)apps/dashboard/app/(app)/authorization/permissions/components/table/permissions-list.tsx(11 hunks)apps/dashboard/app/(app)/authorization/roles/components/table/roles-list.tsx(6 hunks)apps/dashboard/app/(app)/logs/components/table/logs-table.tsx(9 hunks)apps/dashboard/app/(app)/projects/[projectId]/deployments/components/table/deployments-list.tsx(9 hunks)apps/dashboard/app/(app)/projects/_components/list/index.tsx(3 hunks)apps/dashboard/app/(app)/ratelimits/[namespaceId]/_overview/components/table/logs-table.tsx(7 hunks)apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/components/table/logs-table.tsx(10 hunks)apps/dashboard/app/(app)/settings/root-keys/components/table/root-keys-list.tsx(14 hunks)apps/dashboard/components/virtual-table/components/loading-indicator.tsx(2 hunks)
🧰 Additional context used
🧠 Learnings (10)
📚 Learning: 2024-12-03T14:17:08.016Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#2143
File: apps/dashboard/app/(app)/logs/logs-page.tsx:77-83
Timestamp: 2024-12-03T14:17:08.016Z
Learning: The `<LogsTable />` component already implements virtualization to handle large datasets efficiently.
Applied to files:
apps/dashboard/app/(app)/ratelimits/[namespaceId]/_overview/components/table/logs-table.tsxapps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/components/table/logs-table.tsxapps/dashboard/app/(app)/logs/components/table/logs-table.tsxapps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/components/table/logs-table.tsx
📚 Learning: 2025-08-25T13:46:34.441Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3834
File: apps/dashboard/app/(app)/ratelimits/_components/controls/components/namespace-list-refresh.tsx:4-4
Timestamp: 2025-08-25T13:46:34.441Z
Learning: The namespace list refresh component (apps/dashboard/app/(app)/ratelimits/_components/controls/components/namespace-list-refresh.tsx) intentionally uses the overview hook (useFilters from @/app/(app)/ratelimits/[namespaceId]/_overview/hooks/use-filters) rather than a namespace-specific hook. This cross-coupling between namespace list components and overview hooks is an architectural design decision.
Applied to files:
apps/dashboard/app/(app)/ratelimits/[namespaceId]/_overview/components/table/logs-table.tsx
📚 Learning: 2025-08-25T13:46:08.303Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3834
File: apps/dashboard/app/(app)/ratelimits/_components/controls/components/namespace-list-datetime/index.tsx:1-1
Timestamp: 2025-08-25T13:46:08.303Z
Learning: The NamespaceListDateTime component in apps/dashboard/app/(app)/ratelimits/_components/controls/components/namespace-list-datetime/index.tsx is intentionally designed to use the overview hook (useFilters from @/app/(app)/ratelimits/[namespaceId]/_overview/hooks/use-filters) rather than the namespace list hook, as clarified by ogzhanolguncu. This coupling is by design, not an architectural issue.
Applied to files:
apps/dashboard/app/(app)/ratelimits/[namespaceId]/_overview/components/table/logs-table.tsx
📚 Learning: 2025-02-05T12:56:44.873Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#2866
File: apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/components/table/log-details/components/log-footer.tsx:85-98
Timestamp: 2025-02-05T12:56:44.873Z
Learning: The RequestResponseDetails component in the ratelimit logs UI already handles empty content cases by preventing rendering when content is empty, so no additional empty state handling is needed in the parent components.
Applied to files:
apps/dashboard/app/(app)/ratelimits/[namespaceId]/_overview/components/table/logs-table.tsxapps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/components/table/logs-table.tsx
📚 Learning: 2024-10-08T15:33:04.290Z
Learnt from: AkshayBandi027
PR: unkeyed/unkey#2215
File: apps/dashboard/app/(app)/@breadcrumb/authorization/roles/[roleId]/page.tsx:28-29
Timestamp: 2024-10-08T15:33:04.290Z
Learning: In `authorization/roles/[roleId]/update-role.tsx`, the tag `role-${role.id}` is revalidated after updating a role to ensure that the caching mechanism is properly handled for roles.
Applied to files:
apps/dashboard/app/(app)/authorization/roles/components/table/roles-list.tsx
📚 Learning: 2025-07-25T19:09:43.284Z
Learnt from: mcstepp
PR: unkeyed/unkey#3662
File: apps/dashboard/lib/trpc/routers/deployment/list.ts:11-11
Timestamp: 2025-07-25T19:09:43.284Z
Learning: In apps/dashboard/lib/trpc/routers/deployment/list.ts, the listDeployments procedure intentionally queries the versions table rather than a deployments table. The user mcstepp indicated that renaming the table would require a database migration, which was deferred for the current PR focused on UI features.
Applied to files:
apps/dashboard/app/(app)/projects/[projectId]/deployments/components/table/deployments-list.tsx
📚 Learning: 2025-09-12T17:57:12.128Z
Learnt from: mcstepp
PR: unkeyed/unkey#3952
File: apps/dashboard/app/(app)/projects/[projectId]/deployments/components/rollback-dialog.tsx:111-116
Timestamp: 2025-09-12T17:57:12.128Z
Learning: In the Deployment type used across the dashboard deployment components, the source field is required and will always be present, so optional chaining (source?.branch) is not needed when accessing source.branch or source.gitSha.
Applied to files:
apps/dashboard/app/(app)/projects/[projectId]/deployments/components/table/deployments-list.tsx
📚 Learning: 2024-12-03T14:21:19.543Z
Learnt from: chronark
PR: unkeyed/unkey#2143
File: apps/dashboard/app/(app)/logs/components/log-details/components/log-footer.tsx:58-61
Timestamp: 2024-12-03T14:21:19.543Z
Learning: For the "Outcome" field in the `LogFooter` component (`apps/dashboard/app/(app)/logs/components/log-details/components/log-footer.tsx`), default to "N/A" instead of "VALID" when handling null values to avoid confusing customers.
Applied to files:
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/components/table/logs-table.tsx
📚 Learning: 2025-08-29T12:08:57.482Z
Learnt from: perkinsjr
PR: unkeyed/unkey#3880
File: apps/dashboard/app/(app)/settings/root-keys/components/root-key/root-key-dialog.tsx:137-141
Timestamp: 2025-08-29T12:08:57.482Z
Learning: Root keys in the Unkey system require at least one permission at minimum, so there is no valid case where a root key would have zero permissions. This means checking for permission existence when determining UI labels is unnecessary.
Applied to files:
apps/dashboard/app/(app)/settings/root-keys/components/table/root-keys-list.tsx
📚 Learning: 2025-04-24T14:34:30.621Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3115
File: apps/dashboard/components/logs/checkbox/filters-popover.tsx:33-55
Timestamp: 2025-04-24T14:34:30.621Z
Learning: In the ShortcutActivator component within filters-popover.tsx, the purpose is to track keys separately for each filter item, providing a registration mechanism for shortcuts passed to it rather than enforcing specific key combinations like option+shift+key.
Applied to files:
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/keys-list.tsx
🧬 Code graph analysis (11)
apps/dashboard/app/(app)/ratelimits/[namespaceId]/_overview/components/table/logs-table.tsx (2)
apps/dashboard/app/(app)/ratelimits/[namespaceId]/_overview/components/table/hooks/use-logs-query.ts (1)
useRatelimitOverviewLogsQuery(15-129)apps/dashboard/app/(app)/ratelimits/[namespaceId]/_overview/components/table/utils/get-row-class.ts (2)
STATUS_STYLES(16-37)getRowClassName(43-61)
apps/dashboard/app/(app)/authorization/roles/components/table/roles-list.tsx (2)
apps/dashboard/app/(app)/authorization/roles/components/table/hooks/use-roles-list-query.ts (1)
useRolesListQuery(8-82)apps/dashboard/app/(app)/authorization/roles/components/table/components/selection-controls/index.tsx (1)
SelectionControls(14-104)
apps/dashboard/app/(app)/projects/[projectId]/deployments/components/table/deployments-list.tsx (5)
apps/dashboard/app/(app)/projects/[projectId]/deployments/components/table/components/skeletons.tsx (1)
ActionColumnSkeleton(78-89)apps/dashboard/app/(app)/projects/[projectId]/deployments/components/table/hooks/use-deployments-list-query.ts (1)
useDeploymentsListQuery(11-123)apps/dashboard/lib/shorten-id.ts (1)
shortenId(5-48)apps/dashboard/app/(app)/projects/[projectId]/deployments/components/table/components/env-status-badge.tsx (1)
EnvStatusBadge(28-41)apps/dashboard/app/(app)/projects/[projectId]/deployments/components/table/utils/get-row-class.ts (1)
getRowClassName(37-50)
apps/dashboard/app/(app)/projects/_components/list/index.tsx (1)
apps/dashboard/app/(app)/projects/_components/list/projects-card.tsx (1)
ProjectCard(21-111)
apps/dashboard/app/(app)/authorization/permissions/components/table/permissions-list.tsx (5)
apps/dashboard/app/(app)/authorization/permissions/components/table/hooks/use-permissions-list-query.ts (1)
usePermissionsListQuery(11-86)apps/dashboard/lib/trpc/routers/authorization/permissions/query.ts (2)
Permission(19-19)permissions(9-17)internal/icons/src/icons/page-2.tsx (1)
Page2(15-76)apps/dashboard/app/(app)/authorization/permissions/components/table/utils/get-row-class.ts (1)
getRowClassName(26-38)apps/dashboard/app/(app)/authorization/permissions/components/table/components/skeletons.tsx (4)
RoleColumnSkeleton(4-16)SlugColumnSkeleton(18-25)AssignedKeysColumnSkeleton(27-34)AssignedToKeysColumnSkeleton(36-43)
apps/dashboard/app/(app)/apis/_components/api-list-client.tsx (1)
apps/dashboard/app/(app)/apis/_components/controls/index.tsx (1)
ApiListControls(17-29)
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/components/table/logs-table.tsx (4)
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/components/table/utils/get-row-class.ts (2)
categorizeSeverity(59-79)STATUS_STYLES(16-57)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/context/logs.tsx (1)
useKeyDetailsLogsContext(31-37)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/components/table/hooks/use-logs-query.ts (1)
useKeyDetailsLogsQuery(22-236)internal/clickhouse/src/verifications.ts (1)
KeyDetailsLog(66-66)
apps/dashboard/app/(app)/settings/root-keys/components/table/root-keys-list.tsx (5)
packages/rbac/src/permissions.ts (1)
UnkeyPermission(83-83)internal/icons/src/icons/dots.tsx (1)
Dots(15-60)apps/dashboard/lib/trpc/routers/settings/root-keys/query.ts (1)
RootKey(35-35)apps/dashboard/components/virtual-table/types.ts (1)
Column(13-24)apps/dashboard/app/(app)/settings/root-keys/components/table/components/actions/root-keys-table-action.popover.constants.tsx (1)
RootKeysTableActions(12-15)
apps/dashboard/app/(app)/logs/components/table/logs-table.tsx (2)
internal/clickhouse/src/logs.ts (2)
log(24-38)Log(40-40)apps/dashboard/app/(app)/logs/components/table/hooks/use-logs-query.ts (1)
useLogsQuery(19-241)
apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/components/table/logs-table.tsx (2)
apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/components/table/hooks/use-logs-query.ts (1)
useRatelimitLogsQuery(17-208)internal/clickhouse/src/logs.ts (1)
log(24-38)
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/keys-list.tsx (3)
internal/icons/src/icons/dots.tsx (1)
Dots(15-60)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/hooks/use-keys-list-query.ts (1)
useKeysListQuery(13-85)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/hidden-value.tsx (1)
HiddenValueCell(5-47)
🔇 Additional comments (46)
apps/dashboard/app/(app)/settings/root-keys/components/table/root-keys-list.tsx (9)
16-18: Type guard looks good
isUnkeyPermissionkeeps downstream types clean. LGTM.
37-40: Dynamic import fallback polish LGTMLazy-loading actions + accessible loading button state read well.
Also applies to: 47-47, 49-52, 55-55
83-84: Correct dependency for memoized row classIncluding
selectedRootKeyis correct to avoid stale closures.
115-117: Empty state copy tweak LGTMClearer guidance reads well.
145-146: Memoized config LGTMStable ref prevents VirtualTable re-renders.
153-159: Skeleton row spacing alignment LGTMConsistent paddings improve visual rhythm.
Also applies to: 165-165, 177-178
281-283: TimestampInfo class tweak LGTM
306-307: Action column render LGTM
311-312: Columns memo deps LGTMapps/dashboard/app/(app)/authorization/permissions/components/table/permissions-list.tsx (3)
70-71: Selected background toggle LGTM
122-125: Selected-row highlighting LGTMAlso applies to: 136-139, 150-153, 164-167
259-260: Skeleton spacing LGTMAlso applies to: 266-268
apps/dashboard/app/(app)/apis/_components/api-list-client.tsx (3)
32-34: No-op formatting changeTrailing comma removal is fine.
64-68: Controls props reflow LGTM
94-99: Button props reflow LGTMapps/dashboard/app/(app)/projects/_components/list/index.tsx (1)
41-43: Empty state copy wrap LGTMapps/dashboard/app/(app)/ratelimits/[namespaceId]/_overview/components/table/logs-table.tsx (5)
31-41: Query hook destructuring LGTMGood readability and ordering.
76-81: Badge selected/default styling LGTMAlso applies to: 112-115
193-196: Pointer-events guard LGTMPrevents unintended interactions on non-selected rows.
225-227: Row className delegation LGTM
250-253: Empty state copy wrap LGTMapps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/components/table/logs-table.tsx (7)
19-26: UI imports consolidation LGTM
191-211: Query hook usage LGTMPoll interval, startPolling wiring look correct.
213-228: Row className helper: good encapsulationKeeps styling rules centralized.
260-260: Dependency array LGTMIncludes stable utils and timestamp refs.
278-281: Pointer-events guard LGTM
308-311: Badge color selection LGTM after dedupOnce
categorizeSeverityis the single source, this remains clean.
349-356: Long-tag tooltip formatting LGTMReadable truncation with copy affordance looks good.
Also applies to: 367-369, 415-421, 433-435
apps/dashboard/app/(app)/authorization/roles/components/table/roles-list.tsx (5)
25-26: Query wiring LGTM
60-60: Selected background toggle LGTM
168-171: Header SelectionControls injection LGTMOnce IDs are used, bulk actions should align with
useDeleteRole.
191-193: Empty state copy wrap LGTM
221-221: Skeleton spacing LGTMapps/dashboard/app/(app)/apis/_components/api-list-grid.tsx (1)
46-47: Use shared formatter and make this a client component to avoid locale/hydration issuesApply this diff in the shown file:
- Showing {new Intl.NumberFormat().format(apiList.length)} of{" "} - {new Intl.NumberFormat().format(total)} APIs + Showing {formatNumber(apiList.length)} of {formatNumber(total)} APIsAdd the import near the top:
+import { formatNumber } from "@/lib/fmt";If this component renders interactive controls or relies on client-side handlers, make it a client component:
+ "use client";Verified occurrence: new Intl.NumberFormat(...).format found at lines 46–47.
apps/dashboard/components/virtual-table/components/loading-indicator.tsx (2)
66-68: LGTM: countInfoText wrapper improves consistencyWrapping countInfoText is fine and non-breaking.
121-123: LGTM: conditional countInfoText blockNo functional changes; readable structure.
apps/dashboard/app/(app)/logs/components/table/logs-table.tsx (2)
144-147: Confirm cn supports array argumentsYou’re passing an array of class names conditionally. Ensure cn (your wrapper) accepts arrays like clsx; otherwise, join into a string.
If arrays aren’t supported:
- ) && ["opacity-50", "hover:opacity-100"], + ) && "opacity-50 hover:opacity-100",
165-168: Pointer-events disabled on non-selected timestamps — double-check UXDisabling pointer events on other rows’ timestamps may block expected interactions (e.g., copying/opening). Confirm this is intentional.
apps/dashboard/app/(app)/projects/[projectId]/deployments/components/table/deployments-list.tsx (2)
31-33: Verify dynamic import pathConfirm "./components/actions/deployment-list-table-action.popover.constants" exists and exports DeploymentListTableActions as named. If not, this will fail at runtime.
317-319: LGTM: rowClassName extractionDelegating to getRowClassName keeps table logic focused.
apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/components/table/logs-table.tsx (2)
94-101: Confirm cn supports array argumentsSame pattern as main logs table; ensure cn handles arrays; otherwise use a space-joined string.
- ) && ["opacity-50", "hover:opacity-100"], + ) && "opacity-50 hover:opacity-100",
118-121: Pointer-events disabled on non-selected timestamps — verify UXThis may hinder interactions on non-selected rows. Validate intent.
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/keys-list.tsx (4)
40-46: LGTM: loading fallback polishImproved Dots styling is fine and non-functional.
100-116: LGTM: navigating state UXNice touch showing a spinner while navigating; prevents flicker.
223-227: LGTM: pass selected to HiddenValueCellProp will enable focused styling without side effects.
409-419: LGTM: skeletons for unspecified columnsGood UX improvement; safe and scoped.
| <span className="font-mono bg-gray-4 p-1 rounded"> | ||
| {identity} | ||
| </span> | ||
| </Link> | ||
| ) : ( | ||
| <span className="font-mono bg-gray-4 p-1 rounded">{identity}</span> | ||
| <span className="font-mono bg-gray-4 p-1 rounded"> | ||
| {identity} | ||
| </span> | ||
| )} |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Make the tooltip trigger focusable for keyboard users
iconContainer is a div; with asChild, the trigger won’t be focusable. Add tabIndex and an aria-label for a11y.
- {React.cloneElement(iconContainer, {
- className: cn(
- iconContainer.props.className,
- "cursor-pointer"
- ),
- })}
+ {React.cloneElement(iconContainer, {
+ className: cn(iconContainer.props.className, "cursor-pointer"),
+ tabIndex: 0,
+ role: "button",
+ "aria-label": "Key identity info",
+ })}Also applies to: 179-184
🤖 Prompt for AI Agents
In
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/keys-list.tsx
around lines 166-174 (and likewise 179-184), the tooltip trigger is using a div
asChild which is not keyboard-focusable; make the trigger accessible by either
replacing the div with a semantic focusable element (e.g., a button) or adding
tabIndex={0} and a descriptive aria-label to the existing element so it can
receive keyboard focus and convey its purpose to assistive tech; apply the same
change to the second occurrence at lines 179-184.
| <span>Showing</span>{" "} | ||
| <span className="text-accent-12"> | ||
| {new Intl.NumberFormat().format(keys.length)} | ||
| </span> | ||
| <span>of</span> | ||
| {totalCount} | ||
| {new Intl.NumberFormat().format(totalCount)} | ||
| <span>keys</span> | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Reuse a memoized number formatter
Apply the shared formatter to keep consistency with the rest of the PR.
- <span className="text-accent-12">
- {new Intl.NumberFormat().format(keys.length)}
- </span>
+ <span className="text-accent-12">{nf.format(keys.length)}</span>
...
- {new Intl.NumberFormat().format(totalCount)}
+ {nf.format(totalCount)}Support (outside selected lines):
import React, { useCallback, useMemo, useState } from "react";
const nf = useMemo(() => new Intl.NumberFormat(), []);🤖 Prompt for AI Agents
In
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/keys-list.tsx
around lines 352 to 359, the code creates new Intl.NumberFormat instances inline
instead of reusing the shared memoized formatter; replace both occurrences of
new Intl.NumberFormat().format(...) with the existing nf.format(...) and ensure
nf is defined with useMemo (const nf = useMemo(() => new Intl.NumberFormat(),
[])) in the component scope so the shared, memoized formatter is used
consistently across the file.
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/components/table/logs-table.tsx
Show resolved
Hide resolved
| const { | ||
| permissions, | ||
| isLoading, | ||
| isLoadingMore, | ||
| loadMore, | ||
| totalCount, | ||
| hasMore, | ||
| } = usePermissionsListQuery(); | ||
| const [selectedPermission, setSelectedPermission] = | ||
| useState<Permission | null>(null); | ||
| const [selectedPermissions, setSelectedPermissions] = useState<Set<string>>( | ||
| new Set() | ||
| ); | ||
| const [hoveredPermissionName, setHoveredPermissionName] = useState< | ||
| string | null | ||
| >(null); | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Selection state should use stable IDs, not names
Using permission.name risks collisions and breaks bulk actions that typically expect IDs.
Apply:
- const [selectedPermissions, setSelectedPermissions] = useState<Set<string>>(
- new Set()
- );
- const [hoveredPermissionName, setHoveredPermissionName] = useState<
- string | null
- >(null);
+ const [selectedPermissions, setSelectedPermissions] = useState<Set<string>>(new Set());
+ const [hoveredPermissionId, setHoveredPermissionId] = useState<string | null>(null);
@@
- const toggleSelection = useCallback((permissionName: string) => {
+ const toggleSelection = useCallback((permissionId: string) => {
setSelectedPermissions((prevSelected) => {
const newSelected = new Set(prevSelected);
- if (newSelected.has(permissionName)) {
- newSelected.delete(permissionName);
+ if (newSelected.has(permissionId)) {
+ newSelected.delete(permissionId);
} else {
- newSelected.add(permissionName);
+ newSelected.add(permissionId);
}
return newSelected;
});
}, []);Propagate usages:
- const isSelected = selectedPermissions.has(permission.name);
- const isHovered = hoveredPermissionName === permission.name;
+ const isSelected = selectedPermissions.has(permission.permissionId);
+ const isHovered = hoveredPermissionId === permission.permissionId;
@@
- onMouseEnter={() => setHoveredPermissionName(permission.name)}
- onMouseLeave={() => setHoveredPermissionName(null)}
+ onMouseEnter={() => setHoveredPermissionId(permission.permissionId)}
+ onMouseLeave={() => setHoveredPermissionId(null)}
@@
- onCheckedChange={() => toggleSelection(permission.name)}
+ onCheckedChange={() => toggleSelection(permission.permissionId)}Update memo deps accordingly:
- [
- selectedPermissions,
- toggleSelection,
- hoveredPermissionName,
- selectedPermission?.permissionId,
- ]
+ [selectedPermissions, toggleSelection, hoveredPermissionId, selectedPermission?.permissionId]Confirm SelectionControls expects IDs. If not, align both ends.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== 1) Show target file if exists ==="
TARGET='apps/dashboard/app/(app)/authorization/permissions/components/table/permissions-list.tsx'
if [ -f "$TARGET" ]; then
echo "FOUND: $TARGET"
sed -n '1,320p' "$TARGET" || true
else
echo "NOT FOUND: $TARGET"
echo "Searching for files named permissions-list.tsx..."
rg -n --hidden --files -g '!**/node_modules/**' 'permissions-list.tsx' || true
fi
echo
echo "=== 2) Find SelectionControls symbol occurrences ==="
rg -n --hidden -S '\bSelectionControls\b' -g '!**/node_modules/**' -C3 || true
echo
echo "=== 3) If SelectionControls files found, show their first matches ==="
rg --hidden -n --files -g '!**/node_modules/**' '\bSelectionControls\b' | head -n 10 || true
rg -n --hidden -S '\bSelectionControls\b' -g '!**/node_modules/**' --files | while read -r f; do
echo "---- $f ----"
sed -n '1,240p' "$f" || true
echo
done
echo "=== 4) Search for onCheckedChange usages ==="
rg -n --hidden -S 'onCheckedChange' -g '!**/node_modules/**' -C3 || true
echo "=== 5) Search for selectedPermissions variable usages ==="
rg -n --hidden -S '\bselectedPermissions\b' -g '!**/node_modules/**' -C3 || true
echo "=== 6) Search for hoveredPermissionName / hoveredPermissionId usages ==="
rg -n --hidden -S '\bhoveredPermissionName\b' -g '!**/node_modules/**' -C3 || true
rg -n --hidden -S '\bhoveredPermissionId\b' -g '!**/node_modules/**' -C3 || true
echo "=== 7) Search for Permission type/interface definitions ==="
rg -n --hidden -S '\binterface\s+Permission\b' -g '!**/node_modules/**' -C3 || true
rg -n --hidden -S '\btype\s+Permission\b' -g '!**/node_modules/**' -C3 || true
rg -n --hidden -S '\bexport\s+type\s+Permission\b' -g '!**/node_modules/**' -C3 || true
echo "=== 8) Search for permission.permissionId / permission.id / permission.name ==="
rg -n --hidden -S 'permission\.permissionId' -g '!**/node_modules/**' -C3 || true
rg -n --hidden -S 'permission\.id\b' -g '!**/node_modules/**' -C3 || true
rg -n --hidden -S 'permission\.name\b' -g '!**/node_modules/**' -C3 || true
echo "=== 9) Search for toggleSelection function/usages ==="
rg -n --hidden -S '\btoggleSelection\b' -g '!**/node_modules/**' -C5 || true
echo "=== 10) Search for SelectionControls import in the target file ==="
if [ -f "$TARGET" ]; then
rg -n --hidden -S 'SelectionControls' "$TARGET" -C3 || true
fi
echo "=== done ==="Length of output: 9648
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "PWD: $(pwd)"
COMP_DIR='apps/dashboard/app/(app)/authorization/permissions/components/table/components'
echo
echo "=== Listing components dir ==="
if [ -d "$COMP_DIR" ]; then
ls -la "$COMP_DIR" || true
else
echo "Directory not found: $COMP_DIR"
fi
echo
echo "=== Try known selection-controls paths ==="
paths=(
"$COMP_DIR/selection-controls.tsx"
"$COMP_DIR/selection-controls.ts"
"$COMP_DIR/selection-controls.jsx"
"$COMP_DIR/selection-controls.js"
'apps/dashboard/app/(app)/authorization/permissions/components/selection-controls.tsx'
'apps/dashboard/app/(app)/authorization/permissions/components/selection-controls.ts'
)
for p in "${paths[@]}"; do
if [ -f "$p" ]; then
echo "FOUND: $p"
sed -n '1,240p' "$p" || true
else
echo "Not found: $p"
fi
done
echo
echo "=== Find any file names containing selection-controls ==="
find apps -type f -iname '*selection-controls*' -maxdepth 6 -print -exec sed -n '1,240p' {} \; || true
echo
echo "=== Grep for 'SelectionControls' symbol (fallback) ==="
grep -RIn --exclude-dir=node_modules --exclude-dir=.git "SelectionControls" apps || true
echo "=== done ==="Length of output: 3406
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILES=(
'apps/dashboard/app/(app)/authorization/permissions/components/table/components/selection-controls/index.tsx'
'apps/dashboard/app/(app)/authorization/roles/components/table/components/selection-controls/index.tsx'
'apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/selection-controls/index.tsx'
)
for f in "${FILES[@]}"; do
echo "=== FILE: $f ==="
if [ -f "$f" ]; then
sed -n '1,240p' "$f" || true
else
echo "NOT FOUND: $f"
fi
echo
doneLength of output: 15089
Use stable IDs for selection state (permission.permissionId) — critical fix required
SelectionControls expects permission IDs (it passes permissionIds to the delete mutation), but PermissionsList currently toggles/checks by permission.name — this breaks bulk actions. Change selection/hover state and toggle to use permission.permissionId.
- const [selectedPermissions, setSelectedPermissions] = useState<Set<string>>(
- new Set()
- );
- const [hoveredPermissionName, setHoveredPermissionName] = useState<
- string | null
- >(null);
+ const [selectedPermissions, setSelectedPermissions] = useState<Set<string>>(new Set());
+ const [hoveredPermissionId, setHoveredPermissionId] = useState<string | null>(null);
@@
- const toggleSelection = useCallback((permissionName: string) => {
+ const toggleSelection = useCallback((permissionId: string) => {
setSelectedPermissions((prevSelected) => {
const newSelected = new Set(prevSelected);
- if (newSelected.has(permissionName)) {
- newSelected.delete(permissionName);
+ if (newSelected.has(permissionId)) {
+ newSelected.delete(permissionId);
} else {
- newSelected.add(permissionName);
+ newSelected.add(permissionId);
}
return newSelected;
});
}, []);Propagate usages:
- const isSelected = selectedPermissions.has(permission.name);
- const isHovered = hoveredPermissionName === permission.name;
+ const isSelected = selectedPermissions.has(permission.permissionId);
+ const isHovered = hoveredPermissionId === permission.permissionId;
@@
- onMouseEnter={() => setHoveredPermissionName(permission.name)}
- onMouseLeave={() => setHoveredPermissionName(null)}
+ onMouseEnter={() => setHoveredPermissionId(permission.permissionId)}
+ onMouseLeave={() => setHoveredPermissionId(null)}
@@
- onCheckedChange={() => toggleSelection(permission.name)}
+ onCheckedChange={() => toggleSelection(permission.permissionId)}Update memo deps accordingly:
- [
- selectedPermissions,
- toggleSelection,
- hoveredPermissionName,
- selectedPermission?.permissionId,
- ]
+ [selectedPermissions, toggleSelection, hoveredPermissionId, selectedPermission?.permissionId]File: apps/dashboard/app/(app)/authorization/permissions/components/table/permissions-list.tsx — apply changes.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const { | |
| permissions, | |
| isLoading, | |
| isLoadingMore, | |
| loadMore, | |
| totalCount, | |
| hasMore, | |
| } = usePermissionsListQuery(); | |
| const [selectedPermission, setSelectedPermission] = | |
| useState<Permission | null>(null); | |
| const [selectedPermissions, setSelectedPermissions] = useState<Set<string>>( | |
| new Set() | |
| ); | |
| const [hoveredPermissionName, setHoveredPermissionName] = useState< | |
| string | null | |
| >(null); | |
| const { | |
| permissions, | |
| isLoading, | |
| isLoadingMore, | |
| loadMore, | |
| totalCount, | |
| hasMore, | |
| } = usePermissionsListQuery(); | |
| const [selectedPermission, setSelectedPermission] = | |
| useState<Permission | null>(null); | |
| const [selectedPermissions, setSelectedPermissions] = useState<Set<string>>(new Set()); | |
| const [hoveredPermissionId, setHoveredPermissionId] = useState<string | null>(null); |
🤖 Prompt for AI Agents
In
apps/dashboard/app/(app)/authorization/permissions/components/table/permissions-list.tsx
around lines 25–41, the selection and hover state currently use permission.name
but SelectionControls and delete mutation require stable IDs; change state and
logic to use permission.permissionId instead: replace selectedPermissions
Set<string> that stores names with one that stores permission.permissionId,
rename hoveredPermissionName to hoveredPermissionId (string | null) and update
all toggle/check handlers, checkbox checked conditions, and any calls that pass
selectedPermissions to use these IDs; update memo dependencies to reference
permission.permissionId where appropriate and ensure SelectionControls receives
the permissionIds set (not names).
| <span>Showing</span>{" "} | ||
| <span className="text-accent-12"> | ||
| {new Intl.NumberFormat().format(historicalLogs.length)} | ||
| </span> | ||
| <span>of {new Intl.NumberFormat().format(totalCount)}</span> | ||
| <span>rate limit identifiers</span> | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use shared formatter for footer counts
You already import formatNumber; reuse it here too.
- <span>Showing</span>{" "}
- <span className="text-accent-12">
- {new Intl.NumberFormat().format(historicalLogs.length)}
- </span>
- <span>of {new Intl.NumberFormat().format(totalCount)}</span>
+ <span>Showing</span> <span className="text-accent-12">{formatNumber(historicalLogs.length)}</span>
+ <span>of {formatNumber(totalCount)}</span>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <span>Showing</span>{" "} | |
| <span className="text-accent-12"> | |
| {new Intl.NumberFormat().format(historicalLogs.length)} | |
| </span> | |
| <span>of {new Intl.NumberFormat().format(totalCount)}</span> | |
| <span>rate limit identifiers</span> | |
| </div> | |
| <span>Showing</span> <span className="text-accent-12">{formatNumber(historicalLogs.length)}</span> | |
| <span>of {formatNumber(totalCount)}</span> | |
| <span>rate limit identifiers</span> | |
| </div> |
🤖 Prompt for AI Agents
In
apps/dashboard/app/(app)/ratelimits/[namespaceId]/_overview/components/table/logs-table.tsx
around lines 235 to 241, the footer uses new Intl.NumberFormat().format(...) for
counts; replace both usages with the already-imported formatNumber(...) helper
so the component uses the shared formatter consistently, e.g. call
formatNumber(historicalLogs.length) and formatNumber(totalCount) in those spans.
| <span>Showing</span>{" "} | ||
| <span className="text-accent-12"> | ||
| {new Intl.NumberFormat().format(historicalLogs.length)} | ||
| </span> | ||
| <span>of</span> | ||
| {totalCount} | ||
| {new Intl.NumberFormat().format(totalCount)} | ||
| <span>ratelimit requests</span> | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Reuse a memoized number formatter
Apply the shared formatter here too.
- <span className="text-accent-12">
- {new Intl.NumberFormat().format(historicalLogs.length)}
- </span>
+ <span className="text-accent-12">{nf.format(historicalLogs.length)}</span>
...
- {new Intl.NumberFormat().format(totalCount)}
+ {nf.format(totalCount)}Support (outside selected lines):
import { useMemo } from "react";
const nf = useMemo(() => new Intl.NumberFormat(), []);🤖 Prompt for AI Agents
In
apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/components/table/logs-table.tsx
around lines 238 to 245, the code creates new Intl.NumberFormat() inline twice;
replace those inline constructors with a shared, memoized formatter (e.g.,
define const nf = useMemo(() => new Intl.NumberFormat(), [] ) at the top of the
component) and then use nf.format(...) for both historicalLogs.length and
totalCount; ensure useMemo is imported from react and that nf is declared inside
the component so it is reused across renders.
apps/dashboard/app/(app)/settings/root-keys/components/table/root-keys-list.tsx
Show resolved
Hide resolved
| .map((p) => p.name) | ||
| .filter(isUnkeyPermission); | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Permission normalization: optional simplification
Given prior learning that root keys always have ≥1 permission, the nullish coalescing could be dropped. Harmless as-is; consider simplifying.
Based on retrieved learnings from PR #3880, confirm if zero-permission root keys are impossible in current API responses.
🤖 Prompt for AI Agents
In
apps/dashboard/app/(app)/settings/root-keys/components/table/root-keys-list.tsx
around lines 189 to 191, the code currently uses a nullish-coalescing fallback
for permissions; remove the `?? []` fallback so you call
.map(...).filter(isUnkeyPermission) directly since root keys are guaranteed to
have ≥1 permission. Before committing, verify the API contract (PR
unkeyed/unkey#3880) to confirm zero-permission root keys cannot be returned; if
verification fails, keep the defensive fallback or add an explicit runtime
assertion and error handling.
apps/dashboard/components/virtual-table/components/loading-indicator.tsx
Outdated
Show resolved
Hide resolved
|
Hi @naaa760 Please run pnpm fmt from the root of the Unkey monorepo. |
|
@perkinsjr |
apps/dashboard/app/(app)/settings/root-keys/components/table/root-keys-list.tsx
Outdated
Show resolved
Hide resolved
- Add formatNumberFull utility function to /lib/fmt.ts for consistent number formatting - Replace all inline new Intl.NumberFormat().format() calls with formatNumberFull() - Apply formatting to pagination footers across all log tables and lists - Numbers now display as 1,239,436 instead of 1239436 for better readability - Maintains consistency with existing utility pattern in codebase
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/dashboard/app/(app)/authorization/permissions/components/table/permissions-list.tsx (1)
36-54: Critical: selection/hover state must use permission.permissionId (fix required)selectedPermissions, hover state, and toggleSelection currently use permission.name — switch them to permission.permissionId. Name-based checks were found in apps/dashboard/app/(app)/authorization/permissions/components/table/permissions-list.tsx (lines ~63–64); update state, handlers, render checks, event handlers, and dependency arrays, then scan the file for any remaining permission.name uses.
- const [selectedPermissions, setSelectedPermissions] = useState<Set<string>>( - new Set() - ); - const [hoveredPermissionName, setHoveredPermissionName] = useState< - string | null - >(null); + const [selectedPermissions, setSelectedPermissions] = useState<Set<string>>(new Set()); + const [hoveredPermissionId, setHoveredPermissionId] = useState<string | null>(null); - const toggleSelection = useCallback((permissionName: string) => { + const toggleSelection = useCallback((permissionId: string) => { setSelectedPermissions((prevSelected) => { const newSelected = new Set(prevSelected); if (newSelected.has(permissionName)) { newSelected.delete(permissionName); } else { newSelected.add(permissionName); } return newSelected; }); }, []); @@ - const isSelected = selectedPermissions.has(permission.name); - const isHovered = hoveredPermissionName === permission.name; + const isSelected = selectedPermissions.has(permission.permissionId); + const isHovered = hoveredPermissionId === permission.permissionId; @@ - onMouseEnter={() => setHoveredPermissionName(permission.name)} - onMouseLeave={() => setHoveredPermissionName(null)} + onMouseEnter={() => setHoveredPermissionId(permission.permissionId)} + onMouseLeave={() => setHoveredPermissionId(null)} @@ - onCheckedChange={() => toggleSelection(permission.name)} + onCheckedChange={() => toggleSelection(permission.permissionId)} @@ - [ - selectedPermissions, - toggleSelection, - hoveredPermissionName, - selectedPermission?.permissionId, - ] + [selectedPermissions, toggleSelection, hoveredPermissionId, selectedPermission?.permissionId]apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/components/table/logs-table.tsx (2)
169-179: Duration treats 0 as empty; confirm units (ms vs µs)
A duration of 0 currently renders “”. Also verify the unit is actually milliseconds.Apply this diff:
- <div className="font-mono"> - {parsedDuration ? msToSeconds(parsedDuration) : "<EMPTY>"} - </div> + <div className="font-mono"> + {parsedDuration != null ? msToSeconds(parsedDuration) : "<EMPTY>"} + </div>
186-201: Reset column: double JSON parse, inconsistent empty label, and falsy‑0 bug
- Parses
response_bodytwice.- Uses
"<Empty>"vs"<EMPTY>"elsewhere.- Falsy check hides valid
0timestamps.Apply this diff:
- render: (log) => { - const parsedReset = safeParseJson(log.response_body)?.reset; - return parsedReset ? ( - <div className="font-mono"> - <TimestampInfo - value={safeParseJson(log.response_body)?.reset ?? "<EMPTY>" - } - className={cn( - "font-mono group-hover:underline decoration-dotted", - selectedLog && - selectedLog.request_id !== log.request_id && - "pointer-events-none" - )} - /> - </div> - ) : ( - "<Empty>" - ); - }, + render: (log) => { + const body = safeParseJson(log.response_body); + const parsedReset = body?.reset; + return parsedReset != null ? ( + <div className="font-mono"> + <TimestampInfo + value={parsedReset} + className={cn( + "font-mono group-hover:underline decoration-dotted", + selectedLog && + selectedLog.request_id !== log.request_id && + "pointer-events-none" + )} + /> + </div> + ) : ( + "<EMPTY>" + ); + },
♻️ Duplicate comments (8)
apps/dashboard/app/(app)/authorization/roles/components/table/roles-list.tsx (1)
52-73: Critical: selection uses role.name — switch to role.roleIdUse stable IDs to avoid collisions and to match bulk actions.
- const [selectedRoles, setSelectedRoles] = useState<Set<string>>(new Set()); - const [hoveredRoleName, setHoveredRoleName] = useState<string | null>(null); + const [selectedRoles, setSelectedRoles] = useState<Set<string>>(new Set()); + const [hoveredRoleId, setHoveredRoleId] = useState<string | null>(null); @@ - const isSelected = selectedRoles.has(role.name); - const isHovered = hoveredRoleName === role.name; + const isSelected = selectedRoles.has(role.roleId); + const isHovered = hoveredRoleId === role.roleId; @@ - onMouseEnter={() => setHoveredRoleName(role.name)} - onMouseLeave={() => setHoveredRoleName(null)} + onMouseEnter={() => setHoveredRoleId(role.roleId)} + onMouseLeave={() => setHoveredRoleId(null)} @@ - onCheckedChange={() => toggleSelection(role.name)} + onCheckedChange={() => toggleSelection(role.roleId)} @@ - [selectedRoles, toggleSelection, hoveredRoleName, selectedRole?.roleId] + [selectedRoles, toggleSelection, hoveredRoleId, selectedRole?.roleId]Also change toggleSelection to accept roleId (string) and update its internals accordingly.
#!/bin/bash rg -nP --hidden -C2 '\bselectedRoles\.has\(\s*role\.name' apps/dashboard/app/\(app\)/authorization/rolesAlso applies to: 150-151
apps/dashboard/app/(app)/settings/root-keys/components/table/root-keys-list.tsx (1)
188-191: Root key permissions: drop unnecessary fallback (subject to contract)Per prior learning, root keys always have ≥1 permission; the
?? []and validation may be redundant. If API contract guarantees this, simplify; otherwise keep with an assertion.-const permissions = editingKey.permissions ?? []; -const validatedPermissions = permissions - .map((p) => p.name) - .filter(isUnkeyPermission); +const validatedPermissions = editingKey.permissions + .map((p) => p.name) + .filter(isUnkeyPermission);Context: retrieved learnings indicate zero-permission root keys are invalid. Please confirm current API responses.
apps/dashboard/app/(app)/projects/[projectId]/deployments/components/table/deployments-list.tsx (1)
159-161: Hardcoded “2 CPU” — bind to dataEarlier feedback still applies. If CPU varies, read from the deployment object; otherwise annotate why it’s constant.
- <span className="font-semibold text-grayA-12 tabular-nums"> - 2 - </span> + <span className="font-semibold text-grayA-12 tabular-nums"> + {deployment.cpu ?? 2 /* fallback if truly absent */} + </span>apps/dashboard/app/(app)/projects/_components/list/index.tsx (1)
81-83: Date formatting consistencyPrefer a shared date formatter to avoid env-locale variance and ensure consistent timezone/format.
Example:
// lib/date.ts export const formatDate = (ts: number | string | Date, locale = "en-US") => new Intl.DateTimeFormat(locale, { year: "numeric", month: "short", day: "2-digit" }).format(new Date(ts));-commitDate={new Date(project.updatedAt || project.createdAt).toLocaleDateString()} +commitDate={formatDate(project.updatedAt || project.createdAt)}apps/dashboard/components/virtual-table/components/loading-indicator.tsx (1)
1-1: Consider using a memoized formatter to avoid re-instantiation.The
formatNumberFullfunction internally creates a newIntl.NumberFormatinstance on each call. For better performance, especially during re-renders, consider memoizing the formatter instance.+import { useMemo } from "react"; import { formatNumberFull } from "@/lib/fmt";Then in the component:
+const formatNumber = useMemo(() => { + const formatter = new Intl.NumberFormat("en-US"); + return (n: number) => formatter.format(n); +}, []);And use it as:
- title={`${buttonText} • ${formatNumberFull( - totalVisible - )} of ${formatNumberFull(totalCount)} ${itemLabel}`} + title={`${buttonText} • ${formatNumber(totalVisible)} of ${formatNumber(totalCount)} ${itemLabel}`}Also applies to: 61-63, 129-133
apps/dashboard/app/(app)/logs/components/table/logs-table.tsx (1)
188-190: Optimize repeated function calls.The
extractResponseFieldfunction is called twice with identical parameters. Store the result to avoid redundant computation.- {extractResponseField(log, "code") - ? `| ${extractResponseField(log, "code")}` - : ""} + {(() => { + const code = extractResponseField(log, "code"); + return code ? `| ${code}` : ""; + })()}apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/keys-list.tsx (1)
167-174: Add keyboard accessibility to the tooltip trigger.The identity container tooltip trigger is not keyboard accessible. Add proper attributes to support keyboard navigation.
{React.cloneElement(iconContainer, { className: cn( iconContainer.props.className, "cursor-pointer" ), + tabIndex: 0, + role: "button", + "aria-label": "View identity information", })}apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/components/table/logs-table.tsx (1)
238-245: Memoize and reuse a formatter; add SSR hydration guard on dynamic textReplace per-call formatting with a shared formatter and prevent noisy hydration warnings if server/client locales differ.
Apply this diff:
- <div className="flex gap-2"> + <div className="flex gap-2" suppressHydrationWarning> <span>Showing</span>{" "} <span className="text-accent-12"> - {formatNumberFull(historicalLogs.length)} + {nf.format(historicalLogs.length)} </span> <span>of</span> - {formatNumberFull(totalCount)} + {nf.format(totalCount)} <span>ratelimit requests</span> </div>Support (outside selected lines, inside the component):
const nf = useMemo(() => new Intl.NumberFormat(), []);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (14)
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/components/table/logs-table.tsx(16 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/keys-list.tsx(12 hunks)apps/dashboard/app/(app)/apis/_components/api-list-client.tsx(4 hunks)apps/dashboard/app/(app)/apis/_components/api-list-grid.tsx(2 hunks)apps/dashboard/app/(app)/authorization/permissions/components/table/permissions-list.tsx(12 hunks)apps/dashboard/app/(app)/authorization/roles/components/table/roles-list.tsx(7 hunks)apps/dashboard/app/(app)/logs/components/table/logs-table.tsx(10 hunks)apps/dashboard/app/(app)/projects/[projectId]/deployments/components/table/deployments-list.tsx(10 hunks)apps/dashboard/app/(app)/projects/_components/list/index.tsx(4 hunks)apps/dashboard/app/(app)/ratelimits/[namespaceId]/_overview/components/table/logs-table.tsx(8 hunks)apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/components/table/logs-table.tsx(11 hunks)apps/dashboard/app/(app)/settings/root-keys/components/table/root-keys-list.tsx(15 hunks)apps/dashboard/components/virtual-table/components/loading-indicator.tsx(3 hunks)apps/dashboard/lib/fmt.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (9)
📚 Learning: 2024-10-08T15:33:04.290Z
Learnt from: AkshayBandi027
PR: unkeyed/unkey#2215
File: apps/dashboard/app/(app)/@breadcrumb/authorization/roles/[roleId]/page.tsx:28-29
Timestamp: 2024-10-08T15:33:04.290Z
Learning: In `authorization/roles/[roleId]/update-role.tsx`, the tag `role-${role.id}` is revalidated after updating a role to ensure that the caching mechanism is properly handled for roles.
Applied to files:
apps/dashboard/app/(app)/authorization/roles/components/table/roles-list.tsx
📚 Learning: 2025-06-19T11:48:05.070Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3324
File: apps/dashboard/app/(app)/authorization/roles/components/table/components/actions/keys-table-action.popover.constants.tsx:17-18
Timestamp: 2025-06-19T11:48:05.070Z
Learning: In the authorization roles refactor, the RoleBasic type uses `roleId` as the property name for the role identifier, not `id`. This is consistent throughout the codebase in apps/dashboard/lib/trpc/routers/authorization/roles/query.ts.
Applied to files:
apps/dashboard/app/(app)/authorization/roles/components/table/roles-list.tsx
📚 Learning: 2025-07-25T19:09:43.284Z
Learnt from: mcstepp
PR: unkeyed/unkey#3662
File: apps/dashboard/lib/trpc/routers/deployment/list.ts:11-11
Timestamp: 2025-07-25T19:09:43.284Z
Learning: In apps/dashboard/lib/trpc/routers/deployment/list.ts, the listDeployments procedure intentionally queries the versions table rather than a deployments table. The user mcstepp indicated that renaming the table would require a database migration, which was deferred for the current PR focused on UI features.
Applied to files:
apps/dashboard/app/(app)/projects/[projectId]/deployments/components/table/deployments-list.tsx
📚 Learning: 2025-09-12T17:57:12.128Z
Learnt from: mcstepp
PR: unkeyed/unkey#3952
File: apps/dashboard/app/(app)/projects/[projectId]/deployments/components/rollback-dialog.tsx:111-116
Timestamp: 2025-09-12T17:57:12.128Z
Learning: In the Deployment type used across the dashboard deployment components, the source field is required and will always be present, so optional chaining (source?.branch) is not needed when accessing source.branch or source.gitSha.
Applied to files:
apps/dashboard/app/(app)/projects/[projectId]/deployments/components/table/deployments-list.tsx
📚 Learning: 2024-12-03T14:17:08.016Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#2143
File: apps/dashboard/app/(app)/logs/logs-page.tsx:77-83
Timestamp: 2024-12-03T14:17:08.016Z
Learning: The `<LogsTable />` component already implements virtualization to handle large datasets efficiently.
Applied to files:
apps/dashboard/app/(app)/logs/components/table/logs-table.tsxapps/dashboard/app/(app)/ratelimits/[namespaceId]/_overview/components/table/logs-table.tsxapps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/components/table/logs-table.tsxapps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/components/table/logs-table.tsx
📚 Learning: 2025-08-25T13:46:34.441Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3834
File: apps/dashboard/app/(app)/ratelimits/_components/controls/components/namespace-list-refresh.tsx:4-4
Timestamp: 2025-08-25T13:46:34.441Z
Learning: The namespace list refresh component (apps/dashboard/app/(app)/ratelimits/_components/controls/components/namespace-list-refresh.tsx) intentionally uses the overview hook (useFilters from @/app/(app)/ratelimits/[namespaceId]/_overview/hooks/use-filters) rather than a namespace-specific hook. This cross-coupling between namespace list components and overview hooks is an architectural design decision.
Applied to files:
apps/dashboard/app/(app)/ratelimits/[namespaceId]/_overview/components/table/logs-table.tsx
📚 Learning: 2025-02-05T12:56:44.873Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#2866
File: apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/components/table/log-details/components/log-footer.tsx:85-98
Timestamp: 2025-02-05T12:56:44.873Z
Learning: The RequestResponseDetails component in the ratelimit logs UI already handles empty content cases by preventing rendering when content is empty, so no additional empty state handling is needed in the parent components.
Applied to files:
apps/dashboard/app/(app)/ratelimits/[namespaceId]/_overview/components/table/logs-table.tsxapps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/components/table/logs-table.tsx
📚 Learning: 2025-08-29T12:08:57.482Z
Learnt from: perkinsjr
PR: unkeyed/unkey#3880
File: apps/dashboard/app/(app)/settings/root-keys/components/root-key/root-key-dialog.tsx:137-141
Timestamp: 2025-08-29T12:08:57.482Z
Learning: Root keys in the Unkey system require at least one permission at minimum, so there is no valid case where a root key would have zero permissions. This means checking for permission existence when determining UI labels is unnecessary.
Applied to files:
apps/dashboard/app/(app)/settings/root-keys/components/table/root-keys-list.tsx
📚 Learning: 2025-04-24T14:34:30.621Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3115
File: apps/dashboard/components/logs/checkbox/filters-popover.tsx:33-55
Timestamp: 2025-04-24T14:34:30.621Z
Learning: In the ShortcutActivator component within filters-popover.tsx, the purpose is to track keys separately for each filter item, providing a registration mechanism for shortcuts passed to it rather than enforcing specific key combinations like option+shift+key.
Applied to files:
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/keys-list.tsx
🧬 Code graph analysis (13)
apps/dashboard/app/(app)/authorization/roles/components/table/roles-list.tsx (3)
apps/dashboard/app/(app)/authorization/roles/components/table/hooks/use-roles-list-query.ts (1)
useRolesListQuery(8-82)apps/dashboard/app/(app)/authorization/roles/components/table/components/selection-controls/index.tsx (1)
SelectionControls(14-104)apps/dashboard/lib/fmt.ts (1)
formatNumberFull(5-7)
apps/dashboard/app/(app)/apis/_components/api-list-grid.tsx (1)
apps/dashboard/lib/fmt.ts (1)
formatNumberFull(5-7)
apps/dashboard/app/(app)/projects/_components/list/index.tsx (2)
apps/dashboard/app/(app)/projects/_components/list/projects-card.tsx (1)
ProjectCard(21-111)apps/dashboard/lib/fmt.ts (1)
formatNumberFull(5-7)
apps/dashboard/app/(app)/projects/[projectId]/deployments/components/table/deployments-list.tsx (6)
apps/dashboard/app/(app)/projects/[projectId]/deployments/components/table/components/skeletons.tsx (1)
ActionColumnSkeleton(78-89)apps/dashboard/app/(app)/projects/[projectId]/deployments/components/table/hooks/use-deployments-list-query.ts (1)
useDeploymentsListQuery(11-123)apps/dashboard/lib/shorten-id.ts (1)
shortenId(5-48)apps/dashboard/app/(app)/projects/[projectId]/deployments/components/table/components/env-status-badge.tsx (1)
EnvStatusBadge(28-41)apps/dashboard/app/(app)/projects/[projectId]/deployments/components/table/utils/get-row-class.ts (1)
getRowClassName(37-50)apps/dashboard/lib/fmt.ts (1)
formatNumberFull(5-7)
apps/dashboard/components/virtual-table/components/loading-indicator.tsx (1)
apps/dashboard/lib/fmt.ts (1)
formatNumberFull(5-7)
apps/dashboard/app/(app)/apis/_components/api-list-client.tsx (2)
apps/dashboard/app/(app)/apis/_components/controls/index.tsx (1)
ApiListControls(17-29)apps/dashboard/lib/fmt.ts (1)
formatNumberFull(5-7)
apps/dashboard/app/(app)/logs/components/table/logs-table.tsx (3)
internal/clickhouse/src/logs.ts (2)
log(24-38)Log(40-40)apps/dashboard/app/(app)/logs/components/table/hooks/use-logs-query.ts (1)
useLogsQuery(19-241)apps/dashboard/lib/fmt.ts (1)
formatNumberFull(5-7)
apps/dashboard/app/(app)/ratelimits/[namespaceId]/_overview/components/table/logs-table.tsx (3)
apps/dashboard/app/(app)/ratelimits/[namespaceId]/_overview/components/table/hooks/use-logs-query.ts (1)
useRatelimitOverviewLogsQuery(15-129)apps/dashboard/app/(app)/ratelimits/[namespaceId]/_overview/components/table/utils/get-row-class.ts (2)
STATUS_STYLES(16-37)getRowClassName(43-61)apps/dashboard/lib/fmt.ts (1)
formatNumberFull(5-7)
apps/dashboard/app/(app)/settings/root-keys/components/table/root-keys-list.tsx (5)
packages/rbac/src/permissions.ts (1)
UnkeyPermission(83-83)apps/dashboard/lib/fmt.ts (1)
formatNumberFull(5-7)apps/dashboard/lib/trpc/routers/settings/root-keys/query.ts (1)
RootKey(35-35)apps/dashboard/components/virtual-table/types.ts (1)
Column(13-24)apps/dashboard/app/(app)/settings/root-keys/components/table/components/actions/root-keys-table-action.popover.constants.tsx (1)
RootKeysTableActions(12-15)
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/components/table/logs-table.tsx (3)
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/components/table/hooks/use-logs-query.ts (1)
useKeyDetailsLogsQuery(22-236)internal/clickhouse/src/verifications.ts (1)
KeyDetailsLog(66-66)apps/dashboard/lib/fmt.ts (1)
formatNumberFull(5-7)
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/keys-list.tsx (2)
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/hooks/use-keys-list-query.ts (1)
useKeysListQuery(13-85)apps/dashboard/lib/fmt.ts (1)
formatNumberFull(5-7)
apps/dashboard/app/(app)/authorization/permissions/components/table/permissions-list.tsx (4)
apps/dashboard/app/(app)/authorization/permissions/components/table/hooks/use-permissions-list-query.ts (1)
usePermissionsListQuery(11-86)apps/dashboard/lib/trpc/routers/authorization/permissions/query.ts (2)
Permission(19-19)permissions(9-17)apps/dashboard/app/(app)/authorization/permissions/components/table/utils/get-row-class.ts (1)
getRowClassName(26-38)apps/dashboard/lib/fmt.ts (1)
formatNumberFull(5-7)
apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/components/table/logs-table.tsx (2)
apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/components/table/hooks/use-logs-query.ts (1)
useRatelimitLogsQuery(17-208)apps/dashboard/lib/fmt.ts (1)
formatNumberFull(5-7)
🔇 Additional comments (20)
apps/dashboard/app/(app)/apis/_components/api-list-grid.tsx (2)
47-49: LGTM — counts now consistently formattedUsage of formatNumberFull for both visible and total counts looks correct.
2-2: Import alignmentCentralizing number formatting via "@/lib/fmt" is consistent with prior guidance. Good change.
apps/dashboard/app/(app)/settings/root-keys/components/table/root-keys-list.tsx (3)
95-101: LGTM — footer count formattingCorrect adoption of formatNumberFull for “Showing X of Y root keys”.
17-21: LGTM — type guardGood use of a narrow type guard using zod’s safeParse.
38-41: Dynamic import placeholder UX — LGTMGood lightweight skeleton for actions popover with accessible focus styles.
Also applies to: 47-53, 56-56
apps/dashboard/app/(app)/authorization/permissions/components/table/permissions-list.tsx (1)
214-220: LGTM — footer number formattingCounts use formatNumberFull consistently.
apps/dashboard/app/(app)/projects/[projectId]/deployments/components/table/deployments-list.tsx (2)
327-333: LGTM — footer number formattingCorrect usage of formatNumberFull for deployments list.
204-216: LGTM — source fields without optional chainingMatches learning that source is required; clean and safe.
apps/dashboard/app/(app)/projects/_components/list/index.tsx (1)
118-124: LGTM — header count formattingViewing X of Y now uses formatNumberFull consistently.
apps/dashboard/app/(app)/apis/_components/api-list-client.tsx (1)
89-91: LGTM — footer number formattingSwitched to formatNumberFull for both shown and total API counts.
apps/dashboard/app/(app)/authorization/roles/components/table/roles-list.tsx (1)
176-182: LGTM — footer number formattingCounts use formatNumberFull for roles list.
apps/dashboard/lib/fmt.ts (1)
5-7: Make number formatting locale-awareFile: apps/dashboard/lib/fmt.ts (lines 5-7)
Hardcoded "en-US" contradicts "locale-appropriate" — accept an optional locale param (default "en-US") and confirm product decision (fixed en-US vs per-user locale) before changing callsites.
-export function formatNumberFull(n: number): string { - return Intl.NumberFormat("en-US").format(n); -} +export function formatNumberFull(n: number, locale?: string): string { + return Intl.NumberFormat(locale ?? "en-US").format(n); +}apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/components/table/logs-table.tsx (1)
493-499: LGTM! Number formatting correctly implemented.The implementation properly formats the pagination counts using
formatNumberFull, which addresses the requirements from issue #3922 to display large numbers with locale-appropriate separators.apps/dashboard/app/(app)/logs/components/table/logs-table.tsx (1)
268-274: LGTM! Pagination formatting correctly applied.The pagination footer now properly formats numbers using
formatNumberFull, making large counts more readable as per the PR objectives.apps/dashboard/app/(app)/ratelimits/[namespaceId]/_overview/components/table/logs-table.tsx (1)
235-240: LGTM! Consistent number formatting applied.The pagination footer correctly uses
formatNumberFullfor displaying counts, maintaining consistency with other tables in the application.apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/keys-list.tsx (2)
353-359: LGTM! Number formatting properly implemented.The pagination footer correctly formats the counts using
formatNumberFull, ensuring consistency across all table components and addressing the PR objectives.
66-89: Well-structured multi-select implementation.The addition of multi-select functionality with checkbox controls is cleanly implemented with proper state management and callbacks. The UI gracefully shows checkboxes on hover and selection states.
apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/components/table/logs-table.tsx (3)
66-81: Hook restructuring LGTMDestructuring and parameterization of
useRatelimitLogsQuerylooks clean and non‑breaking.
157-167: Limit column change LGTMLayout wrapper added; behavior preserved.
6-6: Hard‑coded en‑US formatter — make formatNumberFull locale‑aware or confirm US defaultapps/dashboard/lib/fmt.ts currently calls Intl.NumberFormat("en-US") and is imported across the dashboard, so this is a global behavioral change. Replace fmt.ts with a locale‑aware, memoized formatter (or explicitly keep en‑US and update the PR title to “consistent US number formatting”). Proposed change (apply to apps/dashboard/lib/fmt.ts):
const nfCache = new Map<string, Intl.NumberFormat>(); function resolveLocale(explicit?: string) { if (explicit) return explicit; try { // Stable across SSR/CSR when your app sets a single locale return Intl.NumberFormat().resolvedOptions().locale; } catch { return "en-US"; } } export function getNumberFormatter( locale?: string, options?: Intl.NumberFormatOptions, ) { const key = JSON.stringify([resolveLocale(locale), options ?? null]); let nf = nfCache.get(key); if (!nf) { nf = new Intl.NumberFormat(resolveLocale(locale), options); nfCache.set(key, nf); } return nf; } export function formatNumberFull(n: number, locale?: string): string { return getNumberFormatter(locale).format(n); }
| !realtimeLogs.some( | ||
| (realtime) => realtime.request_id === log.request_id | ||
| ) && ["opacity-50", "hover:opacity-100"], |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Avoid O(N) scan per row; precompute a Set of realtime IDs
realtimeLogs.some(...) runs for every row render. Use a Set for O(1) membership checks.
Apply this diff:
- !realtimeLogs.some(
- (realtime) => realtime.request_id === log.request_id
- ) && ["opacity-50", "hover:opacity-100"],
+ !realtimeIds.has(log.request_id) && ["opacity-50", "hover:opacity-100"],Support (outside selected lines):
const realtimeIds = useMemo(
() => new Set(realtimeLogs.map((r) => r.request_id)),
[realtimeLogs],
);🤖 Prompt for AI Agents
In
apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/components/table/logs-table.tsx
around lines 95 to 97, currently each row runs realtimeLogs.some(...) causing an
O(N) scan per row; precompute a Set of realtime request IDs with useMemo (depend
on realtimeLogs) and replace the per-row .some(...) check with a constant-time
Set.has(log.request_id) lookup so rendering is O(1) per row.
| export function formatNumber(n: number): string { | ||
| return Intl.NumberFormat("en", { notation: "compact" }).format(n); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consistency: unify locales and avoid per-call allocations (optional)
formatNumber uses "en" while formatNumberFull uses "en-US". Consider aligning both and optionally caching formatters at module scope to avoid repeated Intl allocations. Non-blocking.
🤖 Prompt for AI Agents
In apps/dashboard/lib/fmt.ts around lines 1 to 3, formatNumber currently uses
the locale "en" which is inconsistent with formatNumberFull's "en-US" and it
creates a new Intl.NumberFormat on every call; change the locale to "en-US" to
match the other function and move Intl.NumberFormat instances to module scope
(create and reuse a cached compact formatter and, if needed, a full formatter)
so the exported formatNumber simply calls the pre-created formatter.format(n) to
avoid per-call allocations.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/dashboard/app/(app)/projects/[projectId]/deployments/components/table/deployments-list.tsx (1)
132-136: CPU value is blank — wire up the actual fieldThe "Size" column renders an empty CPU value. This regresses the earlier “hardcoded 2 CPU” issue; now it shows nothing. Wire it to the runtime config CPU field (with a safe fallback).
Apply:
- <span className="font-semibold text-grayA-12 tabular-nums"> - - </span> + <span className="font-semibold text-grayA-12 tabular-nums"> + {deployment.runtimeConfig.cpu ?? "—"} + </span>If the field name differs (e.g., vcpu/cpus/cores), adjust accordingly. To confirm the correct field:
#!/usr/bin/env bash # Find CPU-like fields on runtimeConfig to wire up correctly rg -nP --type=ts '(interface|type)\s+.*RuntimeConfig|runtimeConfig\.(cpu|cpus|vcpu|vcpus|cores|memory)' -C2
🧹 Nitpick comments (2)
apps/dashboard/app/(app)/projects/[projectId]/deployments/components/table/deployments-list.tsx (2)
111-115: Optional: format large numbers with the shared formatterFor consistency with this PR’s objective, format the VM count and memory with the repo’s number formatter.
+import { formatNumberFull } from "@/lib/fmt"; ... - {deployment.runtimeConfig.regions.reduce( - (acc, region) => acc + region.vmCount, - 0, - )} + {formatNumberFull( + deployment.runtimeConfig.regions.reduce( + (acc, region) => acc + region.vmCount, + 0, + ), + )} ... - {deployment.runtimeConfig.memory} + {formatNumberFull(deployment.runtimeConfig.memory)}Also applies to: 140-141
283-283: Tighten useMemo deps to avoid unnecessary recomputeDepending on your data source, tracking whole query objects can churn the memo. Prefer stable slices.
- }, [selectedDeployment?.deployment.id, isCompactView, deployments, activeDeployment]); + }, [ + selectedDeployment?.deployment.id, + isCompactView, + deployments.data, + activeDeployment.data, + ]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/dashboard/app/(app)/projects/[projectId]/deployments/components/table/deployments-list.tsx(9 hunks)apps/dashboard/app/(app)/projects/_components/list/index.tsx(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/dashboard/app/(app)/projects/_components/list/index.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-25T19:09:43.284Z
Learnt from: mcstepp
PR: unkeyed/unkey#3662
File: apps/dashboard/lib/trpc/routers/deployment/list.ts:11-11
Timestamp: 2025-07-25T19:09:43.284Z
Learning: In apps/dashboard/lib/trpc/routers/deployment/list.ts, the listDeployments procedure intentionally queries the versions table rather than a deployments table. The user mcstepp indicated that renaming the table would require a database migration, which was deferred for the current PR focused on UI features.
Applied to files:
apps/dashboard/app/(app)/projects/[projectId]/deployments/components/table/deployments-list.tsx
🧬 Code graph analysis (1)
apps/dashboard/app/(app)/projects/[projectId]/deployments/components/table/deployments-list.tsx (2)
apps/dashboard/app/(app)/projects/[projectId]/deployments/components/table/components/skeletons.tsx (1)
ActionColumnSkeleton(78-89)apps/dashboard/lib/shorten-id.ts (1)
shortenId(5-54)
🔇 Additional comments (2)
apps/dashboard/app/(app)/projects/[projectId]/deployments/components/table/deployments-list.tsx (2)
33-35: LGTM: import formatting, selected styling, accent text, timestamp wrapper, and empty-state copyChanges look good and align with existing patterns.
Also applies to: 39-39, 61-61, 76-76, 163-163, 177-177, 213-217, 300-301
5-5: Whitespace-only changesNo action needed.
Also applies to: 81-81, 293-293
| @@ -407,15 +290,15 @@ export const DeploymentsList = ({ projectId }: Props) => { | |||
| onRowClick={setSelectedDeployment} | |||
| selectedItem={selectedDeployment} | |||
| keyExtractor={(deployment) => deployment.id} | |||
There was a problem hiding this comment.
Fix keyExtractor to use row.deployment.id
VirtualTable rows appear to be shaped as { deployment, environment }. The current extractor uses the row itself as a Deployment, which likely produces undefined/duplicate keys.
- keyExtractor={(deployment) => deployment.id}
+ keyExtractor={(row) => row.deployment.id}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| keyExtractor={(deployment) => deployment.id} | |
| keyExtractor={(row) => row.deployment.id} |
🤖 Prompt for AI Agents
In
apps/dashboard/app/(app)/projects/[projectId]/deployments/components/table/deployments-list.tsx
around line 292, the keyExtractor currently treats the row as a Deployment and
returns deployment.id from the row itself, but rows are shaped as { deployment,
environment } so this yields undefined/duplicate keys; update the keyExtractor
to return row.deployment.id (or otherwise access the nested deployment object)
to ensure a stable, unique key for each VirtualTable row.
|
Can you please go through coderabbit comments and fix the conflicts? |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
apps/dashboard/app/(app)/projects/_components/list/index.tsx (1)
15-21: Add missing imports — import collection & useLiveQuery; confirm ilike sourceuseLiveQuery and collection are used elsewhere in the repo with these canonical imports; add them to the top of apps/dashboard/app/(app)/projects/_components/list/index.tsx. ilike was not found — confirm which package exports it before importing.
+import { collection } from "@/lib/collections"; +import { useLiveQuery } from "@tanstack/react-db";apps/dashboard/app/(app)/projects/[projectId]/deployments/components/table/deployments-list.tsx (2)
42-47: Blocker: Hooks used at module scope; component wrapper missing.
useIsMobile(and later state access) are invoked outside any component, which will crash at runtime and fail TS/ESLint rules. Reintroduce the component function and the missing selection state.Apply:
+export const DeploymentsList = () => { - const isCompactView = useIsMobile({ breakpoint: COMPACT_BREAKPOINT }); + const isCompactView = useIsMobile({ breakpoint: COMPACT_BREAKPOINT }); + + const [selectedDeployment, setSelectedDeployment] = useState<{ + deployment: Deployment; + environment?: Environment; + } | null>(null);And ensure the closing brace at the end of the file closes this component (no extra stray brace). If this file previously exported the component, keep the named export.
47-55: Add missing type imports for Deployment and Environment.Add
import type { Deployment, Environment } from '@/lib/collections';to the top of apps/dashboard/app/(app)/projects/[projectId]/deployments/components/table/deployments-list.tsx.apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/components/table/logs-table.tsx (1)
169-177: 0 ms duration renders as (truthy check bug)Treat 0 as a valid value.
Apply:
- const parsedDuration = safeParseJson(log.request_body)?.duration; + const durationMs = safeParseJson(log.request_body)?.duration; return ( <div className="font-mono"> - {parsedDuration ? msToSeconds(parsedDuration) : "<EMPTY>"} + {typeof durationMs === "number" ? msToSeconds(durationMs) : "<EMPTY>"} </div> );
♻️ Duplicate comments (4)
apps/dashboard/app/(app)/projects/[projectId]/deployments/components/table/deployments-list.tsx (2)
132-147: CPU value missing in “Size” column.The CPU span is empty, causing a blank “CPU” readout.
Apply (confirm the exact field name):
- <span className="font-semibold text-grayA-12 tabular-nums"> - - </span> + <span className="font-semibold text-grayA-12 tabular-nums"> + {deployment.runtimeConfig?.cpu ?? "—"} + </span>If the correct property is
vCPU,cpus, or similar, adjust accordingly. I can update once you confirm the runtimeConfig shape.
298-299: Fix VirtualTable keyExtractor to use row.deployment.id.Rows appear shaped as
{ deployment, environment }. Current extractor likely returnsundefined, causing unstable keys.Apply:
- keyExtractor={(deployment) => deployment.id} + keyExtractor={(row) => row.deployment.id}apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/components/table/logs-table.tsx (2)
95-97: Avoid O(N) scan per row; precompute Set of realtime IDsReplace realtimeLogs.some(...) with Set.has(...) for O(1) checks.
Apply:
- !realtimeLogs.some( - (realtime) => realtime.request_id === log.request_id - ) && ["opacity-50", "hover:opacity-100"], + !realtimeIds.has(log.request_id) && ["opacity-50", "hover:opacity-100"],Support (outside selected lines, inside component):
const realtimeIds = useMemo( () => new Set(realtimeLogs.map((r) => r.request_id)), [realtimeLogs], );
239-245: Footer counts: Good switch to shared formatterThis addresses the earlier request to avoid inline Intl.NumberFormat() in the footer.
🧹 Nitpick comments (6)
apps/dashboard/app/(app)/projects/_components/list/index.tsx (2)
47-49: Empty description element
<Empty.Description>has no content; it adds noise/spacing. Remove or provide helpful copy.- <Empty.Description className="text-left"> - - </Empty.Description> + {/* Consider adding guidance or omit entirely */}
2-8: Unused imports (current state)
Dotsis unused;ProjectActionsandProjectCardare unused until the populated-state render is restored.-import { BookBookmark, Dots } from "@unkey/icons"; +import { BookBookmark } from "@unkey/icons";apps/dashboard/app/(app)/projects/[projectId]/deployments/components/table/deployments-list.tsx (1)
216-225: Remove duplicated typography classes around TimestampInfo.Wrapper and child both set
font-mono text-xs text-gray-9. Keep it in one place.Apply:
- <div - className={cn( - "font-mono text-xs mt-1", - "text-gray-9" - )} - > + <div className="mt-1"> <TimestampInfo value={deployment.createdAt} className="font-mono text-xs text-gray-9" /> </div>apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/components/table/logs-table.tsx (3)
160-165: Avoid double JSON parse in the cellParse once and reuse.
Apply:
- render: (log) => { - return ( - <div className="font-mono"> - {safeParseJson(log.response_body)?.limit ?? "<EMPTY>"} - </div> - ); - }, + render: (log) => { + const response = safeParseJson(log.response_body); + return ( + <div className="font-mono">{response?.limit ?? "<EMPTY>"}</div> + ); + },
190-196: Reuse parsedReset; remove fallback that can pass a string to TimestampInfoAvoid reparsing and ensure type safety.
Apply:
- <TimestampInfo - value={safeParseJson(log.response_body)?.reset ?? "<EMPTY>"} + <TimestampInfo + value={parsedReset} className={cn(
200-201: Normalize placeholder casingUse "" to match other cells.
Apply:
- "<Empty>" + "<EMPTY>"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/dashboard/app/(app)/projects/[projectId]/deployments/components/table/deployments-list.tsx(9 hunks)apps/dashboard/app/(app)/projects/_components/list/index.tsx(3 hunks)apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/components/table/logs-table.tsx(11 hunks)
🧰 Additional context used
🧠 Learnings (8)
📚 Learning: 2025-07-28T20:38:53.244Z
Learnt from: mcstepp
PR: unkeyed/unkey#3662
File: apps/dashboard/app/(app)/projects/[projectId]/diff/[...compare]/components/client.tsx:322-341
Timestamp: 2025-07-28T20:38:53.244Z
Learning: In apps/dashboard/app/(app)/projects/[projectId]/diff/[...compare]/components/client.tsx, mcstepp prefers to keep hardcoded endpoint logic in the getDiffType function during POC phases for demonstrating diff functionality, rather than implementing a generic diff algorithm. This follows the pattern of keeping simplified implementations for demonstration purposes.
Applied to files:
apps/dashboard/app/(app)/projects/_components/list/index.tsx
📚 Learning: 2025-07-28T19:42:37.047Z
Learnt from: mcstepp
PR: unkeyed/unkey#3662
File: apps/dashboard/app/(app)/projects/page.tsx:74-81
Timestamp: 2025-07-28T19:42:37.047Z
Learning: In apps/dashboard/app/(app)/projects/page.tsx, the user mcstepp prefers to keep placeholder functions like generateSlug inline during POC/demonstration phases rather than extracting them to utility modules, with plans to refactor later when the feature matures beyond the proof-of-concept stage.
Applied to files:
apps/dashboard/app/(app)/projects/_components/list/index.tsx
📚 Learning: 2025-07-25T19:09:43.284Z
Learnt from: mcstepp
PR: unkeyed/unkey#3662
File: apps/dashboard/lib/trpc/routers/deployment/list.ts:11-11
Timestamp: 2025-07-25T19:09:43.284Z
Learning: In apps/dashboard/lib/trpc/routers/deployment/list.ts, the listDeployments procedure intentionally queries the versions table rather than a deployments table. The user mcstepp indicated that renaming the table would require a database migration, which was deferred for the current PR focused on UI features.
Applied to files:
apps/dashboard/app/(app)/projects/[projectId]/deployments/components/table/deployments-list.tsx
📚 Learning: 2025-09-12T17:57:12.151Z
Learnt from: mcstepp
PR: unkeyed/unkey#3952
File: apps/dashboard/app/(app)/projects/[projectId]/deployments/components/rollback-dialog.tsx:111-116
Timestamp: 2025-09-12T17:57:12.151Z
Learning: In the Deployment type used across the dashboard deployment components, the source field is required and will always be present, so optional chaining (source?.branch) is not needed when accessing source.branch or source.gitSha.
Applied to files:
apps/dashboard/app/(app)/projects/[projectId]/deployments/components/table/deployments-list.tsx
📚 Learning: 2025-08-25T13:46:08.303Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3834
File: apps/dashboard/app/(app)/ratelimits/_components/controls/components/namespace-list-datetime/index.tsx:1-1
Timestamp: 2025-08-25T13:46:08.303Z
Learning: The NamespaceListDateTime component in apps/dashboard/app/(app)/ratelimits/_components/controls/components/namespace-list-datetime/index.tsx is intentionally designed to use the overview hook (useFilters from @/app/(app)/ratelimits/[namespaceId]/_overview/hooks/use-filters) rather than the namespace list hook, as clarified by ogzhanolguncu. This coupling is by design, not an architectural issue.
Applied to files:
apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/components/table/logs-table.tsx
📚 Learning: 2024-12-03T14:17:08.016Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#2143
File: apps/dashboard/app/(app)/logs/logs-page.tsx:77-83
Timestamp: 2024-12-03T14:17:08.016Z
Learning: The `<LogsTable />` component already implements virtualization to handle large datasets efficiently.
Applied to files:
apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/components/table/logs-table.tsx
📚 Learning: 2025-08-25T13:46:34.441Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3834
File: apps/dashboard/app/(app)/ratelimits/_components/controls/components/namespace-list-refresh.tsx:4-4
Timestamp: 2025-08-25T13:46:34.441Z
Learning: The namespace list refresh component (apps/dashboard/app/(app)/ratelimits/_components/controls/components/namespace-list-refresh.tsx) intentionally uses the overview hook (useFilters from @/app/(app)/ratelimits/[namespaceId]/_overview/hooks/use-filters) rather than a namespace-specific hook. This cross-coupling between namespace list components and overview hooks is an architectural design decision.
Applied to files:
apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/components/table/logs-table.tsx
📚 Learning: 2025-02-05T12:56:44.873Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#2866
File: apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/components/table/log-details/components/log-footer.tsx:85-98
Timestamp: 2025-02-05T12:56:44.873Z
Learning: The RequestResponseDetails component in the ratelimit logs UI already handles empty content cases by preventing rendering when content is empty, so no additional empty state handling is needed in the parent components.
Applied to files:
apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/components/table/logs-table.tsx
🧬 Code graph analysis (2)
apps/dashboard/app/(app)/projects/[projectId]/deployments/components/table/deployments-list.tsx (2)
apps/dashboard/app/(app)/projects/[projectId]/deployments/components/table/components/skeletons.tsx (1)
ActionColumnSkeleton(78-89)apps/dashboard/lib/shorten-id.ts (1)
shortenId(5-54)
apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/components/table/logs-table.tsx (2)
apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/components/table/hooks/use-logs-query.ts (1)
useRatelimitLogsQuery(17-208)apps/dashboard/lib/fmt.ts (1)
formatNumberFull(5-7)
🔇 Additional comments (10)
apps/dashboard/app/(app)/projects/[projectId]/deployments/components/table/deployments-list.tsx (4)
29-38: Dynamic import + skeleton fallback looks good.Lazy-loading actions popover with SSR disabled is appropriate here.
77-82: Styling tweak LGTM.Accent text for the Deployment ID improves contrast/readability.
159-167: Selection highlight depends on missing state.Once the component/state fix is applied, this conditional class will work as intended.
After adding
selectedDeploymentstate, verify row selection visual updates correctly on click.
305-308: Empty state copy LGTM.Clear guidance and link to docs.
apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/components/table/logs-table.tsx (6)
67-81: Hook wiring and query params: LGTM
119-121: Pointer-events gating on other rows: LGTM
131-135: Identifier cell reflow: LGTM
148-149: Status badge styling: LGTM
217-217: Columns memo deps: LGTMScoped to selection only; avoids unnecessary re-creation.
255-257: Empty state copy reflow: LGTM
| import { safeParseJson } from "@/app/(app)/logs/utils"; | ||
| import { VirtualTable } from "@/components/virtual-table/index"; | ||
| import type { Column } from "@/components/virtual-table/types"; | ||
| import { formatNumberFull } from "@/lib/fmt"; |
There was a problem hiding this comment.
Don't force en-US; honor user locale and cache formatter
Import looks good, but current formatNumberFull uses Intl.NumberFormat("en-US"), which conflicts with the PR goal of “locale-appropriate” formatting and allocates a formatter per call. Switch the helper to default-locale (or param) and cache per locale.
Support (outside selected lines, apps/dashboard/lib/fmt.ts):
-export function formatNumberFull(n: number): string {
- return Intl.NumberFormat("en-US").format(n);
-}
+const nfCache = new Map<string | undefined, Intl.NumberFormat>();
+function getNF(locale?: string) {
+ if (!nfCache.has(locale)) nfCache.set(locale, new Intl.NumberFormat(locale));
+ return nfCache.get(locale)!;
+}
+export function formatNumberFull(n: number, locale?: string): string {
+ return getNF(locale).format(n);
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { formatNumberFull } from "@/lib/fmt"; | |
| // apps/dashboard/lib/fmt.ts | |
| const nfCache = new Map<string | undefined, Intl.NumberFormat>(); | |
| function getNF(locale?: string) { | |
| if (!nfCache.has(locale)) nfCache.set(locale, new Intl.NumberFormat(locale)); | |
| return nfCache.get(locale)!; | |
| } | |
| export function formatNumberFull(n: number, locale?: string): string { | |
| return getNF(locale).format(n); | |
| } |
- Use early return instead of break in getSelectedKeysState for better readability - Apply code formatting fixes from biome
|
neha seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/keys-list.tsx (1)
101-110: Make the tooltip trigger keyboard-focusable; sync hover with focus (repeat of prior ask)Add focusability + role/label to the trigger and set hover state on focus so keyboard users can reveal/toggle the checkbox.
const iconContainer = ( <div className={cn( "size-5 rounded flex items-center justify-center cursor-pointer", identity ? "bg-successA-3" : "bg-grayA-3", isSelected && "bg-brand-5" )} onMouseEnter={() => setHoveredKeyId(key.id)} onMouseLeave={() => setHoveredKeyId(null)} + onFocus={() => setHoveredKeyId(key.id)} + onBlur={() => setHoveredKeyId(null)} + tabIndex={0} + role="button" + aria-label={identity ? "Key identity info" : "Key info"} >Optionally reinforce at the clone site (works even if the container changes later):
- > - {React.cloneElement(iconContainer, { - className: cn( - iconContainer.props.className, - "cursor-pointer" - ), - })} + > + {React.cloneElement(iconContainer, { + className: cn(iconContainer.props.className, "cursor-pointer"), + tabIndex: 0, + role: "button", + "aria-label": "Key identity info", + })} </InfoTooltip>Also applies to: 181-185
🧹 Nitpick comments (3)
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/keys-list.tsx (3)
4-4: Locale-aware number formatting is hard-coded to en-US; also re-instantiates per callPR goal says “locale‑appropriate,” but formatNumberFull currently fixes locale to "en-US" and constructs a formatter per call. Prefer a single, memoized formatter defaulting to the user’s locale.
Proposed change in apps/dashboard/lib/fmt.ts:
-export function formatNumberFull(n: number): string { - return Intl.NumberFormat("en-US").format(n); -} +// Memoize once; default to user locale in browser, system default otherwise. +const nfFull = + typeof window !== "undefined" + ? new Intl.NumberFormat(navigator.language) + : new Intl.NumberFormat(undefined); + +export function formatNumberFull(n: number): string { + return nfFull.format(n); +}Please confirm product intent: should counts follow the viewer’s locale or be normalized (e.g., always en-US)? If the latter, keep "en-US" but still memoize.
292-323: Fix edge case: returns “all-enabled” when none of the selected IDs exist in the current keys listIf selectedKeys contains only stale/filtered-out IDs, foundCount stays 0 and both flags remain true, yielding “all-enabled.” Return null in that case.
const getSelectedKeysState = useCallback(() => { if (selectedKeys.size === 0) { return null; } let allEnabled = true; let allDisabled = true; + let foundCount = 0; for (const keyId of selectedKeys) { const key = keys.find((k) => k.id === keyId); if (!key) { continue; } + foundCount++; if (key.enabled) { allDisabled = false; } else { allEnabled = false; } // Early exit if we already found a mix if (!allEnabled && !allDisabled) { return "mixed"; } } + if (foundCount === 0) { + return null; + } if (allEnabled) { return "all-enabled"; } - - return "all-disabled"; + return "all-disabled"; }, [selectedKeys, keys]);
351-357: Nit: consistent emphasis for both numbersStyle-only: wrap totalCount with the same accent as the first number for visual consistency.
<span>of</span> - {formatNumberFull(totalCount)} + <span className="text-accent-12"> + {formatNumberFull(totalCount)} + </span> <span>keys</span>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/components/table/logs-table.tsx(2 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/keys-list.tsx(13 hunks)apps/dashboard/app/(app)/apis/_components/api-list-client.tsx(2 hunks)apps/dashboard/app/(app)/apis/_components/api-list-grid.tsx(2 hunks)apps/dashboard/app/(app)/authorization/permissions/components/table/permissions-list.tsx(2 hunks)apps/dashboard/app/(app)/authorization/roles/components/table/roles-list.tsx(2 hunks)apps/dashboard/app/(app)/logs/components/table/logs-table.tsx(2 hunks)apps/dashboard/app/(app)/projects/[projectId]/deployments/components/table/deployments-list.tsx(2 hunks)apps/dashboard/app/(app)/projects/_components/list/index.tsx(2 hunks)apps/dashboard/app/(app)/ratelimits/[namespaceId]/_overview/components/table/logs-table.tsx(2 hunks)apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/components/table/logs-table.tsx(2 hunks)apps/dashboard/app/(app)/settings/root-keys/components/table/root-keys-list.tsx(3 hunks)apps/dashboard/components/virtual-table/components/loading-indicator.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (12)
- apps/dashboard/app/(app)/projects/_components/list/index.tsx
- apps/dashboard/app/(app)/projects/[projectId]/deployments/components/table/deployments-list.tsx
- apps/dashboard/app/(app)/settings/root-keys/components/table/root-keys-list.tsx
- apps/dashboard/app/(app)/authorization/roles/components/table/roles-list.tsx
- apps/dashboard/app/(app)/apis/_components/api-list-grid.tsx
- apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/components/table/logs-table.tsx
- apps/dashboard/app/(app)/apis/_components/api-list-client.tsx
- apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/components/table/logs-table.tsx
- apps/dashboard/components/virtual-table/components/loading-indicator.tsx
- apps/dashboard/app/(app)/authorization/permissions/components/table/permissions-list.tsx
- apps/dashboard/app/(app)/ratelimits/[namespaceId]/_overview/components/table/logs-table.tsx
- apps/dashboard/app/(app)/logs/components/table/logs-table.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-04-24T14:34:30.621Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3115
File: apps/dashboard/components/logs/checkbox/filters-popover.tsx:33-55
Timestamp: 2025-04-24T14:34:30.621Z
Learning: In the ShortcutActivator component within filters-popover.tsx, the purpose is to track keys separately for each filter item, providing a registration mechanism for shortcuts passed to it rather than enforcing specific key combinations like option+shift+key.
Applied to files:
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/keys-list.tsx
🧬 Code graph analysis (1)
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/keys-list.tsx (3)
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/hooks/use-keys-list-query.ts (1)
useKeysListQuery(13-85)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/hidden-value.tsx (1)
HiddenValueCell(5-47)apps/dashboard/lib/fmt.ts (1)
formatNumberFull(5-7)
🔇 Additional comments (2)
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/keys-list.tsx (2)
32-32: LGTM: dynamic import and loading placeholderNamed export selection and focus-visible styles on the fallback button look good.
Also applies to: 41-47, 50-50
112-116: LGTM: UI/UX polishLoading state wrapper, identity badge styling, HiddenValueCell props split, adjusted paddings, and skeleton fallback condition read well.
Also applies to: 167-174, 224-228, 398-398, 408-418
- Use memoized number formatters instead of creating new instances - Extract inline array to constant to avoid recreation - Fix type safety by replacing 'any' with proper type assertion - Apply code formatting fixes
|
If you sign the CLA above and resolve the coderabbits comments you fixed, we can move forward. And, please let's not fix anything other than formatting pagination numbers otherwise this becomes a nightmare to review and we might accidentally regress stuff. |
|
Hello @ogzhanolguncu |
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
| @@ -1,5 +1,4 @@ | |||
| import { collection } from "@/lib/collections"; | |||
| import { ilike, useLiveQuery } from "@tanstack/react-db"; | |||
|
|
|||
There was a problem hiding this comment.
Critical syntax errors: Missing required imports for collection, ilike, and useLiveQuery, and malformed JSX with missing closing </div> tag.
View Details
📝 Patch Details
diff --git a/apps/dashboard/app/(app)/projects/_components/list/index.tsx b/apps/dashboard/app/(app)/projects/_components/list/index.tsx
index b0d569ece..e2d6fe4ea 100644
--- a/apps/dashboard/app/(app)/projects/_components/list/index.tsx
+++ b/apps/dashboard/app/(app)/projects/_components/list/index.tsx
@@ -1,6 +1,8 @@
import { BookBookmark, Dots } from "@unkey/icons";
import { Button, Empty } from "@unkey/ui";
+import { ilike, useLiveQuery } from "@tanstack/react-db";
+import { collection } from "@/lib/collections";
import { useProjectsFilters } from "../hooks/use-projects-filters";
import { ProjectActions } from "./project-actions";
import { ProjectCard } from "./projects-card";
@@ -101,6 +103,6 @@ export const ProjectsList = () => {
/>
))}
</div>
-
+ </div>
);
};
Analysis
Critical syntax errors in ProjectsList component: Missing imports and unclosed JSX tag
What fails: File apps/dashboard/app/(app)/projects/_components/list/index.tsx has compilation-blocking errors preventing TypeScript compilation
How to reproduce:
cd apps/dashboard && pnpm run typecheckResult: TypeScript compiler fails with errors:
- Line 15:
Cannot find name 'useLiveQuery' - Line 18:
Cannot find name 'collection' - Line 19:
Cannot find name 'ilike' - Line 70:
JSX element 'div' has no corresponding closing tag
Expected: File should compile without TypeScript errors
Fix applied:
- Added missing imports:
import { ilike, useLiveQuery } from "@tanstack/react-db";andimport { collection } from "@/lib/collections"; - Added missing closing
</div>tag to complete JSX structure
| import type { Column } from "@/components/virtual-table/types"; | ||
| import { useIsMobile } from "@/hooks/use-mobile"; | ||
| import type { Deployment, Environment } from "@/lib/collections"; | ||
|
|
There was a problem hiding this comment.
Missing type imports cause compilation error: Deployment and Environment types are used but their import statement was removed.
View Details
📝 Patch Details
diff --git a/apps/dashboard/app/(app)/projects/[projectId]/deployments/components/table/deployments-list.tsx b/apps/dashboard/app/(app)/projects/[projectId]/deployments/components/table/deployments-list.tsx
index afe222b25..9876d5843 100644
--- a/apps/dashboard/app/(app)/projects/[projectId]/deployments/components/table/deployments-list.tsx
+++ b/apps/dashboard/app/(app)/projects/[projectId]/deployments/components/table/deployments-list.tsx
@@ -2,6 +2,7 @@
import { VirtualTable } from "@/components/virtual-table/index";
import type { Column } from "@/components/virtual-table/types";
import { useIsMobile } from "@/hooks/use-mobile";
+import type { Deployment, Environment } from "@/lib/collections";
import { shortenId } from "@/lib/shorten-id";
import { BookBookmark, Cloud, CodeBranch, Cube } from "@unkey/icons";
Analysis
Missing type imports cause TypeScript compilation errors in deployments-list.tsx
What fails: TypeScript compilation fails with "Cannot find name 'Deployment'" and "Cannot find name 'Environment'" errors in deployments-list.tsx on lines 44-45 and throughout the component
How to reproduce:
cd apps/dashboard
pnpm exec tsc --noEmit --isolatedModules --jsx react-jsx app/\(app\)/projects/\[projectId\]/deployments/components/table/deployments-list.tsxResult: TypeScript errors:
error TS2304: Cannot find name 'Deployment'.
error TS2304: Cannot find name 'Environment'.
Expected: Code should compile without type errors since the types are defined and exported from @/lib/collections
Fix: Added import type { Deployment, Environment } from "@/lib/collections"; to resolve the missing type definitions used in component state and prop declarations.
| @@ -1,5 +1,4 @@ | |||
| import { collection } from "@/lib/collections"; | |||
| import { ilike, useLiveQuery } from "@tanstack/react-db"; | |||
There was a problem hiding this comment.
Missing imports cause compilation error: collection, useLiveQuery, and ilike are used but not imported after the import statements were accidentally removed.
View Details
📝 Patch Details
diff --git a/apps/dashboard/app/(app)/projects/_components/list/index.tsx b/apps/dashboard/app/(app)/projects/_components/list/index.tsx
index b0d569ece..e2d6fe4ea 100644
--- a/apps/dashboard/app/(app)/projects/_components/list/index.tsx
+++ b/apps/dashboard/app/(app)/projects/_components/list/index.tsx
@@ -1,6 +1,8 @@
import { BookBookmark, Dots } from "@unkey/icons";
import { Button, Empty } from "@unkey/ui";
+import { ilike, useLiveQuery } from "@tanstack/react-db";
+import { collection } from "@/lib/collections";
import { useProjectsFilters } from "../hooks/use-projects-filters";
import { ProjectActions } from "./project-actions";
import { ProjectCard } from "./projects-card";
@@ -101,6 +103,6 @@ export const ProjectsList = () => {
/>
))}
</div>
-
+ </div>
);
};
Analysis
Missing imports cause TypeScript compilation errors in ProjectsList component
What fails: ProjectsList component in apps/dashboard/app/(app)/projects/_components/list/index.tsx uses useLiveQuery, collection.projects, and ilike on lines 15-19 without importing them, causing TypeScript compilation errors.
How to reproduce:
cd apps/dashboard && pnpm run typecheckResult: TypeScript compiler reports:
app/(app)/projects/_components/list/index.tsx(15,20): error TS2304: Cannot find name 'useLiveQuery'.
app/(app)/projects/_components/list/index.tsx(18,26): error TS2304: Cannot find name 'collection'.
app/(app)/projects/_components/list/index.tsx(19,33): error TS2304: Cannot find name 'ilike'.
Expected: Clean compilation with proper imports from @tanstack/react-db and @/lib/collections per TanStack DB React documentation
4 participants
What does this PR do?
Implements proper number formatting for pagination counts in log footers across the dashboard. Large numbers like
1239436are now displayed as1,239,436(or locale-appropriate formatting) making them much more readable for users.new Intl.NumberFormat().format(number)to all "Showing X of Y" pagination text in:fixes #3922
Type of change
How should this be tested?
Checklist
Required
pnpm buildpnpm fmtconsole.logsgit pull origin mainSummary by CodeRabbit
New Features
Style