Skip to content

useAsync is the only hook in the codebase with no test file, despite being the async primitive every data component depends on #426

Description

@Jagadeeshftw

Priority: High  ·  Area: Test coverage / core hook  ·  Est. effort: 7–11 h

📌 Problem

src/hooks/ contains nine hooks. Eight have a matching .test.ts:

useDebouncedValue ✓   useFocusShortcut ✓   useInterval ✓   useQueryState ✓
useSortableData ✓     useTheme ✓           useToast ✓      useWallet ✓
useAsync          ✗   ← no test file

useAsync is the one that matters most. Its documented contract (src/hooks/useAsync.ts:12-27) is "Runs an abortable async loader on mount and exposes a reload trigger", returning four members — state, reload, refresh and mutate — over an AsyncState<T> union of loading/ready/error. Every component that loads data goes through it.

Its behaviour is exactly the kind that breaks silently:

  • Abort on unmount — does an in-flight request get cancelled, or does it resolve into an unmounted component?
  • Race handling — if reload is called while a request is in flight, does the stale response win?
  • refresh vs reload — the doc says reload "also surfaces a loading state", implying refresh does not. Untested, that distinction drifts.
  • mutate — optimistic updates followed by a real response.
  • Error transitions — does a failed reload after a successful load clear the data or keep it?

There is corroborating evidence this is already a problem: MetricsBar.test.tsx mocks useAsync with only { state, refresh }, omitting reload and mutate — a mismatch that produces type errors and is tracked separately. Components are mocking a hook nobody has pinned down.

🎯 Design decision required

State and defend:

  1. Behaviour inventory. Read useAsync.ts and enumerate every behaviour worth asserting before writing tests. Post it in the PR — the inventory is the deliverable that makes the tests reviewable.
  2. Race and abort strategy. These need deterministic control over promise resolution order. Describe your approach — deferred promises, fake timers, or a controllable mock loader — and why it is not timing-dependent.
  3. Behaviour that turns out to be wrong. If a test reveals the hook mishandles a race or leaks an update after unmount, that is a bug report, not a reason to adjust the test. Say so explicitly if it happens.

🧩 Requirements and context

  • Cover every member of the returned contract: state, reload, refresh, mutate.
  • Cover all three AsyncState variants and the transitions between them.
  • Abort-on-unmount must be tested. A React state update after unmount is a real defect class.
  • Tests must be deterministic — no reliance on real elapsed time. Prove it with three consecutive runs.
  • Do not change useAsync's behaviour unless a test exposes a genuine bug; if it does, report it prominently and fix minimally.
  • The repo configures 95% coverage thresholds (vitest.config.ts), though CI does not currently enforce them (separate issue). Aim to meet them for this file.

🛠️ Suggested execution

  1. Post the behaviour inventory.
  2. Write tests for the happy path and all state transitions.
  3. Add abort-on-unmount and concurrent-reload race tests.
  4. Add mutate and refresh-vs-reload tests.
  5. Report any defect found.

✅ Acceptance criteria

  • The PR contains the behaviour inventory for useAsync.
  • src/hooks/useAsync.test.ts exists and covers state, reload, refresh and mutate.
  • All three AsyncState variants and their transitions are covered.
  • A test proves an in-flight request is aborted on unmount with no post-unmount state update.
  • A test covers a reload issued while a request is in flight, asserting the stale response does not win.
  • The refresh vs reload loading-state distinction is asserted.
  • Tests pass across three consecutive runs with no flakiness.
  • Any defect found is reported, not silently patched.

🚫 Out of scope

  • Rewriting useAsync, unless a test exposes a real bug.
  • MetricsBar.test.tsx's mock mismatch — separate issue, though your tests define the contract it should mock.
  • Enforcing coverage in CI — separate issue.

🧪 Verification

npm ci
npm test src/hooks/useAsync.test.ts
npm test && npm test && npm test
npx tsc --noEmit

📤 What your PR must include

  1. The behaviour inventory.
  2. Your race/abort control strategy and why it is deterministic.
  3. Any defect the tests exposed.
  4. Determinism evidence.
  5. Closes #<n>.

🔒 Security notes

useAsync mediates every network response the application renders. An unhandled race means a stale response can overwrite fresh data — so a user can be shown balances or settlement statuses that have since changed, with no indication the view is out of date. A missing abort means responses continue to be processed after the component that requested them is gone. Both are correctness failures with user-visible consequences on a financial interface.

📋 Guidelines

  • Minimum 95% test coverage on changed lines
  • Clear documentation
  • Timeframe: 96 hours from assignment
  • One logical change per commit; no merge commits

💬 Join our community

Working on this, or want to sanity-check your approach before you start? Come and ask — the maintainers are there and happy to help.

Telegram: https://t.me/Grainlify

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    GrantFox OSSGrantFox open-source programMaybe RewardedGrantFox: potentially rewarded contributionThird CampaignGrantFox third campaign issuepriority:highHigh difficulty / architectural or cross-cuttingtestingTests and coverage

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions