From cfd4b72416f1637372893f80ff2646d182c18c09 Mon Sep 17 00:00:00 2001 From: galiprandi <20272796+galiprandi@users.noreply.github.com> Date: Mon, 8 Jun 2026 04:40:38 +0000 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20Quality:=20Improve=20robustness=20a?= =?UTF-8?q?nd=20documentation=20of=20AsyncBlock?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix race condition by distinguishing between manual and timeout aborts in `AsyncBlock`. - Update `AsyncBlockProps` to make the `error` prop optional. - Add comprehensive JSDoc documentation with examples for `AsyncBlock`. - Enhance test suite with race condition and optional prop test cases. - Add `vi.restoreAllMocks()` for better test isolation in `AsyncBlock` tests. --- .axioma/quality.md | 4 + lib/components/AsyncBlock/index.test.tsx | 98 +++++++++++++++++++++++- lib/components/AsyncBlock/index.tsx | 31 ++++++-- 3 files changed, 125 insertions(+), 8 deletions(-) diff --git a/.axioma/quality.md b/.axioma/quality.md index c8366d3..57eeb14 100644 --- a/.axioma/quality.md +++ b/.axioma/quality.md @@ -73,3 +73,7 @@ ## 2026-06-07 - [Explicit AbortError State Management] **Learning:** When wrapping browser-native asynchronous APIs (like the Built-in AI APIs), it's critical to explicitly handle the `AbortError` in `catch` blocks by resetting the hook's status to `'idle'`. This ensures that if an operation is cancelled (e.g., via a signal or component unmount), the UI state doesn't remain "stuck" in a loading or active state, improving the robustness of state management across the application. **Action:** Always transition the status to `'idle'` when an `AbortError` is caught in AI-related hooks to maintain synchronization between the UI and the underlying API state. + +## 2026-06-08 - Distinguishing Abort Reasons in Async Components +**Learning:** When using `AbortController` in components that also implement a `timeOut`, it is critical to distinguish between a manual abort (e.g., due to dependency change or unmount) and a timeout abort. Indiscriminately ignoring all aborted promises in a `.catch` block can swallow legitimate timeout errors. +**Action:** Use `signal.reason` to filter aborts in `.catch` blocks: `if (signal.aborted && signal.reason !== 'Timeout') return`. This ensures race conditions are prevented while still allowing the component to transition to an error state upon timeout. diff --git a/lib/components/AsyncBlock/index.test.tsx b/lib/components/AsyncBlock/index.test.tsx index d7966a2..24f05ff 100644 --- a/lib/components/AsyncBlock/index.test.tsx +++ b/lib/components/AsyncBlock/index.test.tsx @@ -1,10 +1,21 @@ -import { describe, it, expect, beforeEach, vi } from 'vitest' +import { + describe, + it, + expect, + beforeEach, + afterEach, + vi, +} from 'vitest' import { render, cleanup, screen, act, fireEvent } from '@testing-library/react' import { AsyncBlock } from './index' describe('', () => { beforeEach(cleanup) + afterEach(() => { + vi.restoreAllMocks() + }) + it('should render pending state', () => { const pendingContent = 'Loading...' render( @@ -261,4 +272,89 @@ describe('', () => { expect(onError).not.toHaveBeenCalled() vi.useRealTimers() }) + + it('should not update state if aborted due to dependency change (race condition)', async () => { + let rejectFirstPromise: (reason?: unknown) => void + const firstPromise = new Promise((_, reject) => { + rejectFirstPromise = reject + }) + + const secondPromise = Promise.resolve('Success 2') + + const promiseFn = vi + .fn() + .mockReturnValueOnce(firstPromise) + .mockReturnValueOnce(secondPromise) + + const onError = vi.fn() + const onSuccess = vi.fn() + + const { rerender } = render( +
{data}
} + error={() =>
Error
} + deps={[1]} + onSuccess={onSuccess} + onError={onError} + />, + ) + + expect(promiseFn).toHaveBeenCalledTimes(1) + expect(screen.getByText('Loading')).toBeDefined() + + // Trigger dependency change, which aborts the first promise + rerender( +
{data}
} + error={() =>
Error
} + deps={[2]} + onSuccess={onSuccess} + onError={onError} + />, + ) + + expect(promiseFn).toHaveBeenCalledTimes(2) + + // Now reject the first promise (which is already aborted) + await act(async () => { + rejectFirstPromise!(new Error('First promise failed')) + }) + + // It should NOT be in error state because the first promise was aborted + expect(screen.queryByText('Error')).toBeNull() + expect(onError).not.toHaveBeenCalled() + + // Wait for the second promise to resolve + await screen.findByText('Success 2') + expect(onSuccess).toHaveBeenCalledWith('Success 2') + }) + + it('should render nothing if error occurs and no error component is provided', async () => { + const promiseFn = () => Promise.reject(new Error('Failed')) + + const { container } = render( + 'Success'} + // error prop is missing + />, + ) + + // Wait for it to fail (it starts with "Loading") + await act(async () => { + try { + await promiseFn() + } catch { + // Ignore + } + }) + + // It should render nothing + expect(container.firstChild).toBeNull() + }) }) diff --git a/lib/components/AsyncBlock/index.tsx b/lib/components/AsyncBlock/index.tsx index 4a1c847..85b6482 100644 --- a/lib/components/AsyncBlock/index.tsx +++ b/lib/components/AsyncBlock/index.tsx @@ -9,10 +9,15 @@ import { /** * AsyncBlock component – Declaratively renders asynchronous content with pending, success, and error states. * - * @param {AsyncBlockProps} props - Props to control the behavior and rendering of the async operation. - * @returns {JSX.Element | null} - Rendered result based on async state. + * This component manages the lifecycle of an asynchronous operation (e.g., a fetch request), + * automatically handling pending, success, and error states. It also supports timeouts, + * dependency-based re-execution, and manual reloads. * - * Example usage: + * @template T - The type of data returned by the async operation. + * @param props - Props to control the behavior and rendering of the async operation. + * @returns Rendered result based on async state. + * + * @example * * ```tsx * ({ setData(null) setErr(null) - let didTimeOut = false let timer: ReturnType | null = null const signal = abortController.signal if (timeOut) { timer = setTimeout(() => { - didTimeOut = true abortController.abort('Timeout') if (isMounted.current) { setState('error') @@ -83,7 +86,7 @@ export const AsyncBlock = ({ } }) .catch((e) => { - if (signal.aborted && didTimeOut) return + if (signal.aborted && signal.reason !== 'Timeout') return if (isMounted.current) { if (timer) clearTimeout(timer) @@ -137,29 +140,43 @@ export interface AsyncBlockProps { /** * A function that returns a Promise to be executed. * Optionally receives an AbortSignal that can be used to cancel the request. + * + * @param signal - An AbortSignal to handle request cancellation. */ promiseFn: (signal?: AbortSignal) => Promise /** * Element or function to render while the promise is pending. * Optionally receives a reload function to re-run the async operation. + * + * @param reload - Function to re-run the async operation. */ pending: ReactNode | ((reload: () => void) => ReactNode) /** * Function to render if the promise resolves successfully. * Receives the data and a reload function. + * + * @param data - The data returned by the promise. + * @param reload - Function to re-run the async operation. */ success: (data: T, reload: () => void) => ReactNode /** * Function to render if the promise is rejected or times out. * Receives the error and a reload function. + * + * @param err - The error returned by the promise or the timeout error. + * @param reload - Function to re-run the async operation. */ - error: (err: unknown, reload: () => void) => ReactNode + error?: (err: unknown, reload: () => void) => ReactNode /** * Optional timeout in milliseconds before failing the promise. + * + * @default undefined */ timeOut?: number /** * Dependencies to determine when to re-run the promise. + * + * @default [] */ deps?: unknown[] /**