-
Notifications
You must be signed in to change notification settings - Fork 94
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
Add useAsync
hook to leverage new Hooks proposal
#9
Merged
Merged
Changes from 1 commit
Commits
Show all changes
22 commits
Select commit
Hold shift + click to select a range
cf5ff97
Add useAsync.
ghengeveld 4168f2b
Don't include spec files in build.
ghengeveld edb8913
Add useAsync to README.
ghengeveld 0ba8813
Nicer way to pass a value to the setTimeout callback.
ghengeveld 3c6a75b
Add first (failing) test for useState.
ghengeveld 4e4d1d7
Add support for shorthand useState.
ghengeveld d8a8245
Update useAsync to use useRef for instance variables.
ghengeveld 246b41a
Fix tests.
ghengeveld 735d7b4
Use flushEffects instead of the rerender hack.
ghengeveld ddbcaef
Merge branch 'master' into useAsync
ghengeveld e213f0e
Fix static member definitions.
ghengeveld f20944f
Merge branch 'master' into useAsync
ghengeveld a15adbd
Merge branch 'master' into useAsync
ghengeveld 907d226
Add typings for useAsync.
ghengeveld 8cf78ed
Sync package-lock.
ghengeveld e3dadc9
Make sure the right React version is installed to test useAsync with.
ghengeveld 6fdf304
Add test for cancel().
ghengeveld 46f8b8c
Fix coverage reporting.
ghengeveld 5d1442a
Merge branch 'master' into useAsync
ghengeveld a1ffcb7
Fix conflict.
ghengeveld 6b1f521
Restructure tests.
ghengeveld 73e6cb2
Use latest React version with hooks.
ghengeveld File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Add useAsync to README.
- Loading branch information
commit edb89139ae50a763a24d9c2056c9c3891fb2b452
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Can we avoid isLoading, and use React Suspense instead?
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.
Suspense is not released yet. Eventually React Async will support both. At first, React Async will maintain its API but use Suspense underneath. Later on I'll look into better integration with the Suspense components (e.g. Placeholder/Timeout). Right now there's no telling how Suspense will be used in practice.
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 have to agree with @sibelius. It seems like any new api surface for async right now should at least consider the usage of resources, cache invalidation and suspense. The react team has mentioned that react-cache is a WIP and that invalidation is outstanding. Their vision seems to be using resource for data fetching. Any new api surface surface should at least be able to show how it fits into that future.
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 agree, I want React Async to be future compatible. I'm confident it will still be relevant despite Suspense offering very similar features. Personally I'm surprised that they even took the whole thing as far as providing a cache mechanism. That to me sound like a very application specific thing. A simple library around Promises could easily provide the same functionality, which is why React Async doesn't do any caching, you can simply bring your own.
So what would closer integration with Suspense look like? I'm hesitant to adopt the same API, and relying on interop between the two (i.e. using Async with Timeout or Placeholder) seems awkward and error prone. What are your thoughts on this?
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 haven't dug into code that does the suspending in React reconciler, but the mechanism how WIP
react-cache
package gets render to suspend is through throwing a pending promise -https://github.com/facebook/react/blob/master/packages/react-cache/src/ReactCache.js#L158 Before throwing the pending promise out to React renderer, one needs to store the promise somewhere, to access it again when promise gets resolved and render is retried, hence the need forreact-cache
. Also I think thereact-cache
won't be mandatory for suspense, so anyone can roll their own:Atleast that's how I understand it.