fix: hooks audit round 3 — BUG-08 a BUG-17 (10 bugs corrigidos)#436
Conversation
…nt timer cancellation Root cause: saveToDb was in the snapshot useEffect deps. Any kitState change (e.g., totalPrice recalculation) recreated saveToDb, triggering the effect. Same snapshot → early return, but React still ran the previous cleanup (clearTimeout) without setting a new timer. Auto-save silently cancelled. Fix: saveToDbRef pattern — ref updated on every render, timeout calls ref.
bulkCartProducts was an identical copy of selectedProducts (same code, same deps). Replace with a plain alias — zero extra computation on each render.
… — stop polling restart on markAsRead Root cause: notifications.length in fetchNotifications deps caused markAsRead to recreate fetchNotifications, triggering the polling useEffect to rerun (clearing + restarting the 30s interval on every notification read). Fix: notificationsLengthRef updated each render; notifications.length removed from fetchNotifications deps array.
Deprecated hook was calling setState via Promise chain with no cleanup.
If the component unmounted before the fetch resolved, setState was called
on an already-unmounted component causing React warnings.
Fix: let isMounted = true; return () => { isMounted = false } in useEffect.
Each setState guarded with if (isMounted).
…ptions
No cleanup existed: rapid techniqueCode changes (user clicking between
techniques) would cause the previous fetch's setState calls to fire on
a component that may have already been unmounted or moved to a new selection.
Fix: isMounted flag with return () => { isMounted = false } in the effect.
…Validation Both structures were recomputed on every render (plain variables outside useMemo). Any scroll, hover, or unrelated state update in a kit with many items triggered the full O(n) aggregation loop unnecessarily. Fix: single useMemo with deps [stockData, box, items, kitQuantity].
…index updates Root cause: pushState captured historyIndex via closure and used it inside setHistory functional updater. With React batching, two rapid pushState calls (e.g., during logo drag) both saw the same stale historyIndex, causing the second call to slice at the same point and discard the first push. Fix: useReducer with historyReducer updates both history and historyIndex atomically in a single dispatch, eliminating the stale closure entirely.
The 1s dedupe timer in addToRecentlyViewed was not stored, so it could not be cleared on unmount or on a subsequent call within the 1s window. On unmount the callback would still fire, accessing lastAddedRef on an already-unmounted component. Fix: dedupeTimerRef stores the id; useEffect cleanup clears it on unmount; previous timer cleared before each new call.
undo() and redo() called setTimeout without storing the id. On unmount the 100ms timer would still fire; on rapid undo calls multiple timers could stack and race. reset() also didn't cancel an in-flight timer. Fix: restoreTimerRef stores the id; previous timer cleared before each new one; useEffect cleanup clears on unmount; reset() clears the timer too.
…king
Root cause: fetch('https://ipapi.co/json/') had no cancellation mechanism.
If an admin navigated away before the response arrived (~200-500ms), the
.then chain would call setCurrentCountry on an unmounted component.
Fix: fetchCurrentCountry accepts an optional AbortSignal; useEffect creates
an AbortController, passes the signal, and calls controller.abort() on cleanup.
AbortError is explicitly silenced as it is the expected unmount behavior.
Full audit report for 378 hook files across 21 directories. Documents BUG-08 through BUG-17 with root causes, fixes, and commit SHAs.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
More reviews will be available in 28 minutes and 42 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (11)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d6abf0caf8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| } finally { | ||
| setIsLoading(false); | ||
| if (isMounted) setIsLoading(false); |
There was a problem hiding this comment.
Guarding setIsLoading(false) behind isMounted can leave isLoading stuck as true when techniqueCode changes to a falsy value while a previous fetch is still in flight: cleanup sets isMounted = false, the old request's finally skips resetting loading, and the early-return branch for empty techniqueCode does not clear loading. In CustomizationOptions, this can keep the "Carregando opções..." state visible indefinitely for that instance.
Useful? React with 👍 / 👎.
|
|
||
| const reset = useCallback(() => { | ||
| // Clear any pending restore timer when resetting | ||
| if (restoreTimerRef.current) clearTimeout(restoreTimerRef.current); |
There was a problem hiding this comment.
reset() now cancels the pending restore timer but never forces isRestoringRef.current back to false. If a caller invokes reset() within the 100ms window after undo()/redo(), the timer that would clear the flag is removed and pushSnapshot will keep returning early, effectively disabling future history capture for that hook instance.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR applies a set of “hooks audit” fixes focused on preventing stale-closure bugs, avoiding state updates after unmount, and reducing unnecessary re-renders/computation in several React hooks.
Changes:
- Stabilize effects/callbacks by removing volatile deps (using refs) and making multi-state updates atomic (useReducer).
- Add unmount-safe cleanup for async work (isMounted flags, AbortController, timer refs + cleanup effects).
- Reduce repeated derived computations by consolidating and memoizing derived data.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/hooks/ui/useWorkspaceNotifications.tsx | Prevents polling interval resets by removing notifications.length from callback deps via a ref. |
| src/hooks/simulation/useTechniquePricing.ts | Prevents setState after unmount / stale async resolution via an isMounted guard. |
| src/hooks/simulation/usePositionHistory.ts | Fixes stale-closure history corruption by switching to atomic useReducer updates. |
| src/hooks/simulation/useGravacaoPriceV2.ts | Adds unmount-safe guards to legacy reactive pricing hook; formatting/doc cleanup. |
| src/hooks/products/useRecentlyViewed.ts | Prevents timeout leak by storing/clearing dedupe timer across calls and on unmount. |
| src/hooks/kit-builder/useKitUndoRedo.ts | Prevents stacked/uncleared restore timers by storing/clearing timeout id and cleaning up on unmount. |
| src/hooks/kit-builder/useKitStockValidation.ts | Avoids O(n) recomputation each render by deriving Map/alerts inside useMemo. |
| src/hooks/kit-builder/useKitAutoSave.ts | Prevents autosave timer cancellation by calling latest saveToDb via ref (not effect deps). |
| src/hooks/common/useEntitySelectionMode.ts | Removes duplicate memoized computation by aliasing instead of recomputing. |
| src/hooks/admin/useGeoBlocking.ts | Avoids setState-after-unmount by aborting geo lookup fetch on unmount. |
| docs/hooks-audit-round3-2026-05.md | Documents the audit findings/fixes (BUG-08..BUG-17) and their rationale. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // and when addToRecentlyViewed is called again before the previous 1s window | ||
| // has elapsed. Previously setTimeout was called without storing the id, | ||
| // leaving a pending callback that would fire after unmount. | ||
| const dedupeTimerRef = useRef<ReturnType<typeof setTimeout>>(); |
| // would fire and attempt to mutate a ref on a component that may have been | ||
| // destroyed. Also: if undo was called rapidly, multiple timers could stack, | ||
| // each resetting isRestoringRef independently. | ||
| const restoreTimerRef = useRef<ReturnType<typeof setTimeout>>(); |
| tableCodeOption: t.table_code_option as string | null, | ||
| tableFullcode: t.table_fullcode as string | null, | ||
| techniqueName: t.customization_type_name as string, | ||
| maxColors: (t.max_colors as number) || 1, |
| }); | ||
| } catch (error) { | ||
| // AbortError is expected on unmount — silence it | ||
| if (error instanceof Error && error.name === 'AbortError') return; |
🔍 Terceira Auditoria Exaustiva de Hooks — Maio 2026
Auditoria completa de 378 arquivos de hooks em 21 diretórios de
src/hooks/. Encontrados e corrigidos 10 novos bugs.Bugs Corrigidos
useKitAutoSave.tssaveToDbnas deps do snapshot effectuseEntitySelectionMode.tsbulkCartProductseselectedProductscom cálculo idêntico duplicadouseWorkspaceNotifications.tsxnotifications.lengthnas deps causa restart do polling a cada leiturauseGravacaoPriceV2.tsuseCustomizationPriceReactiveLegacyuseTechniquePricing.tsfetchPriceOptionsuseKitStockValidation.tsstockByProductealertsfora deuseMemo(recalculados todo render)usePositionHistory.tspushStateusahistoryIndexstale em chamadas rápidas consecutivasuseRecentlyViewed.tssetTimeoutnão limpo no unmountuseKitUndoRedo.tssetTimeoutnão limpo no unmount; delay de 100ms sem gerenciamentouseGeoBlocking.tsfetchCurrentCountrysemAbortController— setState após unmountBUG-08 —
useKitAutoSave.ts(P1 🔴)Root cause:
saveToDbera recriado a cada mudança dekitState(incluindototalPrice). Como estava nas deps do snapshotuseEffect, o effect re-executava, o cleanup anterior limpava o timer pendente, mas o effect retornava cedo (snapshot igual) sem criar novo timer. Auto-save silenciosamente cancelado.Fix:
saveToDbRef— ref atualizada a cada render viauseEffect([saveToDb]). O timeout chamasaveToDbRef.current()em vez desaveToDb.saveToDbremovido das deps do snapshot effect.BUG-09 —
useEntitySelectionMode.ts(P2)Root cause:
bulkCartProductseselectedProductseram doisuseMemocom código e deps absolutamente idênticos — computação duplicada em todo render com seleção ativa.Fix:
bulkCartProducts = selectedProducts(alias, sem nova computação).BUG-10 —
useWorkspaceNotifications.tsx(P1 🔴)Root cause:
notifications.lengthnas deps defetchNotifications.markAsRead→setNotifications→notifications.lengthmuda →fetchNotificationsrecriado → pollinguseEffect([user, fetchNotifications])reexecuta →clearInterval + setInterval→ timer de 30s resetado.Fix:
notificationsLengthRef.current = notifications.length(sincronizado a cada render).notifications.lengthremovido das deps defetchNotifications.BUG-11 —
useGravacaoPriceV2.ts(P2)Root cause:
useCustomizationPriceReactiveLegacy(deprecated, em uso legado) tinha Promise chain sem cleanup. setState chamado após unmount.Fix:
let isMounted = true+return () => { isMounted = false }. Cada setState guarded comif (isMounted).BUG-12 —
useTechniquePricing.ts(P2)Root cause:
fetchPriceOptionsdentro deuseEffectsem flag de cancelamento. Troca rápida detechniqueCodecausava setState da request anterior no componente novo.Fix: Flag
isMountedcom cleanupreturn () => { isMounted = false }.BUG-13 —
useKitStockValidation.ts(P3)Root cause:
stockByProduct(Map) ealerts(Array) construídos fora deuseMemo— recalculados em todos os renders (scroll, hover, qualquer state).Fix: Único
useMemocom deps[stockData, box, items, kitQuantity].BUG-14 —
usePositionHistory.ts(P2)Root cause:
pushStatecapturavahistoryIndexvia closure emsetHistory. Duas chamadas rápidas antes do re-render (drag de logo) usavam o mesmohistoryIndexstale — segunda chamada descartava o push da primeira.Fix: Migrado para
useReducercomhistoryReducer. Dispatch é atômico —historyehistoryIndexsempre consistentes.BUG-15 —
useRecentlyViewed.ts(P3)Root cause:
setTimeoutde 1s emaddToRecentlyViewednão armazenado. Callback continuava após unmount. Múltiplas chamadas podiam stackar timers.Fix:
dedupeTimerRef+ cleanup nouseEffect+ clear antes de cada novo setTimeout.BUG-16 —
useKitUndoRedo.ts(P3)Root cause:
undo()eredo()chamavamsetTimeoutsem armazenar o id. Timer não limpo no unmount.reset()não cancelava timer em-flight. Chamadas rápidas podiam stackar múltiplos timers.Fix:
restoreTimerRefcentralizado + clear antes de cada novo timer +useEffectcleanup +reset()limpa o timer.BUG-17 —
useGeoBlocking.ts(P2)Root cause:
fetch('https://ipapi.co/json/')semAbortController. Admin navegando rapidamente pela área admin ativavasetCurrentCountryapós unmount (~200-500ms de latência da API externa).Fix:
fetchCurrentCountry(signal?: AbortSignal).useEffectcriaAbortController, passa o signal, cleanup chamacontroller.abort().AbortErrorsilenciado.Commits
e1a71ac692836670be644b5bc1cff22c2e9ddd0cb32767b528068286869c2ab9840027f20ec1f22fd6abf0caDocumentação completa:
docs/hooks-audit-round3-2026-05.mdHistórico de Auditorias
Summary by cubic
Fixed 10 hook issues from the third audit (BUG-08 → BUG-17) to improve stability, correctness, and performance. Prevents silent auto-save cancellations, stops notification polling resets, and removes setState-after-unmount and stale-closure bugs.
useKitAutoSave; no more silent cancellation (BUG-08, P1).notifications.lengthfrom deps inuseWorkspaceNotifications; interval no longer resets on read (BUG-10, P1).useGravacaoPriceV2(legacy),useTechniquePricing, anduseGeoBlockingto stop setState after unmount (BUG-11, BUG-12, BUG-17).usePositionHistorytouseReducerfor atomic history/index updates; fixes stale-closure drops on rapid updates (BUG-14).useRecentlyViewedanduseKitUndoRedo, including on unmount and rapid calls (BUG-15, BUG-16).useEntitySelectionModeby aliasingbulkCartProductstoselectedProducts(BUG-09).useKitStockValidation(BUG-13).Written for commit d6abf0c. Summary will update on new commits. Review in cubic