Skip to content

useToast has no auto-dismiss timeout and a module-level mutable id counter shared across all instances #24

Description

@abayomicornelius

Problem

src/hooks/useToast.ts, in full:

import { useState, useCallback } from 'react'
type T = 'success'|'error'|'info'
interface Toast { id: number; message: string; type: T }
let n = 0
export function useToast() {
  const [toasts, setToasts] = useState<Toast[]>([])
  const show = useCallback((message: string, type: T = 'info') => setToasts(p => [...p, { id: ++n, message, type }]), [])
  const dismiss = useCallback((id: number) => setToasts(p => p.filter(t => t.id !== id)), [])
  return { toasts, dismiss, success: (m: string) => show(m,'success'), error: (m: string) => show(m,'error'), info: show }
}

Two distinct problems:

  1. No auto-dismiss. show pushes a toast into state and nothing ever removes it except an explicit call to dismiss(id). Every other toast/notification hook convention (and the timeout parameter present on the sibling useClipboard hook in this same hooks/ directory) auto-clears after a delay; this one requires whatever UI renders toasts to remember to call dismiss itself, on a timer it has to build from scratch. If any call site (or a future one) forgets to wire up a dismiss timer, toasts accumulate forever in state — a slow, invisible memory/DOM leak for any long-lived session that calls success/error/info repeatedly (e.g. once per completed batch/escrow/subscription action, which is exactly the kind of app this is).
  2. Module-level mutable counter let n = 0 shared across every component instance and every test file that imports this module. Since Vitest by default shares module instances within a test file (and depending on config, potentially across files in the same worker), toast IDs are not scoped to a single useToast() call — two unrelated components each calling useToast() increment the same global counter, so IDs are merely unique app-wide, not meaningfully tied to either hook instance. More concretely: in tests, n's value persists between test cases (it's never reset), so toast ID assertions become order-dependent on whatever other tests ran first in the same module graph — a classic source of flaky, load-order-sensitive test failures.

Why it matters

  • The always-growing toast array (point 1) means any screen that fires toasts frequently (batch payment success/error per attempt, escrow release/refund, subscription create/cancel — all of which call into hooks that would plausibly use a toast for their onSuccess/onError) will keep every toast ever shown in memory and, if rendered naively (toasts.map(...)), in the DOM, for the lifetime of the component tree.
  • Point 2 makes useToast-based tests inherently order-dependent — a test asserting toasts[0].id === 1 will pass in isolation and fail once run after any other test file that also called show(), without any change to the test itself. This is exactly the kind of flakiness that erodes trust in CI runs.

Reproduction

  • Render two independent components that both call useToast() and fire a toast from each; observe both draw IDs from the same incrementing n, not independent per-hook counters.
  • Call show() several times and confirm nothing removes a toast from toasts without an explicit dismiss() call — there is no timer anywhere in this file.

Suggested fix

  • Add an auto-dismiss setTimeout inside show (mirroring useClipboard's timeout pattern in the same directory) that calls dismiss(id) after a configurable delay, with dismiss/clearTimeout cleanup handled correctly if the toast is manually dismissed before the timer fires (to avoid a dangling timer trying to dismiss an already-removed id — harmless but wasteful).
  • Replace the module-level let n = 0 with a useRef-based counter (or crypto.randomUUID()/useId()) scoped to each hook instance, so IDs are stable per-instance and tests don't leak state across files.

Edge cases

  • If multiple useToast() instances are meant to share a single toast queue (e.g. a global toast host rendered once at the app root) rather than being genuinely independent, the "shared counter" bug is arguably intentional-but-accidental — worth confirming whether useToast is meant to be called once at the app root and passed down via context, vs. called ad hoc per-component, since the current implementation doesn't behave correctly under the latter usage pattern that its hook-based API invites.

Testing strategy

  • Unit test: fire show(), advance fake timers past the auto-dismiss delay, assert the toast is removed without an explicit dismiss() call.
  • Unit test: instantiate useToast() in two separate renderHook calls, fire from each, and assert their toast IDs don't collide or otherwise leak into each other (guards against regressing back to a shared module-level counter).

Metadata

Metadata

Assignees

No one assigned

    Labels

    GrantFox OSSIssue tracked in GrantFox OSSMaybe RewardedIssue may be eligible for a GrantFox rewardOfficial Campaign | FWC26Campaign: Official Campaign | FWC26bugSomething isn't workingvery hardVery difficult / senior-level bounty issue

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions