Closed Dremora closed 2 years ago
@Dremora where are you getting mutateAsync
from? Blitz exports mutateAsync
as the first item in the tuple returned from useMutation.
@flybayer in the example above, I'm importing useMutation
directly from react-query
. The issue still persists when using useMutation
from Blitz (and originally was discovered in the code that was using Blitz version of the hook), I just wanted to keep the Blitz and non-Blitz examples as close as possible, and avoid writing any backend code.
@Dremora got it.
So I'm not sure what the difference here is, but you need to add useCallback
around your submit
function so that the useEffect doesn't run on every render. Like this
const submit = useCallback(() => {
console.log("submit called");
mutateAsync();
}, [mutateAsync]);
That makes the blitz version work.
Also, for what it's worth, "scheduling" an async function like you are with useEffect is an anti-pattern, and it's possible to cause random bugs with react 18.
But we'll try to investigate further later to see why blitz is different.
I have deliberately left out useCallback
to simulate what is happening in the real world code that I have based my example on. There, submit
function depends on the data that is being submitted, and it also updates the data before submitting it, which shouldn't be a problem, given that by the next render isSubmitScheduled
should become false
(except it doesn't). The temporary solution was to exclude submit
function from the dependency array of useEffect
.
I haven't heard of this being an anti-pattern, if you have any links to React documentation showing why this is a bad practice, it'd be appreciated. In fact, I have introduced this in order to work correctly with useTransition
. The use case is the following. We run form validation inside useTransition
in order to improve form performance, and thus validation state lags behind form state. We can't submit form state when the user presses the submit button, we only do so when validation catches up with the form state (and isPending
becomes false).
Ok I see.
Found the difference. In your CRA example console:
Warning: ReactDOM.render is no longer supported in React 18. Use createRoot instead. Until you switch to the new API, your app will behave as if it's running React 17. Learn more: https://reactjs.org/link/switch-to-createroot
Once you change to this:
const root = ReactDOM.createRoot(document.getElementById('root'));
root.render(<App />)
then the console.log render order is exactly the same as Blitz.
You cannot depend on render & useEffect order. It's a react implementation that's subject to change.
In general this sounds like something better suited to a bit of xstate code or possibly the new useEvent hook (not available yet). Managing async code with useEffect is always a recipe for disaster, although you can mitigate it somewhat by using refs.
You can use a ref to ensure submit is only called once like this. This works in the blitz example without useCallback.
const wasSubmitted = useRef(false);
useEffect(() => {
if (isSubmitScheduled && !wasSubmitted.current) {
setIsSubmitScheduled(false);
submit();
wasSubmitted.current = true;
}
}, [isSubmitScheduled, submit]);
Thanks for discovering where the difference comes from! I can indeed confirm it's not an issue with Blitz.
I still, however, don't think this is expected behaviour, or that this combination of asynchronous code should lead to unpredictable results. State updates should either be batched, or applied in the order they were called. I dug deep into react-query's source code, and managed to narrow it down to state being updated within Promise.resolve
within useEffect
, and raised a bug against React: https://github.com/facebook/react/issues/24649. There are many related bugs raised there as well, so I hope this gets fixed!
What is the problem?
The exact conditions seem to be quite elusive and hard to describe, but the bug is consistently reproducible. Best to demonstrate it with some code.
Why am I raising bug request with Blitz? Because exactly the same code works well without it. See this create-react-app codesandbox with same verions of React and react-query.
Paste all relevant code snippets here:
See this codesandbox for a live example.
mutateAsync
here comes fromreact-query
. When usinguseMutation
provided by Blitz itself, the outcome doesn't change.What are detailed steps to reproduce this?
What I expect to see in the console (after button is clicked) (actual output of create-react-app version):
What I actually get:
For a slightly more real-world scenario, see this codepen. Here the order of rendering maters, because it effectively freezes the app, causing an infinite render cycle.
Run
blitz -v
and paste the output here:Note that the issue persists with Blitz v2 alpha, as well as React 17.