useBountyStatus accepts onStatusChange and fallbackBounty and then ignores both: the status-change callback is never invoked by any code path

Open
  • Is something wrong?
#844 0 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

Nobody has claimed this yet.

Assessment

Difficulty
3/5
Estimated time
1-2 days
Newbie friendliness
72/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
react, typescript
Domain
frontend

Research direction

Start by reading src/hooks/useBountyStatus.ts and the shared registry in src/hooks/bountyPoller; check the hook's consumers to understand how they use both options. Implement the intended callback and fallback behavior, making the first status notification decision explicit and keeping the reported source accurate. Done when tests cover status transitions and the initial fallback state, and the hook no longer silently ignores either option.

Written by the indexing model from the issue text.

Description

bug

Location

src/hooks/useBountyStatus.ts:16-22 (the UseBountyStatusOptions interface) and :35-48 (the hook body).

Problem

Both options are destructured and never used again:

export function useBountyStatus({
  bountyId,
  fallbackBounty,     // never referenced below
  interval = 5000,
  enabled = true,
  onStatusChange,     // never referenced below
}: UseBountyStatusOptions): UseBountyStatusResult {

Search the file: after that destructure, neither identifier appears again. The body is a useSyncExternalStore over the module-level registry in ./bountyPoller, and that registry has no notion of either option — subscribeBounty(id, listener, interval) takes no callback and no fallback, and notify() only invokes the store-change listeners React registered.

So:

  • onStatusChange is a dead callback. A consumer that passes it (to refresh the page when a bounty flips to claimed, to re-render a payout panel when it flips to paid, to stop offering a Claim button) is silently never called, for any transition, ever. There is no runtime error and no type error — the option is declared, so it typechecks and reads as supported.
  • fallbackBounty is a dead input. The hook's UseBountyStatusResult.bounty comes solely from getBountySnapshot(bountyId).bounty, which is null until the first successful poll, so a caller that already has a server-rendered bounty and passes it as fallbackBounty still gets bounty: null on first paint and renders an empty/loading state for a bounty it already has in hand.

This looks like fallout from the #631 rewrite that replaced the hook's internal useSmartPolling loop with the shared registry: the options survived the rewrite, their implementations did not. (Note this is the opposite of the previously reported behaviour where onStatusChange fired with a wrong value — on the current default branch it does not fire at all, and fallbackBounty is not substituted at all. Worth reconciling the two when triaging.)

Why it matters

The failure mode is a silent no-op on the bounty detail screen: whatever a consumer wired to onStatusChange simply does not happen. Because the option is accepted, nothing — not the compiler, not a lint rule, not a runtime warning — tells the author their callback is dropped. The status itself is still returned, so a consumer can poll status in its own useEffect instead, which means the bug reads as "my callback never ran" rather than "the data is wrong", and is easy to misattribute to the poller.

Reproduction

const { status } = useBountyStatus({
  bountyId: "b1",
  interval: 1000,
  onStatusChange: (s) => console.log("changed to", s),
});
  1. Render this against a backend where bounty b1 is open.
  2. Claim b1 from another session so the backend status becomes claimed.
  3. status returned by the hook updates to "claimed" on the next tick, but nothing is ever logged.

Suggested fix

Either implement the options or remove them from the public type — do not leave them declared and unimplemented.

To implement onStatusChange, derive the transition in the hook rather than in the shared registry (each subscriber wants its own callback, and the registry is shared):

const prevStatusRef = useRef<BountyStatus | null>(null);
const status = snapshot.bounty?.status ?? null;
useEffect(() => {
  if (status !== null && status !== prevStatusRef.current) {
    const first = prevStatusRef.current === null;
    prevStatusRef.current = status;
    if (!first) onStatusChange?.(status);   // or fire on first observation, document which
  }
}, [status, onStatusChange]);

Note the first-observation decision needs to be explicit and documented: firing on the initial load is what produced spurious notifications before.

For fallbackBounty, return it when the registry has nothing yet (bounty: snapshot.bounty ?? fallbackBounty ?? null) and keep source honest by reporting a distinct value (or null) for that case, so a consumer can still tell server-rendered-fallback from polled-live.

Dominant language
TypeScript
Stars
1
Forks
94
Avg merge
1h 2m
Merged PRs (30d)
25

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from MergeFi/frontend

All issues in MergeFi/frontend

Similar issues

More TypeScript issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.