-
Notifications
You must be signed in to change notification settings - Fork 46.4k
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
Bug: Nondeterministic first render #21059
Comments
I would not expect this to be related to batched updates, since generally effects execute after paint. Although I think there might be some cases where we flush them more eagerly than necessary. I would appreciate if someone could look at:
|
Re: idiomatic way to do it. I think currently what you're doing is probably the best we have so far. (But like you said, I think it's worth investigating why just doing it in In a future version of React using Concurrent Mode, we'll likely have a more idiomatic solution for this. See #19936. I made a sandbox showing that solution here: function ChartWithSuspense({ data }: { data: Data }) {
return (
<Suspense unstable_expectedLoadTime={1000} fallback={<p>Loading </p>}>
<Chart data={data} />
</Suspense>
);
} But that API is only available in experimental versions and will likely change. It might not even make it into the initial Concurrent Mode release. However, it's definitely on our mind. |
I give a try to use the older version of react and result is this |
Yes, it also happens for v16.14.0: |
Usings React's As you can see, the render is no longer blocking when beginning the state update (aka. re-rendering the chart). Only the real painting blocks the render shortly, but I can't find differences with loading states (like we've seen with v17.x) anymore. |
Looks like this works with 18. |
It is possible that this is not a bug, but rather expected - I couldn't find any material on the internet for this particular use case though.
React version: v17.0.1
TL;DR React sometimes renders a loading state and sometimes not, without changes in the UI. Maybe this has something to do with batched updates, where the initial state and the next state are batched?
Steps To Reproduce
Here's the setup, a chart that takes a long time to render. So long that the render is blocking. There are three different ways to render the chart here:
setTimeout
Option 2 & 3 both have a small
useState
to check whether they've been mounted. I do this to show a "Loading" state conditionally:I did this, because I want to show a "Loading" state instead of a blocking render, so e.g. page switches or ternary rendering (e.g.
hasData ? <p>No data</p> : <Chart />
) are shown immediately, instead of blocking. (If there are better ways, please let me know!)The current behavior
Now, each button will render one of the three options/charts. Again, the second and third chart have a small hack to check whether they're mounted or not.
Try clicking on the first button and the second button back & forth quickly.
You will see that sometimes the "Chart with mount hack" will ("correctly") render the "Loading" state, but sometimes it just doesn't render the "Loading" - instead it blocks the render up until the chart is finished rendering (skips the "Loading" state).
I think this is due to the render cycles and whether you get the two updates in one cycle of the batching. (first:
isMounted === false
-> second:isMounted === true
)I can't really tell how to reproduce this, hence the "nondeterministic" in the title. Sometimes you also have to click on "Regenerate data" and click back & forth after that.
The expected behavior
Option 3 ("Chart with mount hack with timeout") ALWAYS gives me the "Loading" state, which is exactly what I want. The only difference to option 2 is using a
setTimeout
in theuseEffect
whereisMounted
is set to true.setTimeout
is used here to break out of the update batching.Is there a better way to opt-out of the batching, so
isMounted
will always render with its initial value (false
)?Shouldn't the first render and its initial values be outside of the batched updates? If not, how can I tell React to do so (given that stuff like async/
setTimeout
/.. are just "hacks" to circumvent that right now)?The text was updated successfully, but these errors were encountered: