Skip to content

fix(useThrottledCallback): invoke the callback when the first value is false - #404

Merged
hyesungoh merged 1 commit into
toss:mainfrom
bbjbc:fix/throttled-callback-initial-false
Aug 13, 2026
Merged

fix(useThrottledCallback): invoke the callback when the first value is false#404
hyesungoh merged 1 commit into
toss:mainfrom
bbjbc:fix/throttled-callback-initial-false

Conversation

@bbjbc

@bbjbc bbjbc commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Overview

useThrottledCallback drops the very first call when the value is false.

The hook skips redundant invocations by comparing the incoming value against the last one it forwarded to onChange. That comparison is seeded with false:

const ref = useRef({ value: false, clearPreviousThrottle: () => {} });
...
if (nextValue === ref.current.value) {
  return;
}

Since the hook takes no initial value from the caller, a first call of false matches the seed and returns early, so onChange never fires.

const onChange = vi.fn();
const { result } = renderHookSSR(() => useThrottledCallback({ onChange, timeThreshold: 100 }));

result.current(false);
// onChange is never called

result.current(true);
// works. only the first `false` is lost

This matters for consumers whose state does not start at false. If a panel is open by default, the first close emits false, and that transition is silently dropped with no way to tell the hook the state started as true.

useDebouncedCallback has the same seed. I left it alone here because #267 is already changing that line. Its generic rewrite switches the seed to null, which fixes this case as a side effect, though a narrower version of the problem survives there: a first call of null would still be dropped.

Changes

  • Seed the comparison with a module-level sentinel instead of false, so the first call is forwarded regardless of its value. The dedup comparison itself is unchanged, so repeated values are still skipped.
  • Add two tests: one for the first false, one asserting a repeated false is still skipped so the dedup behavior stays pinned.

A Symbol is used rather than null or undefined because it cannot collide with a caller-supplied value if this hook later becomes generic, as useDebouncedCallback is doing in #267.

Existing tests all start with result.current(true), which is why this was not caught. The early-return branch is already covered by the repeated-true cases, so branch coverage was 100% while the first-call-false path was never exercised.

Checklist

  • Did you write the test code?
  • Have you run yarn run fix to format and lint the code and docs?
  • Have you run yarn run test:coverage to make sure there is no uncovered line?
  • Did you write the JSDoc?

…s false

The hook compares an incoming value against the last one it forwarded to
onChange so it can skip redundant invocations. That comparison was seeded
with `false`, but the hook never receives an initial value from the caller,
so a first call of `false` looked redundant and was dropped.

Consumers whose state starts as `true` therefore lost the transition back
to `false` entirely, with no way to tell the hook otherwise. Seed the
comparison with a sentinel instead so the first call is always forwarded.
@changeset-bot

changeset-bot Bot commented Aug 13, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 8f04f14

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
react-simplikit Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (ea7deae) to head (8f04f14).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##              main      #404   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           58        58           
  Lines         1639      1643    +4     
  Branches       499       499           
=========================================
+ Hits          1639      1643    +4     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@bbjbc
bbjbc force-pushed the fix/throttled-callback-initial-false branch 2 times, most recently from be481ef to 8f04f14 Compare August 13, 2026 14:54
@bbjbc bbjbc removed their assignment Aug 13, 2026

@hyesungoh hyesungoh left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution! 👍

@hyesungoh
hyesungoh merged commit cac80cb into toss:main Aug 13, 2026
29 checks passed
@github-actions github-actions Bot mentioned this pull request Aug 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants