From ae0bba446e1434b1fb57535b920c7286cb75e813 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 22 Jun 2026 12:58:43 +0000 Subject: [PATCH 1/3] fix(Toast): clear auto-close timer on unmount Radix Toast v1.2.2 never clears its internal `closeTimerRef` on unmount, which leaks the auto-close `setTimeout` past the host's lifetime. In jsdom-based test runs the leftover timer fires after vitest tears down the test file's DOM, and `handleClose` -> `document.activeElement` throws `ReferenceError: document is not defined`, aborting the whole suite. (Upstream fix: radix-ui/primitives#3794, still unmerged.) Side-step it inside click-ui itself: pass `duration={Infinity}` to the underlying Radix `Toast.Root` (which short-circuits its `startTimer` so no real timeout is scheduled) and own the auto-close timer in our wrapper with a `useEffect` cleanup. Radix's public `onPause`/`onResume` callbacks are wired through so viewport-level pause-on-blur/focus behavior is preserved. Co-authored-by: Shauli Bracha --- src/components/Toast/Toast.test.tsx | 84 +++++++++++++++++++++++++++++ src/components/Toast/Toast.tsx | 68 +++++++++++++++++++++-- 2 files changed, 149 insertions(+), 3 deletions(-) create mode 100644 src/components/Toast/Toast.test.tsx diff --git a/src/components/Toast/Toast.test.tsx b/src/components/Toast/Toast.test.tsx new file mode 100644 index 000000000..ec46fdc69 --- /dev/null +++ b/src/components/Toast/Toast.test.tsx @@ -0,0 +1,84 @@ +import { act } from '@testing-library/react'; +import { renderCUI } from '@/utils/test-utils'; +import { Toast } from '@/components/Toast'; + +describe('Toast', () => { + beforeEach(() => { + vi.useFakeTimers({ + // Don't fake `setImmediate` / `queueMicrotask`, otherwise React's + // scheduler / styled-components batching can hang. + toFake: ['setTimeout', 'clearTimeout', 'Date'], + }); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + it('auto-closes after the configured duration', () => { + const onClose = vi.fn(); + renderCUI( + + ); + + expect(onClose).not.toHaveBeenCalled(); + act(() => { + vi.advanceTimersByTime(3000); + }); + expect(onClose).toHaveBeenCalledWith(false); + }); + + it('clears the auto-close timer when the toast unmounts (regression for radix-ui/primitives#3703)', () => { + const onClose = vi.fn(); + const { unmount } = renderCUI( + + ); + + act(() => { + vi.advanceTimersByTime(1000); + }); + expect(onClose).not.toHaveBeenCalled(); + + unmount(); + + // No pending timers must remain after unmount. Radix v1.2.2 leaks its + // internal `closeTimerRef` (the auto-close `setTimeout`) here because it + // never `clearTimeout`s it on unmount. The leftover timer later fires + // after vitest tears down jsdom and calls `handleClose`, which accesses + // `document.activeElement` -- producing + // `ReferenceError: document is not defined` and aborting the whole test + // file. We side-step this by passing `duration={Infinity}` to Radix and + // owning the timer in click-ui itself with a cleanup effect. + expect(vi.getTimerCount()).toBe(0); + + // And nothing left behind should be able to invoke our `onClose` later. + act(() => { + vi.advanceTimersByTime(60_000); + }); + expect(onClose).not.toHaveBeenCalled(); + }); + + it('does not auto-close when duration is Infinity', () => { + const onClose = vi.fn(); + renderCUI( + + ); + + act(() => { + vi.advanceTimersByTime(60_000); + }); + expect(onClose).not.toHaveBeenCalled(); + }); +}); diff --git a/src/components/Toast/Toast.tsx b/src/components/Toast/Toast.tsx index f732b65c6..45ca0160d 100644 --- a/src/components/Toast/Toast.tsx +++ b/src/components/Toast/Toast.tsx @@ -1,4 +1,4 @@ -import { createContext, useEffect, useState } from 'react'; +import { createContext, useCallback, useEffect, useRef, useState } from 'react'; import * as RadixUIToast from '@radix-ui/react-toast'; import { keyframes, styled } from 'styled-components'; import { toastsEventEmitter } from './toastEmitter'; @@ -7,6 +7,10 @@ import { IconButton } from '@/components/IconButton'; import { Button } from '@/components/Button'; import { ToastContextProps, ToastProps, ToastAlignment, ToastType } from './Toast.types'; +// Radix's default auto-close duration, mirrored here so our internally managed +// timer matches Radix's behaviour when no `duration` is provided. +const DEFAULT_TOAST_DURATION = 5000; + export const ToastContext = createContext({ createToast: () => null, }); @@ -124,7 +128,7 @@ export const Toast = ({ title, description, actions = [], - duration, + duration = DEFAULT_TOAST_DURATION, onClose, ...props @@ -137,10 +141,68 @@ export const Toast = ({ } else if (type && ['danger', 'warning'].includes(type)) { iconName = 'warning'; } + + // We manage the auto-close timer ourselves (and pass `duration={Infinity}` to + // Radix below) so that the timer is always cleared on unmount. The Radix + // implementation (v1.2.2) never clears its internal `closeTimerRef`, which + // causes the timer to fire after the host (e.g. jsdom in tests) has been + // torn down, surfacing as `ReferenceError: document is not defined`. See + // https://github.com/radix-ui/primitives/pull/3794. Once that upstream fix + // ships and we adopt it we can revert this in favour of Radix's own timer. + const closeTimerRef = useRef | undefined>(undefined); + const closeTimerStartTimeRef = useRef(0); + const closeTimerRemainingTimeRef = useRef(duration); + + const clearCloseTimer = useCallback(() => { + if (closeTimerRef.current !== undefined) { + clearTimeout(closeTimerRef.current); + closeTimerRef.current = undefined; + } + }, []); + + const startCloseTimer = useCallback( + (remaining: number) => { + clearCloseTimer(); + if (!remaining || remaining === Infinity) { + return; + } + closeTimerStartTimeRef.current = Date.now(); + closeTimerRef.current = setTimeout(() => { + closeTimerRef.current = undefined; + onClose(false); + }, remaining); + }, + [clearCloseTimer, onClose] + ); + + useEffect(() => { + closeTimerRemainingTimeRef.current = duration; + startCloseTimer(duration); + return clearCloseTimer; + }, [duration, startCloseTimer, clearCloseTimer]); + + const handlePause = useCallback(() => { + if (closeTimerRef.current === undefined) { + return; + } + const elapsed = Date.now() - closeTimerStartTimeRef.current; + closeTimerRemainingTimeRef.current = Math.max( + 0, + closeTimerRemainingTimeRef.current - elapsed + ); + clearCloseTimer(); + }, [clearCloseTimer]); + + const handleResume = useCallback(() => { + startCloseTimer(closeTimerRemainingTimeRef.current); + }, [startCloseTimer]); + return ( From 177d1094ec7c51f528b861ff99444402ba5377de Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 22 Jun 2026 13:39:58 +0000 Subject: [PATCH 2/3] fix(Toast): keep auto-close timer stable across parent re-renders The previous commit put `onClose` in the dep array of `startCloseTimer` (and therefore of the auto-close `useEffect`). `ToastProvider` produces a new `onClose={onClose(id)}` closure on every render, so any provider update (e.g. another toast appearing) would re-run the effect, reset `closeTimerRemainingTimeRef` to the full duration, and discard pause progress -- effectively letting a frequently-rerendering parent keep a toast alive indefinitely. Mirror Radix's own `useCallbackRef` pattern: keep a ref to the latest `onClose` (updated in an effect), have `startCloseTimer` invoke `onCloseRef.current` instead of capturing `onClose` directly, and drop `onClose` from its deps. The effect now only re-runs when `duration` actually changes. Added a regression test that re-renders the toast with a fresh `onClose` identity mid-countdown and asserts the original timer still fires at the original deadline (test fails before this commit, passes after). Co-authored-by: Shauli Bracha --- src/components/Toast/Toast.test.tsx | 43 +++++++++++++++++++++++++++++ src/components/Toast/Toast.tsx | 23 +++++++++++---- 2 files changed, 61 insertions(+), 5 deletions(-) diff --git a/src/components/Toast/Toast.test.tsx b/src/components/Toast/Toast.test.tsx index ec46fdc69..ed6345769 100644 --- a/src/components/Toast/Toast.test.tsx +++ b/src/components/Toast/Toast.test.tsx @@ -66,6 +66,49 @@ describe('Toast', () => { expect(onClose).not.toHaveBeenCalled(); }); + it('does not reset the auto-close timer when the parent re-renders with a new onClose identity', () => { + // Regression for: in ToastProvider, `onClose={onClose(id)}` produces a + // fresh function on every provider render. Without a stable callback ref + // inside Toast, every provider update (e.g. another toast appearing) + // would restart the countdown from the full duration, so a toast could + // be kept alive indefinitely by a frequently-rerendering parent. + const onClose1 = vi.fn(); + const onClose2 = vi.fn(); + const { rerender } = renderCUI( + + ); + + act(() => { + vi.advanceTimersByTime(600); + }); + expect(onClose1).not.toHaveBeenCalled(); + + rerender( + + ); + + // Total elapsed since mount = 1100ms > original duration of 1000ms. + // If the timer was reset on re-render, no onClose would have fired yet + // (only 500ms of a fresh 1000ms countdown would have elapsed). + act(() => { + vi.advanceTimersByTime(500); + }); + // The latest onClose must have been invoked exactly once. + expect(onClose2).toHaveBeenCalledTimes(1); + expect(onClose2).toHaveBeenCalledWith(false); + // And the original onClose -- which is no longer the prop -- must not + // have been invoked. + expect(onClose1).not.toHaveBeenCalled(); + }); + it('does not auto-close when duration is Infinity', () => { const onClose = vi.fn(); renderCUI( diff --git a/src/components/Toast/Toast.tsx b/src/components/Toast/Toast.tsx index 45ca0160d..1721f7efc 100644 --- a/src/components/Toast/Toast.tsx +++ b/src/components/Toast/Toast.tsx @@ -144,15 +144,28 @@ export const Toast = ({ // We manage the auto-close timer ourselves (and pass `duration={Infinity}` to // Radix below) so that the timer is always cleared on unmount. The Radix - // implementation (v1.2.2) never clears its internal `closeTimerRef`, which - // causes the timer to fire after the host (e.g. jsdom in tests) has been - // torn down, surfacing as `ReferenceError: document is not defined`. See + // implementation (v1.2.2, still the case on 1.2.17) never clears its + // internal `closeTimerRef`, which causes the timer to fire after the host + // (e.g. jsdom in tests) has been torn down, surfacing as + // `ReferenceError: document is not defined`. See // https://github.com/radix-ui/primitives/pull/3794. Once that upstream fix // ships and we adopt it we can revert this in favour of Radix's own timer. const closeTimerRef = useRef | undefined>(undefined); const closeTimerStartTimeRef = useRef(0); const closeTimerRemainingTimeRef = useRef(duration); + // Stable ref to the latest `onClose` so that `startCloseTimer` does not + // depend on `onClose`'s identity. Without this, the auto-close `useEffect` + // below would re-run (and the countdown would restart from the full + // duration) every time the parent passes a fresh function -- which + // `ToastProvider` does on every render because it uses `onClose={onClose(id)}` + // (a new closure per render). That would let a frequently-rerendering parent + // keep a toast alive indefinitely and would also discard pause progress. + const onCloseRef = useRef(onClose); + useEffect(() => { + onCloseRef.current = onClose; + }); + const clearCloseTimer = useCallback(() => { if (closeTimerRef.current !== undefined) { clearTimeout(closeTimerRef.current); @@ -169,10 +182,10 @@ export const Toast = ({ closeTimerStartTimeRef.current = Date.now(); closeTimerRef.current = setTimeout(() => { closeTimerRef.current = undefined; - onClose(false); + onCloseRef.current(false); }, remaining); }, - [clearCloseTimer, onClose] + [clearCloseTimer] ); useEffect(() => { From 3022a2c4feb6e84d9762e66a50073e58835f01dc Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 22 Jun 2026 14:02:10 +0000 Subject: [PATCH 3/3] fix(Toast): address PR review nits (provider duration, resume-after-deadline, baseline-relative leak test) Addresses three issues raised in PR review: 1) Provider-level duration (Copilot, Toast.tsx:132) Previously, an unset `ToastProps.duration` let Radix's `ToastProvider duration` (the default for every toast) take effect. After moving the timer into our wrapper, defaulting `duration` to `5000` at the `Toast` component boundary silently overrode any consumer-supplied `` / `config.toast` value. Add an internal `ToastDurationContext` populated by `ToastProvider` (still defaulting to 5000), and resolve in `Toast` as `toast.duration ?? providerDuration ?? DEFAULT_TOAST_DURATION` -- restoring the original prop > provider > default resolution order. 2) Resume-after-deadline close (Copilot, Toast.tsx:211) If `handlePause` ran after the auto-close deadline had already passed (event-loop delay / timer throttling beat the close timer to the punch), remaining time would clamp to 0 and `handleResume -> startCloseTimer(0)` short-circuited, leaving the toast open indefinitely. Track `wasPausedRef` so resume can distinguish "never paused" from "paused with deadline already elapsed" and call `onClose(false)` immediately in the latter case. 3) Baseline-relative leak detection (Copilot, Toast.test.tsx:67) Replaced the brittle `getTimerCount() === 0` assertion with a semantic check: stub `Document.prototype.activeElement` and advance time past the leaked deadline. If Radix's auto-close handler ever fires post-unmount it dereferences `document.activeElement`, which now flips a boolean we assert against. Insensitive to unrelated Radix/styled-components internal timer counts. Added two new regression tests covering #1 and #2; both fail on `main` without the corresponding fix and pass with it. Co-authored-by: Shauli Bracha --- src/components/Toast/Toast.test.tsx | 153 +++++++++++++++++++++++++--- src/components/Toast/Toast.tsx | 69 ++++++++++--- 2 files changed, 195 insertions(+), 27 deletions(-) diff --git a/src/components/Toast/Toast.test.tsx b/src/components/Toast/Toast.test.tsx index ed6345769..b606982b6 100644 --- a/src/components/Toast/Toast.test.tsx +++ b/src/components/Toast/Toast.test.tsx @@ -1,6 +1,28 @@ -import { act } from '@testing-library/react'; +import { useEffect } from 'react'; +import { act, render } from '@testing-library/react'; +import { useToast } from '@/hooks/useToast'; +import { ClickUIProvider } from '@/providers'; import { renderCUI } from '@/utils/test-utils'; import { Toast } from '@/components/Toast'; +import type { ToastProviderProps } from '@/components/Toast'; + +// `ClickUIProvider` already nests `ToastProvider` and forwards `config.toast`, +// so we render directly into that to exercise the full +// `ToastProvider` -> `Toast` integration path (including provider-level +// defaults like `duration`). +const renderInToastProvider = ( + ui: React.ReactNode, + providerProps: Omit = {} +) => + render( + + {ui} + + ); describe('Toast', () => { beforeEach(() => { @@ -32,7 +54,7 @@ describe('Toast', () => { expect(onClose).toHaveBeenCalledWith(false); }); - it('clears the auto-close timer when the toast unmounts (regression for radix-ui/primitives#3703)', () => { + it('does not leak the auto-close timer past unmount (regression for radix-ui/primitives#3703)', () => { const onClose = vi.fn(); const { unmount } = renderCUI( { unmount(); - // No pending timers must remain after unmount. Radix v1.2.2 leaks its - // internal `closeTimerRef` (the auto-close `setTimeout`) here because it - // never `clearTimeout`s it on unmount. The leftover timer later fires - // after vitest tears down jsdom and calls `handleClose`, which accesses - // `document.activeElement` -- producing - // `ReferenceError: document is not defined` and aborting the whole test - // file. We side-step this by passing `duration={Infinity}` to Radix and - // owning the timer in click-ui itself with a cleanup effect. - expect(vi.getTimerCount()).toBe(0); - - // And nothing left behind should be able to invoke our `onClose` later. - act(() => { - vi.advanceTimersByTime(60_000); + // Simulate the post-unmount + post-jsdom-teardown environment: any + // subsequent access to `document.activeElement` would normally throw + // `ReferenceError: document is not defined`. Here we stub the getter so + // that an access flips a boolean we can assert against. If Radix's + // internal close timer leaks past unmount (radix-ui/primitives#3703, + // still present in 1.2.17), `handleClose` will fire after unmount and + // dereference `document.activeElement`, tripping the stub. With this + // PR's fix, we own the timer ourselves and clear it from the unmount + // cleanup, so no Radix `handleClose` ever runs post-unmount. + const originalDescriptor = Object.getOwnPropertyDescriptor( + Document.prototype, + 'activeElement' + ); + if (!originalDescriptor) { + throw new Error('Expected Document.prototype.activeElement descriptor'); + } + let leakedDocumentAccess = false; + Object.defineProperty(Document.prototype, 'activeElement', { + configurable: true, + get: () => { + leakedDocumentAccess = true; + return null; + }, }); + try { + act(() => { + vi.advanceTimersByTime(60_000); + }); + } finally { + Object.defineProperty(Document.prototype, 'activeElement', originalDescriptor); + } + expect(leakedDocumentAccess).toBe(false); expect(onClose).not.toHaveBeenCalled(); }); @@ -124,4 +164,87 @@ describe('Toast', () => { }); expect(onClose).not.toHaveBeenCalled(); }); + + it('falls back to the provider-level duration when a toast does not set its own', () => { + // Regression: before this PR, an unset `ToastProps.duration` let Radix's + // `ToastProvider duration` (the default for every toast) take effect. + // After moving the timer into click-ui's wrapper, we must keep the same + // resolution order: prop > provider > built-in default. + const onCreated = vi.fn(); + const TriggerWithSpy = () => { + const { createToast } = useToast(); + useEffect(() => { + createToast({ title: 'hello' }); + onCreated(); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []); + return null; + }; + + renderInToastProvider(, { duration: 2000 }); + expect(onCreated).toHaveBeenCalledTimes(1); + + // The toast was created with no explicit `duration`, so it should pick + // up the provider's 2000 ms and not the built-in 5000 ms default. + act(() => { + vi.advanceTimersByTime(1999); + }); + // The DOM should still have the toast. + expect(document.querySelector('[role="status"]')).not.toBeNull(); + + act(() => { + vi.advanceTimersByTime(2); + }); + // After 2001 ms total the provider-level deadline has elapsed and the + // toast should have been removed from the provider's state, which + // unmounts it from the DOM. + expect(document.querySelector('[role="status"]')).toBeNull(); + }); + + it('closes immediately on resume when the auto-close deadline elapsed during a pause', () => { + // Regression for: if `handlePause` runs after the auto-close deadline + // has already passed (event-loop delay / throttling beat Radix's + // close-timer to the punch), remaining time clamps to 0 and a naive + // `handleResume` -> `startCloseTimer(0)` would short-circuit, leaving + // the toast open forever. + const onClose = vi.fn(); + // Use renderCUI so we get a real `Viewport` (which is what Radix's + // window-blur listener dispatches `toast.viewportPause` against). + renderCUI( + + ); + + // Advance the wall clock past the deadline WITHOUT draining the fake + // timer queue, so the close-timer hasn't run yet but `Date.now()` + // reports an elapsed time greater than `duration`. + act(() => { + vi.setSystemTime(Date.now() + 500); + }); + expect(onClose).not.toHaveBeenCalled(); + + // Simulate Radix's viewport-level pause/resume. Radix's outer + // `
` wraps the actual viewport `
    `; the + // `VIEWPORT_PAUSE`/`VIEWPORT_RESUME` custom events are dispatched on the + // inner `
      ` (that's what `ToastImpl` listens on), not the wrapper. + const viewport = document.querySelector('[role="region"] > ol'); + if (!viewport) { + throw new Error('Expected Radix Toast viewport
        to be in the DOM'); + } + act(() => { + viewport.dispatchEvent(new CustomEvent('toast.viewportPause')); + }); + // Pause cleared the timer, but the deadline already elapsed -> remaining = 0. + act(() => { + viewport.dispatchEvent(new CustomEvent('toast.viewportResume')); + }); + + // Without the fix, onClose would never be invoked. With the fix, resume + // detects `remaining <= 0` after a real pause and closes immediately. + expect(onClose).toHaveBeenCalledTimes(1); + expect(onClose).toHaveBeenCalledWith(false); + }); }); diff --git a/src/components/Toast/Toast.tsx b/src/components/Toast/Toast.tsx index 1721f7efc..2a3922739 100644 --- a/src/components/Toast/Toast.tsx +++ b/src/components/Toast/Toast.tsx @@ -1,4 +1,11 @@ -import { createContext, useCallback, useEffect, useRef, useState } from 'react'; +import { + createContext, + useCallback, + useContext, + useEffect, + useRef, + useState, +} from 'react'; import * as RadixUIToast from '@radix-ui/react-toast'; import { keyframes, styled } from 'styled-components'; import { toastsEventEmitter } from './toastEmitter'; @@ -15,6 +22,13 @@ export const ToastContext = createContext({ createToast: () => null, }); +// Provider-level default duration. Mirrors Radix's `ToastProvider duration` +// prop semantics so that consumers can do `` +// (or ``) to set the +// fallback for every toast that doesn't explicitly set its own `duration`. +// Defaults to `DEFAULT_TOAST_DURATION`. +const ToastDurationContext = createContext(DEFAULT_TOAST_DURATION); + // Lazy wrapper to avoid circular dependency issues at module load time const ToastIconWrapper = (props: IconProps & { $type?: ToastType }) => ( @@ -128,11 +142,18 @@ export const Toast = ({ title, description, actions = [], - duration = DEFAULT_TOAST_DURATION, + duration: durationProp, onClose, ...props }: ToastProps & { onClose: (open: boolean) => void }) => { + // Resolve the effective duration: explicit prop wins, then the provider-level + // duration from `ToastDurationContext` (`` / + // ``), then Radix's + // historical default of 5000 ms. + const providerDuration = useContext(ToastDurationContext); + const duration = durationProp ?? providerDuration; + let iconName = ''; if (type === 'default') { iconName = 'info-in-circle'; @@ -153,6 +174,11 @@ export const Toast = ({ const closeTimerRef = useRef | undefined>(undefined); const closeTimerStartTimeRef = useRef(0); const closeTimerRemainingTimeRef = useRef(duration); + // Tracks whether `handlePause` actually paused a running timer. Without + // this, `handleResume` could not tell apart "duration was 0/unset, never + // had a timer" from "timer was running and got paused with no time left", + // and the latter case must close the toast immediately on resume. + const wasPausedRef = useRef(false); // Stable ref to the latest `onClose` so that `startCloseTimer` does not // depend on `onClose`'s identity. Without this, the auto-close `useEffect` @@ -190,6 +216,7 @@ export const Toast = ({ useEffect(() => { closeTimerRemainingTimeRef.current = duration; + wasPausedRef.current = false; startCloseTimer(duration); return clearCloseTimer; }, [duration, startCloseTimer, clearCloseTimer]); @@ -204,9 +231,23 @@ export const Toast = ({ closeTimerRemainingTimeRef.current - elapsed ); clearCloseTimer(); + wasPausedRef.current = true; }, [clearCloseTimer]); const handleResume = useCallback(() => { + if (!wasPausedRef.current) { + return; + } + wasPausedRef.current = false; + // If the deadline elapsed during the pause window (e.g. event-loop delay + // or timer throttling pushed the pause past the auto-close deadline + // before the timer callback ran), close immediately on resume instead of + // letting `startCloseTimer(0)` short-circuit and leave the toast open + // forever. + if (closeTimerRemainingTimeRef.current <= 0) { + onCloseRef.current(false); + return; + } startCloseTimer(closeTimerRemainingTimeRef.current); }, [startCloseTimer]); @@ -291,6 +332,7 @@ export interface ToastProviderProps extends RadixUIToast.ToastProviderProps { export const ToastProvider = ({ children, align = 'end', + duration: providerDuration = DEFAULT_TOAST_DURATION, ...props }: ToastProviderProps) => { const [toasts, setToasts] = useState>>({ @@ -343,18 +385,21 @@ export const ToastProvider = ({ - - {children} - {Array.from(toasts[align]).map(([id, toast]) => ( - - ))} - + + + {children} + {Array.from(toasts[align]).map(([id, toast]) => ( + + ))} + + );