-
Notifications
You must be signed in to change notification settings - Fork 27
feat: add useAsyncSetState #12
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
Conversation
|
We occasionally need this, like in our auth contexts |
Codecov Report
@@ Coverage Diff @@
## master #12 +/- ##
==========================================
+ Coverage 58.14% 60.89% +2.75%
==========================================
Files 27 28 +1
Lines 270 289 +19
Branches 63 65 +2
==========================================
+ Hits 157 176 +19
Misses 113 113
Continue to review full report at Codecov.
|
src/useAsyncSetState.ts
Outdated
| setState(stateBefore => { | ||
| try { | ||
| let nextState: TState | ||
| if (typeof update === 'function') { |
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.
i don't suppose instanceof makes TS happy?
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.
it does but instanceof Function is weird
src/useAsyncSetState.ts
Outdated
|
|
||
| useEffect(() => { | ||
| resolvers.current.forEach(resolve => resolve(state)) | ||
| resolvers.current = [] |
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.
| resolvers.current = [] | |
| resolvers.current.length = 0; |
easier? :p
| // react won't re-render and effect will not resolve. If there are already | ||
| // resolvers queued, then it should be safe to assume an update will happen | ||
| if (!resolvers.current.length && Object.is(nextState, prevState)) { | ||
| resolve(nextState) |
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.
ok; you could also add another "should run resolve manually flag", but i guess eagerly resolving the first few is fine
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.
ah right in the case of a bunch of sets. yeah that's a good approach
No description provided.