Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: debounce cancel() should clear pending timeout #108

Closed
wants to merge 1 commit into from

Conversation

adamhamlin
Copy link
Contributor

Tip

The owner of this PR can publish a preview release by commenting /publish in this PR. Afterwards, anyone can try it out by running pnpm add radashi@pr<PR_NUMBER>.

Summary

This PR changes the behavior of a debounced function's cancel() method:

BEFORE: Calling cancel() preempts any pending invocation and permanently makes all future invocations execute immediately (i.e., not be debounced)
AFTER: Calling cancel() cancels/clears any pending invocation. Subsequent invocations are debounced as normal.

Related issue, if any:

Closes #105

For any code change,

  • Related documentation has been updated, if needed
  • Related tests have been added or updated, if needed
  • Related benchmarks have been added or updated, if needed

Does this PR introduce a breaking change?

Yes

@aleclarson sounds like we're considering this a "bug", so not sure we'd do anything other than make a note in release notes? Please advise

Bundle impact

Calculating...

@adamhamlin adamhamlin force-pushed the fix/debounce-cancel branch from 6065256 to 61c9888 Compare July 13, 2024 04:08
@adamhamlin adamhamlin force-pushed the fix/debounce-cancel branch from 61c9888 to 353a7fc Compare July 13, 2024 04:10
@adamhamlin adamhamlin marked this pull request as ready for review July 13, 2024 04:11
@adamhamlin adamhamlin requested a review from aleclarson as a code owner July 13, 2024 04:11
@aleclarson
Copy link
Member

aleclarson commented Jul 13, 2024

Looks like we fixed this at the same time: #107. Sorry for the communication fail on my part.

I will add Co-Authored-By: + you to that PR to show appreciation for your work here. ✌️

Lastly, I'll be treating this as a breaking change, since its old behavior was documented.

@aleclarson aleclarson closed this Jul 13, 2024
@aleclarson aleclarson added the duplicate This issue or pull request already exists label Jul 13, 2024
@adamhamlin
Copy link
Contributor Author

Looks like we fixed this at the same time: #107. Sorry for the communication fail on my part.

No worries, I didn't communicate either. I would've just assigned the issue to myself, but I don't have the perms. Do you wanna go that route for deconfliction? Or just claim the issue by commenting on it? Something else?

@adamhamlin adamhamlin deleted the fix/debounce-cancel branch July 14, 2024 12:55
@aleclarson
Copy link
Member

Sorry @adamhamlin, I somehow missed your reply!

I would've just assigned the issue to myself, but I don't have the perms. Do you wanna go that route for deconfliction? Or just claim the issue by commenting on it? Something else?

Oh darn, I figured adding you to the team would give you such permissions. You should have them now, though, as well as some others.

@adamhamlin
Copy link
Contributor Author

Oh darn, I figured adding you to the team would give you such permissions. You should have them now, though, as well as some others.

All good. And thanks, I do have assignment perms now. Tho the link you posted is a 404 for me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate This issue or pull request already exists
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix debounce cancel
2 participants