🐛 Fix bug with useMutation shared results
#1616
Conversation
- Fix a scenario where the component calling `reset` needed to call it twice when using shared component results
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit f2c3606:
|
|
To test the demos below, try triggering a mutation, then resetting from the same component (i.e. click trigger, then click the reset button next to it) Beforehttps://codesandbox.io/s/examples-query-react-basic-forked-185pk?file=/src/App.tsx
Afterhttps://codesandbox.io/s/examples-query-react-basic-forked-i9vlk?file=/src/App.tsx
|
| if (promise) { | ||
| setPromise(undefined) | ||
| } else if (fixedCacheKey) { | ||
| } | ||
| if (fixedCacheKey) { | ||
| dispatch( | ||
| api.internalActions.removeMutationResult({ | ||
| requestId, |
There was a problem hiding this comment.
Hmm. Do we want to wrap the whole thing in a batch now? 🤔
There was a problem hiding this comment.
In it's current state without batch, calling reset() only seems to result in one render for each component using the hook, including the one that called the original trigger. Is there some other benefit that batch will provide?
Regardless of the above, I'm not against batching it anyway as a "might help, and can't hurt" scenario
There was a problem hiding this comment.
It depends how you call it. If you call it in an effect or click handler, React will automatically batch and only rerender once. If you do it in something async, for example after a timeout, it would lead to two renders
There was a problem hiding this comment.
ah, for some reason I assumed click handlers were exempt from the default batching. It's definitely worth batching this, I'll change that
|
LGTM :) |
resetneeded to call it twice when using shared component resultsAddresses the scenario described here: #1477 (comment)
A regression test is included.