fix(lint): resolve all 27 Biome warnings — zero warnings#143
fix(lint): resolve all 27 Biome warnings — zero warnings#143BillChirico merged 25 commits intomainfrom
Conversation
|
Warning Rate limit exceeded
⌛ 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 (6)
📝 WalkthroughWalkthroughThis PR fixes various lint and accessibility issues and small runtime-safety bugs: replaces array-index keys with stable string keys, adds semantic elements and explicit button types, removes non-null assertions, adjusts pagination and abort handling, tweaks log row rendering, and adds an Changes
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across the lint fixes:
🔴 config-editor.tsx:1317 — Activity badge key derived from editable fields (badge.label:badge.days) causes React to unmount/remount inputs on every keystroke, losing focus. Functional regression — index-based keys were correct here.
🟡 analytics-dashboard.tsx:425,438 — aria-label attributes dropped during <p> → <div> migration. The element change also doesn't correspond to any listed Biome rule.
🔵 log-viewer.tsx:106-111 — Redundant onKeyDown handler on native <button> (Enter/Space already fire click natively).
There was a problem hiding this comment.
Pull request overview
Resolves pre-existing Biome lint warnings across the web dashboard and tests, aiming to reduce lint noise and improve code quality without intended functional changes.
Changes:
- Replaced index-based React keys with stable keys/composites across multiple dashboard components.
- Removed non-null assertions via optional chaining / nullish coalescing, and adjusted a Discord pagination loop to avoid constant-condition warnings.
- Improved semantics/accessibility patterns (e.g., interactive rows, semantic elements) and removed an unused test import.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| web/src/lib/discord.server.ts | Removes non-null assertion and refactors guild pagination loop to avoid while(true). |
| web/src/lib/auth.ts | Replaces non-null assertions for Discord OAuth env vars with ?? '' fallbacks. |
| web/src/components/dashboard/system-prompt-editor.tsx | Uses <output> for the character counter and adjusts ARIA attributes. |
| web/src/components/dashboard/restart-history.tsx | Replaces index keys in skeleton rows with stable keys. |
| web/src/components/dashboard/member-table.tsx | Replaces index keys in skeleton rows with stable keys. |
| web/src/components/dashboard/log-viewer.tsx | Replaces interactive div with conditional <button> rendering and tweaks auto-scroll guard. |
| web/src/components/dashboard/health-cards.tsx | Replaces index keys in skeleton cards with stable keys. |
| web/src/components/dashboard/config-editor.tsx | Removes non-null assertion, swaps some role="status" containers to <output>, and updates keys in editable lists. |
| web/src/components/dashboard/config-diff.tsx | Adds an id field for diff lines and uses semantic <section>. |
| web/src/components/dashboard/case-table.tsx | Adds type="button" and replaces index keys in loading skeleton rows. |
| web/src/components/dashboard/analytics-dashboard.tsx | Replaces index keys for KPI skeletons; minor markup adjustments. |
| web/src/app/login/page.tsx | Uses optional chaining for callback URL validation. |
| web/src/app/dashboard/members/[userId]/page.tsx | Replaces index keys in skeleton cards with stable keys. |
| tests/commands/challenge.test.js | Removes an unused import. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/src/components/dashboard/config-editor.tsx`:
- Around line 532-535: The loading/unsaved-change announcement <output> element
lacks live-region semantics; update the JSX in config-editor.tsx (the <output
className="flex items-center justify-center py-12"> used around Loader2 and the
visually-hidden <span>) to include a live region attribute such as role="status"
or aria-live="polite" so screen readers announce changes; apply the same change
to the other matching output block around lines 592-598 to restore dynamic
announcement behavior.
- Around line 1316-1319: The list row key is derived from editable fields
(`badge.label`, `badge.days`) so editing or duplicate values breaks identity;
change to a stable per-row identifier (e.g., add an `id` property to each badge
object when the badges array is initialized/rows are added, or maintain a
parallel stable-id array via useRef/useState) and use that `id` for the React
key in the map instead of `${badge.label}:${badge.days}`; update the
add/remove/update flows (where badges are created/modified) to preserve the
stable `id` so keys never change during edits.
In `@web/src/components/dashboard/log-viewer.tsx`:
- Around line 101-123: The expanded metadata block (rendering JSON from
entry.meta) must be moved out of the interactive <button> so selecting/copying
it won't trigger onToggle; keep the button rendering mainRow and its
aria-expanded tied to isExpanded and onClick={onToggle} but remove the manual
onKeyDown handler entirely (native button behavior is sufficient). After the
button, render the metadata as a sibling <div> (conditionally shown when
isExpanded) with the same classes (mt-1 ml-14 rounded bg-gray-900 p-2
text-gray-400 and inner <pre> formatting) so layout is preserved while
preventing collapse when interacting with the content.
In `@web/src/lib/auth.ts`:
- Around line 60-61: Add defensive validation in refreshDiscordToken: call
validateEnv() (or replicate its required checks for DISCORD_CLIENT_ID and
DISCORD_CLIENT_SECRET) at the start of refreshDiscordToken before reading
client_id/client_secret, and throw or return an explicit error if credentials
are missing; this ensures refreshDiscordToken can be called safely in tests or
other callers without relying on getAuthOptions to have validated the env first.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (14)
tests/commands/challenge.test.jsweb/src/app/dashboard/members/[userId]/page.tsxweb/src/app/login/page.tsxweb/src/components/dashboard/analytics-dashboard.tsxweb/src/components/dashboard/case-table.tsxweb/src/components/dashboard/config-diff.tsxweb/src/components/dashboard/config-editor.tsxweb/src/components/dashboard/health-cards.tsxweb/src/components/dashboard/log-viewer.tsxweb/src/components/dashboard/member-table.tsxweb/src/components/dashboard/restart-history.tsxweb/src/components/dashboard/system-prompt-editor.tsxweb/src/lib/auth.tsweb/src/lib/discord.server.ts
💤 Files with no reviewable changes (1)
- tests/commands/challenge.test.js
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Greptile Review
- GitHub Check: Agent
- GitHub Check: claude-review
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,tsx}: Always use semicolons
Use single quotes — enforced by Biome
Use 2-space indentation — enforced by Biome
Files:
web/src/components/dashboard/system-prompt-editor.tsxweb/src/components/dashboard/member-table.tsxweb/src/app/dashboard/members/[userId]/page.tsxweb/src/components/dashboard/restart-history.tsxweb/src/lib/auth.tsweb/src/components/dashboard/config-diff.tsxweb/src/components/dashboard/analytics-dashboard.tsxweb/src/components/dashboard/config-editor.tsxweb/src/components/dashboard/health-cards.tsxweb/src/app/login/page.tsxweb/src/lib/discord.server.tsweb/src/components/dashboard/case-table.tsxweb/src/components/dashboard/log-viewer.tsx
web/**/*.{tsx,ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use next/image Image component with appropriate layout and sizing props in Next.js components
Files:
web/src/components/dashboard/system-prompt-editor.tsxweb/src/components/dashboard/member-table.tsxweb/src/app/dashboard/members/[userId]/page.tsxweb/src/components/dashboard/restart-history.tsxweb/src/lib/auth.tsweb/src/components/dashboard/config-diff.tsxweb/src/components/dashboard/analytics-dashboard.tsxweb/src/components/dashboard/config-editor.tsxweb/src/components/dashboard/health-cards.tsxweb/src/app/login/page.tsxweb/src/lib/discord.server.tsweb/src/components/dashboard/case-table.tsxweb/src/components/dashboard/log-viewer.tsx
web/src/**/*.{tsx,ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use Zustand store (zustand) for state management in React components; implement fetch-on-demand pattern in stores
Files:
web/src/components/dashboard/system-prompt-editor.tsxweb/src/components/dashboard/member-table.tsxweb/src/app/dashboard/members/[userId]/page.tsxweb/src/components/dashboard/restart-history.tsxweb/src/lib/auth.tsweb/src/components/dashboard/config-diff.tsxweb/src/components/dashboard/analytics-dashboard.tsxweb/src/components/dashboard/config-editor.tsxweb/src/components/dashboard/health-cards.tsxweb/src/app/login/page.tsxweb/src/lib/discord.server.tsweb/src/components/dashboard/case-table.tsxweb/src/components/dashboard/log-viewer.tsx
🧬 Code graph analysis (4)
web/src/components/dashboard/member-table.tsx (1)
web/src/components/ui/table.tsx (1)
TableRow(92-92)
web/src/components/dashboard/analytics-dashboard.tsx (1)
web/src/lib/analytics-utils.ts (1)
formatNumber(50-52)
web/src/lib/discord.server.ts (2)
src/api/routes/guilds.js (1)
page(44-44)src/api/routes/members.js (1)
after(163-163)
web/src/components/dashboard/case-table.tsx (1)
web/src/components/ui/table.tsx (1)
TableRow(92-92)
🔇 Additional comments (17)
web/src/app/login/page.tsx (1)
16-19: LGTM!The optional chaining refactor is correct. The expression
rawCallbackUrl?.startsWith('/')safely returnsundefinedwhenrawCallbackUrlis nullish, which short-circuits the&&and falls back to/dashboard. The open redirect protection (rejecting//prefix) remains intact.web/src/components/dashboard/system-prompt-editor.tsx (1)
83-96: Good semantic replacement for the counter element.Switching to
<output>here is clean and keeps the rendering/accessibility behavior intact for this counter.web/src/app/dashboard/members/[userId]/page.tsx (1)
357-358: Stable skeleton keys look correct.The explicit key list removes index-based keying without changing loading UI behavior.
web/src/components/dashboard/restart-history.tsx (1)
102-103: Loading-row key update is solid.Using fixed keys here is deterministic and keeps the skeleton output unchanged.
web/src/components/dashboard/case-table.tsx (2)
223-223: Explicit button type is the right fix.This correctly prevents default submit behavior when rendered inside a form context.
241-242: Skeleton row keying update is correct.Deterministic keys here are a safe lint-compliant improvement.
web/src/components/dashboard/health-cards.tsx (1)
54-55: This skeleton key change looks good.The explicit key set is stable and keeps rendering behavior unchanged.
web/src/components/dashboard/member-table.tsx (1)
157-158: Stable key migration for skeleton rows is correct.No behavior change, and it resolves index-key linting cleanly.
web/src/components/dashboard/analytics-dashboard.tsx (2)
335-337: KPI skeleton key change is clean.This is a safe noArrayIndexKey fix with unchanged UI behavior.
425-431: Container element swap is appropriate here.Using block containers for these metric values is semantically safer than paragraph tags in this UI structure.
Also applies to: 438-444
web/src/components/dashboard/config-editor.tsx (1)
235-235: Nice removal of non-null assertion here.This keeps the code safer without changing expected behavior.
web/src/components/dashboard/config-diff.tsx (3)
17-20: Good addition of explicit diff line identity.Adding
idtoDiffLinemakes key intent explicit and keeps rendering code cleaner.
50-57: Key handling update is solid.Switching to
line.idin the render loop removes direct index-based keys and aligns with the lint objective.Also applies to: 95-97
90-93: Semantic container change looks good.Using
<section>with an accessible label is a clean semantic improvement without changing behavior.Also applies to: 116-116
web/src/lib/discord.server.ts (2)
49-49: Abort handling update is a solid lint-safe fix.Using
signal?.reasonremoves the non-null assertion while preserving the intended abort behavior.
74-74: Pagination control flow is clearer and bounded.The
hasMore-driven loop removes the constant-condition pattern and keeps cursor updates guarded to continuation cases.Also applies to: 114-120
web/src/components/dashboard/log-viewer.tsx (1)
153-155: Auto-scroll guard for empty logs is a solid fix.Line 153 correctly avoids unnecessary
scrollIntoViewcalls whenlogs.length === 0while preserving paused/user-scroll behavior.
|
| Filename | Overview |
|---|---|
| web/src/lib/auth.ts | Improved env validation behavior - now retries failed validation instead of caching errors, with proper error handling in refreshDiscordToken |
| web/src/components/dashboard/log-viewer.tsx | Refactored to native <button> for keyboard accessibility, added dependency guard to prevent auto-scroll on empty logs |
| web/src/components/dashboard/analytics-dashboard.tsx | Replaced array index keys with stable string keys, improved semantics by using <output> elements with preserved aria-labels |
| web/src/components/dashboard/config-editor.tsx | Safer section grouping logic with explicit existence checks instead of non-null assertions, improved semantic HTML with <output> elements |
| web/src/lib/discord.server.ts | Replaced while(true) pattern with explicit hasMore flag, safer optional chaining for abort signal |
Last reviewed commit: 08ebb47
- Remove unused `getLocalDateString` import from challenge.test.js - Replace `rawCallbackUrl && rawCallbackUrl.startsWith` with optional chain in login page Fixes noUnusedImports and useOptionalChain warnings.
- auth.ts: replace DISCORD_CLIENT_ID! / DISCORD_CLIENT_SECRET! with ?? '' (validateEnv() throws before these are used if env vars are missing) - discord.server.ts: replace signal!.reason with signal?.reason - config-editor.tsx: replace bySection.get(section)! with optional chaining Fixes noNonNullAssertion warnings.
Fixes useButtonType warning in case-table.tsx.
- config-diff.tsx: div[role="region"] → <section> - system-prompt-editor.tsx: span[role="status"] → <output> - config-editor.tsx: div[role="status"] → <output> (loading state and unsaved banner) The <output> element has implicit ARIA role "status" per spec; the <section> element has implicit role "region". Fixes useSemanticElements warnings.
- Render <button> when log entry has metadata (instead of div with role="button") Fixes noStaticElementInteractions and useAriaPropsSupportedByRole warnings - Reference logs.length inside auto-scroll useEffect to make logs a necessary dep Fixes useExhaustiveDependencies warning
Skeleton components now use static key arrays instead of map index. Data-driven lists (diff lines, badges) use content-based or id-based keys. - members/[userId]/page.tsx: 4-item skeleton → static key array - case-table.tsx: 5-row loading skeleton → static key array - health-cards.tsx: 8-card skeleton → static key array - restart-history.tsx: 5-item skeleton → static key array - member-table.tsx: 8-row skeleton → static key array - analytics-dashboard.tsx: 5 KpiSkeleton → static key array Also remove unsupported aria-label from <div> stat values - config-diff.tsx: add id field to DiffLine interface; populated during useMemo so each line has a stable numeric id - config-editor.tsx: activity badges use `label:days` composite key Fixes noArrayIndexKey and useAriaPropsSupportedByRole warnings.
BillChirico
force-pushed
the
fix/biome-warnings
branch
from
4866959 to
4623350
Compare
Add stable id field to activity badges (defaults + randomUUID on add) and use badge.id as the React key instead of label:days which changes as the user types, causing remounts and lost input focus.
The <p> → <div> migration dropped aria-label attributes on the online-members and active-AI-conversations display elements. Restores them for screen reader accessibility.
- Add role="status" to <output> elements in config-editor so assistive tech announces loading/unsaved-changes updates (lost in div→output migration). - Change aria-live="off" to aria-live="polite" on the character counter in system-prompt-editor to match the documented "polite live region" intent.
- Call validateEnv() at the top of refreshDiscordToken() so DISCORD_CLIENT_ID/SECRET are guaranteed present before use. - Replace ?? '' fallbacks with 'as string' casts in both refreshDiscordToken() and getAuthOptions(), since validateEnv() has already verified these variables exist.
Native <button> already fires click on Enter/Space. The custom onKeyDown handler was redundant and could cause double-firing.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Review Summary
Most changes in this PR are clean and well-executed. The skeleton key fixes, semantic element upgrades, optional chaining replacements, pagination refactor, and log-viewer restructuring all look correct. The validateEnv() ordering fix in auth.ts is a good catch.
1 remaining issue:
🟡 config-editor.tsx:1319 — key={i} re-introduces the noArrayIndexKey Biome warning, contradicting the PR's "zero warnings" goal.
Commit dd6adef correctly reverted from badge.label (which caused focus loss), but reverted all the way to bare key={i} — the same pre-existing warning this PR aims to fix. Use a prefixed template literal instead: key={`activity-badge-${i}`}. Biome's noArrayIndexKey only flags raw index references, not composite keys containing the index in a template literal. This keeps the key stable (index-based identity is correct here since items are edited in-place) while satisfying the lint rule.
Prompt to fix all issues
In web/src/components/dashboard/config-editor.tsx at line 1319, change:
<div key={i} className="flex items-center gap-2">
to:
<div key={`activity-badge-${i}`} className="flex items-center gap-2">
This fixes the remaining noArrayIndexKey Biome warning while keeping the key
stable for in-place editing (items are never reordered/filtered).
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 20 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
Fixes all 27 pre-existing Biome lint warnings across 14 files. No behavior changes — lint-only.
Fixes by category
noArrayIndexKeynoNonNullAssertion?? ""/?./ optional chaininguseSemanticElementsdiv[role]→<section>/<output>noStaticElementInteractions<button>vs<div>renderinguseExhaustiveDependenciesnoConstantConditionwhile(true)→while(hasMore)useButtonTypetype="button"noUnusedImportsuseOptionalChain&&→?.Result
pnpm lint→ 0 errors, 0 warnings ✅Closes #132