Throttle API calls #150
Throttle API calls #150
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
|
||
| type MembersHomePageProps = { | ||
| members: User[]; | ||
| }; | ||
|
|
||
| const EBOARD_POSITIONS = [ | ||
| "Social Coordinator", | ||
| "Senior Outreach Coordinator", | ||
| "Outreach Coordinator", |
There was a problem hiding this comment.
Text is too long
| </div> | ||
| {dropdownComponent} | ||
| {student ? ( |
There was a problem hiding this comment.
I moved the styling around a bit, since it looks weird for long text.
| if (res.code === "SUCCESS") { | ||
| router.refresh(); | ||
| } else { | ||
| alert("Please refresh the page and try again"); |
There was a problem hiding this comment.
Looks like we opt out of the alert when the request encounters an an error, maybe it's better to keep so that users get that feedback?
There was a problem hiding this comment.
I think I was lazy to handle the cases. But you're probably right that we should handle errors.
| callback: (res) => { | ||
| if (res.code === "SUCCESS") { | ||
| setShowAddFilePopUp(false); | ||
| router.refresh(); |
There was a problem hiding this comment.
I noticed that not all the states are reset, but I tested the AddFile component by completing and cancelling the form and it works well currently.
src/hooks/useApiThrottle.tsx
Outdated
| return; | ||
| } | ||
| setFetching(true); | ||
| await props.fn(...args).then(props.callback); |
There was a problem hiding this comment.
can callback be combined into fn? that way you're just passing in a single function into useApiThrottle
There was a problem hiding this comment.
Discussed with @johnny-t06. We think having callback separately make it nice to use the hooks directly using a client-side API without the need to wrap it.
src/hooks/useApiThrottle.tsx
Outdated
| await props.fn(...args).then(props.callback); | ||
| setFetching(false); | ||
| }, | ||
| [props, fetching] |
There was a problem hiding this comment.
would recommend destructuring props immediately in this hook! looks like the consumer of useApiThrottle is actually creating a new props object on each render, so memoization here isn't actually doing anything
There was a problem hiding this comment.
Got it, thanks for the catch!
src/hooks/useApiThrottle.tsx
Outdated
| * Prevent additional API call until the most recent one has completed. | ||
| */ | ||
| const useApiThrottle = <T extends ApiCall>(props: UseApiThrottleProps<T>) => { | ||
| const [fetching, setFetching] = React.useState(false); |
There was a problem hiding this comment.
could also useRef here in case the callback gets called twice before state updates
There was a problem hiding this comment.
Is useRef safe we use current to display the loading indicator?
Ah got it. I think we will stick with the current options for now, given our use case! Thank you for the suggestion!! |
nickbar01234
force-pushed
the
nickbar01234/throttle-api
branch
from
1e1636f to
40b977f
Compare
|
Merging in ahead for client meeting! |
Description
Add throttling to prevent multiple API calls.
Issues
Resolve #147.