-
Notifications
You must be signed in to change notification settings - Fork 227
Conversation
be07489
to
4cafbed
Compare
cc/ @michenly |
@michenly can you please review this when you get a chance? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall this looks great, readable code and well tested.
The main thing this lacks for me right now is documentation, I'd like to see some added to the README, and I think it might be a good case for adding a simple TSDoc style comment so editors can pickup some context to display about what the options do (maxWait
in particular is somewhat unintuitive)
maxWait?: number; | ||
} | ||
|
||
export default function debounce<F extends (...args: any[]) => void>( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A TSDoc comment might be nice here to get some tooltip explanations of what the options here do. We do some of that in Quilt libraries such as react-form
and I think here it could be helpful.
PS @AndrewMusgrave sorry for the long wait |
Thanks for the review @TheMallen π I'll add some docs at the beginning of next week π |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions. |
Description
Add debounce and throttle to function enhancers from polaris-rails and the tests πββοΈ and typescriptify it. We never return a value this version of debounce, I'll create an issue for it to be addressed later.
Type of change
Checklist