-
Notifications
You must be signed in to change notification settings - Fork 464
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Replace useImmediate* hooks with react-wonka #447
Conversation
It's probably fair to bundle this for now. It's tiny and this way we can compare the bundlesize to before (+.02kB min+gzip)
@@ -25,7 +24,8 @@ export const useMutation = <T = any, V = object>( | |||
query: DocumentNode | string | |||
): UseMutationResponse<T, V> => { | |||
const client = useClient(); | |||
const [state, setState] = useImmediateState<UseMutationState<T>>({ | |||
|
|||
const [state, setState] = useState<UseMutationState<T>>({ |
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.
Small note on this change; This is not a breaking change. Since mutations are realistically run only after a component has mounted, useImmediateState
was never doing anything here
This gets rid of some of the internal structures and functions in favour of a simpler Source memo.
e269a64
to
085ab12
Compare
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.
Amazing work, can only imagine how much time went into this
This is intended to improve the stability of our hooks and ensure that we won't run into problems with concurrent mode in the future. At its core is a tiny library I've written,
react-wonka
. https://github.com/kitten/react-wonkaIt takes care of transforming changing inputs to streams and using the given output stream to then return output values from its hooks, synchronously if possible, and with updates if not.
It is aware of concurrent mode and uses an internal counter to trigger updates, which seems to be popular these days 😓
It's tests are more thorough than what we could write realistically for each of our hooks: https://github.com/kitten/react-wonka/blob/master/src/index.test.ts
In practice this may even make
useQuery
anduseSubscription
look a little nicer, if someone is open to help me tweak the implementation a little. Doesn't look 100% pretty yet 😅In practice, our bundle-size only changes by
+0.02kB min+gzip
(this branch compared to the last release) when I'm includingreact-wonka
in the bundle, which seems better for a fair comparison.This should also improve our handling of suspense. Our
useImmediateEffect
hook runs normal effects after the initial mount. When we update a hook we'd right now throw a suspense-promise in a normal effect, which is not allowed.