fix(react-hooks): rules-of-hooks 8 violations (Onda 2 PR 2.1)#101
Merged
Conversation
Resolve TODAS as 8 ocorrências de react-hooks/rules-of-hooks no repo —
bugs reais que poderiam causar 'Rendered fewer hooks than expected'.
Reduz errors de 182 → 174.
## Os 8 fixes em 5 arquivos
### 1) e2e/fixtures/auth.ts:31 — falso positivo (Playwright fixture)
`await use(page)` é o callback da fixture do Playwright, NÃO o React `use()`.
Override em eslint.config.js: `react-hooks/rules-of-hooks: off` no escopo
existente de `scripts/**`, `e2e/**`, `tests/e2e/**` (esses arquivos não são
React, a regra é falso positivo).
### 2) src/components/contacts/ContactQuickView.tsx:38 — useMemo após early return
Antes:
```tsx
if (!contact) return null;
const typeCfg = ...;
const initials = ...;
const health = useMemo(() => contact ? calculateContactHealth(contact) : 0, [contact]);
```
Depois:
```tsx
// Hooks ANTES do early return (Rules of Hooks)
const health = useMemo(() => contact ? calculateContactHealth(contact) : 0, [contact]);
if (!contact) return null;
const typeCfg = ...;
const initials = ...;
```
### 3) src/components/ui/mobile-components.tsx:229 — useTransform dentro de JSX condicional
Antes:
```tsx
{isRefreshing ? <X/> : (
<motion.div style={{ rotate: useTransform(pullProgress, [0, 1], [0, 180]) }} />
)}
```
Depois (extraído pra const acima do return):
```tsx
const pullRotate = useTransform(pullProgress, [0, 1], [0, 180]);
// ...
{isRefreshing ? <X/> : (
<motion.div style={{ rotate: pullRotate }} />
)}
```
### 4) src/features/inbox/components/SLAIndicatorForContact.tsx (3 hooks após early return)
Refactor: extrair sub-componente `SLAIndicatorForContactInner` que assume
contact garantido (early return ficou só no wrapper externo).
```tsx
export function SLAIndicatorForContact({ conversation, compact, className }) {
const contact = conversation.contact;
if (!contact) return null;
return <SLAIndicatorForContactInner conversation={conversation} compact={compact} className={className} />;
}
function SLAIndicatorForContactInner({ conversation, compact, className }) {
const contact = conversation.contact; // garantido pelo wrapper acima
const { data: applicable, isLoading } = useApplicableSLA({...});
const lastSlaRef = useRef<string | null>(null);
// ... resto da lógica original
useEffect(...);
return (...);
}
```
### 5) src/features/inbox/components/contact-details/SLATimelineSection.tsx (2 hooks após early returns)
Mesmo padrão: extrair `SLATimelineSectionInner` que recebe `timeline` (NonNullable)
e `sla` validados via props.
Estrutura final:
- **Wrapper (`SLATimelineSection`):** 6 hooks (useState×3, useEffect, useMemo×2,
useConversationSLATimeline, useApplicableSLA) + early returns (skeleton/empty state)
- **Inner (`SLATimelineSectionInner`):** useMemo (handleOpenConversation) + useSLAAlerts
Tipos derivados via `NonNullable<ReturnType<typeof useConversationSLATimeline>['data']>`
para garantir type-safety nos props do Inner.
## Por que era CRÍTICO
`react-hooks/rules-of-hooks` é um dos poucos errors que costumavam ser
mantidos como 'error' — não warn — porque é um bug real:
> React Hooks must be called in the exact same order in every component render.
Se um hook é chamado APÓS um early return, e a condição muda entre renders
(de truthy pra falsy ou vice-versa), o React monta o estado interno errado
e o app crasha com 'Rendered fewer hooks than expected'.
Esses 8 errors eram bugs de produção esperando acontecer.
## Status pós-PR
| Métrica | Antes | Depois |
|---|---|---|
| Total problems | 2251 | 2243 |
| Errors | 182 | 174 |
| react-hooks/rules-of-hooks | 8 | 0 |
## Stress-test pré-commit
- bun run lint: 0 ocorrências de `rules-of-hooks` ✓
- bun run build: OK (built in 1m 2s) ✓
- Refactor preserva comportamento: mesmos hooks, mesmos efeitos, mesma UI
## Refs
- /workspace/notes/faxina-onda-2-plano.md (plano da Onda 2)
- docs/auditorias/2026-05-08-1548-auditoria-profunda.md (Achado A1)
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughO PR garante conformidade com React Rules of Hooks: ajusta ESLint para globs de scripts/E2E, move/centraliza hooks ( ChangesReact Hooks Rules Compliance
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutos 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
Corrige violações de react-hooks/rules-of-hooks que poderiam causar inconsistência na ordem de hooks (ex.: Rendered fewer hooks than expected), ajustando componentes com early returns e casos de “hook-like calls” fora de React (Playwright).
Changes:
- Refatoração com padrão Wrapper/Inner em componentes que faziam early return antes de hooks (
SLAIndicatorForContact,SLATimelineSection). - Reordenação de hook para rodar antes de early return (
ContactQuickView) e extração deuseTransformpara fora do JSX (mobile-components). - Ajuste do ESLint para desabilitar
react-hooks/rules-of-hooksemscripts/ee2e/(onde há chamadas comotest.use()que não são hooks React).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/features/inbox/components/SLAIndicatorForContact.tsx | Split Wrapper/Inner para garantir que hooks não fiquem após early return. |
| src/features/inbox/components/contact-details/SLATimelineSection.tsx | Split Wrapper/Inner para remover hooks após early returns e manter tipagem derivada dos hooks. |
| src/components/ui/mobile-components.tsx | Move useTransform para constante antes do JSX, evitando chamada “inline” em render. |
| src/components/contacts/ContactQuickView.tsx | Move useMemo para antes do early return quando contact é nulo. |
| eslint.config.js | Desliga react-hooks/rules-of-hooks para scripts/e2e (Playwright), mantendo regras no app. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Merged
2 tasks
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/ui/mobile-components.tsx (1)
196-203:⚠️ Potential issue | 🟠 Major | ⚡ Quick winTrate erro de
onRefreshpara não travar estado de loading/pull.Em Line 199, se
onRefresh()rejeitar, o componente pode ficar comisRefreshing/isPullinginconsistente e gerar rejeição não tratada.Patch sugerido
const handleDragEnd = async () => { if (pullY.get() > 80) { - setIsRefreshing(true); - await onRefresh(); - setIsRefreshing(false); + setIsRefreshing(true); + try { + await onRefresh(); + } catch (error) { + // opcional: enviar para logger/telemetria + } finally { + setIsRefreshing(false); + } } setIsPulling(false); };As per coding guidelines, "Promises sem await ou .catch()".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/ui/mobile-components.tsx` around lines 196 - 203, handleDragEnd currently awaits onRefresh() without handling rejection which can leave isRefreshing/isPulling stuck; wrap the onRefresh call in try/catch/finally (or use .catch/.finally) so that setIsRefreshing(false) and setIsPulling(false) always run even if onRefresh throws, and log or swallow the error as appropriate; update the logic inside handleDragEnd to check pullY.get() > 80, setIsRefreshing(true), then call onRefresh() with error handling, and ensure setIsRefreshing(false) and setIsPulling(false) are executed in the finally block (references: handleDragEnd, onRefresh, setIsRefreshing, setIsPulling, pullY.get()).src/features/inbox/components/contact-details/SLATimelineSection.tsx (1)
257-265:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMantenha o fallback para
queue_id/assigned_tona busca da SLA.Aqui o escopo passa a depender só de
conversation.queue?.ideconversation.assignedTo?.id. Se a conversa vier apenas com os IDs legados — shape que oSLAIndicatorForContactainda cobre — o hook resolve uma regra menos específica e o timeline pode mostrar limites/alertas errados sem falhar visivelmente.🔧 Ajuste sugerido
- const slaQueueId = scope === 'current' || scope === 'queue' ? (conversation.queue?.id ?? null) : null; - const slaAgentId = scope === 'current' || scope === 'agent' ? (conversation.assignedTo?.id ?? null) : null; + const slaQueueId = + scope === 'current' || scope === 'queue' + ? (conversation.queue?.id ?? conversation.queue_id ?? null) + : null; + const slaAgentId = + scope === 'current' || scope === 'agent' + ? (conversation.assignedTo?.id ?? conversation.assigned_to ?? null) + : null;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/inbox/components/contact-details/SLATimelineSection.tsx` around lines 257 - 265, O código calcula slaQueueId e slaAgentId apenas a partir de conversation.queue?.id e conversation.assignedTo?.id, perdendo os IDs legados e fazendo com que useApplicableSLA resolva regras menos específicas; ajuste slaQueueId para usar conversation.queue?.id ?? conversation.queue_id ?? null e slaAgentId para usar conversation.assignedTo?.id ?? conversation.assigned_to ?? null antes de passar para useApplicableSLA (referências: slaQueueId, slaAgentId, useApplicableSLA, SLAIndicatorForContact).
🧹 Nitpick comments (1)
src/features/inbox/components/SLAIndicatorForContact.tsx (1)
104-109: ⚡ Quick winCria tipo dedicado para
Innergarantindocontactnão-nulo.O wrapper faz o check em runtime (
if (!contact) return null), mas reutilizarSLAIndicatorForContactProps(comconversation: any) no Inner esconde essa garantia do TypeScript. Resultado:contact.id,contact.company,contact.job_title,contact.contact_type(linhas 112-115) seguem sem narrowing estático — uma alteração nesse shape só aparece em runtime.Adicione
SLAIndicatorForContactInnerPropscom type refinado:+type SLAIndicatorForContactInnerProps = { + conversation: Conversation & { contact: NonNullable<Conversation['contact']> }; + compact?: boolean; + className?: string; +}; + function SLAIndicatorForContact({ conversation, compact, className }: SLAIndicatorForContactProps) { const contact = conversation.contact; if (!contact) return null; return <SLAIndicatorForContactInner conversation={conversation} compact={compact} className={className} />; } -function SLAIndicatorForContactInner({ conversation, compact, className }: SLAIndicatorForContactProps) { +function SLAIndicatorForContactInner({ conversation, compact, className }: SLAIndicatorForContactInnerProps) { const contact = conversation.contact;Guideline:
any/unknown sem narrowing posterior.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/inbox/components/SLAIndicatorForContact.tsx` around lines 104 - 109, SLAIndicatorForContactInner uses SLAIndicatorForContactProps which allows conversation.contact to be nullable and hides the runtime check; create a new type SLAIndicatorForContactInnerProps where conversation: { contact: { id: string; company: string; job_title?: string; contact_type: string; /* match actual shape */ } /* plus any other used fields*/ } (or refine contact to non-nullable) and update the SLAIndicatorForContactInner signature to accept SLAIndicatorForContactInnerProps so TypeScript knows contact is non-null and its properties (contact.id, contact.company, contact.job_title, contact.contact_type) are statically narrowed; keep the runtime wrapper check as-is but rely on the new type for static safety.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/components/ui/mobile-components.tsx`:
- Around line 196-203: handleDragEnd currently awaits onRefresh() without
handling rejection which can leave isRefreshing/isPulling stuck; wrap the
onRefresh call in try/catch/finally (or use .catch/.finally) so that
setIsRefreshing(false) and setIsPulling(false) always run even if onRefresh
throws, and log or swallow the error as appropriate; update the logic inside
handleDragEnd to check pullY.get() > 80, setIsRefreshing(true), then call
onRefresh() with error handling, and ensure setIsRefreshing(false) and
setIsPulling(false) are executed in the finally block (references:
handleDragEnd, onRefresh, setIsRefreshing, setIsPulling, pullY.get()).
In `@src/features/inbox/components/contact-details/SLATimelineSection.tsx`:
- Around line 257-265: O código calcula slaQueueId e slaAgentId apenas a partir
de conversation.queue?.id e conversation.assignedTo?.id, perdendo os IDs legados
e fazendo com que useApplicableSLA resolva regras menos específicas; ajuste
slaQueueId para usar conversation.queue?.id ?? conversation.queue_id ?? null e
slaAgentId para usar conversation.assignedTo?.id ?? conversation.assigned_to ??
null antes de passar para useApplicableSLA (referências: slaQueueId, slaAgentId,
useApplicableSLA, SLAIndicatorForContact).
---
Nitpick comments:
In `@src/features/inbox/components/SLAIndicatorForContact.tsx`:
- Around line 104-109: SLAIndicatorForContactInner uses
SLAIndicatorForContactProps which allows conversation.contact to be nullable and
hides the runtime check; create a new type SLAIndicatorForContactInnerProps
where conversation: { contact: { id: string; company: string; job_title?:
string; contact_type: string; /* match actual shape */ } /* plus any other used
fields*/ } (or refine contact to non-nullable) and update the
SLAIndicatorForContactInner signature to accept SLAIndicatorForContactInnerProps
so TypeScript knows contact is non-null and its properties (contact.id,
contact.company, contact.job_title, contact.contact_type) are statically
narrowed; keep the runtime wrapper check as-is but rely on the new type for
static safety.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 81015a02-1fb0-4af1-8200-0696f53ff0eb
📒 Files selected for processing (5)
eslint.config.jssrc/components/contacts/ContactQuickView.tsxsrc/components/ui/mobile-components.tsxsrc/features/inbox/components/SLAIndicatorForContact.tsxsrc/features/inbox/components/contact-details/SLATimelineSection.tsx
1 task
CodeRabbit identificou 2 issues legítimos ("outside diff range") em arquivos
tocados pela PR. Aplicando ambos os fixes pequenos:
## Issue 1 — mobile-components.tsx handleDragEnd
`onRefresh()` era awaited sem try/catch — se rejeitar, `isRefreshing` ficaria
travado em true e o componente preso em loading state.
```diff
const handleDragEnd = async () => {
if (pullY.get() > 80) {
setIsRefreshing(true);
- await onRefresh();
- setIsRefreshing(false);
+ try {
+ await onRefresh();
+ } catch {
+ // Erro em onRefresh — log opcional via telemetria
+ } finally {
+ setIsRefreshing(false);
+ }
}
setIsPulling(false);
};
```
## Issue 2 — SLATimelineSection slaQueueId/slaAgentId fallback
Bug pré-existente: quando `conversation` vinha apenas com IDs legados
(`queue_id`/`assigned_to`) sem o shape estruturado (`queue.id`/`assignedTo.id`),
o hook resolvia uma regra de SLA menos específica e timeline mostrava limites
errados sem falhar.
O `SLAIndicatorForContact.tsx` JÁ tinha esse fallback. Agora SLATimelineSection
está consistente:
```diff
- const slaQueueId = scope === 'current' || scope === 'queue' ? (conversation.queue?.id ?? null) : null;
- const slaAgentId = scope === 'current' || scope === 'agent' ? (conversation.assignedTo?.id ?? null) : null;
+ const slaQueueId =
+ scope === 'current' || scope === 'queue'
+ ? (conversation.queue?.id ?? conversation.queue_id ?? null)
+ : null;
+ const slaAgentId =
+ scope === 'current' || scope === 'agent'
+ ? (conversation.assignedTo?.id ?? conversation.assigned_to ?? null)
+ : null;
```
## Stress-test
- ✅ bun run build (1m)
- ✅ Lint sem regressão (174 errors mantidos)
- ✅ Comportamento preservado para o shape novo, melhorado para legado
This was referenced May 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resumo
Resolve TODAS as 8 ocorrências de
react-hooks/rules-of-hooksno repo. Cada uma é um bug real que poderia causarRendered fewer hooks than expectedem produção.Reduz errors de 182 → 174 (-4%).
Os 8 fixes em 5 arquivos
e2e/fixtures/auth.tsuse(), não React)ContactQuickView.tsxuseMemoANTES deif (!contact) return nullmobile-components.tsxuseTransformpraconstacima do JSXSLAIndicatorForContact.tsxInnerSLATimelineSection.tsxInnerPor que era CRÍTICO
Se um hook é chamado depois de um early return, e a condição muda entre renders, o React desalinha o estado interno e o app crasha. Esses 8 errors eram bugs de produção esperando acontecer.
Padrão idiomático: extrair sub-componente
Para casos onde múltiplos hooks ficam depois de early returns (
SLAIndicatorForContact,SLATimelineSection), o fix correto é extrair sub-componente (não "defensive programming" com optional chaining em todos os hooks).Stress-test
bun run lint(rules-of-hooks)bun run buildNonNullable<ReturnType<typeof ...>>Métricas Onda 2 acumuladas
Refs
Summary by CodeRabbit
Notas de Lançamento
Correções de Bugs
Melhorias