Skip to content
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

Closed
bennettdams opened this issue Mar 23, 2021 · 6 comments
Closed

Bug: Nondeterministic first render #21059

bennettdams opened this issue Mar 23, 2021 · 6 comments
Labels
Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug

Comments

@bennettdams
Copy link

bennettdams commented Mar 23, 2021

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

See on CodeSandbox

enter image description here

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:

  1. the "normal" way
  2. with a "mounted" render hack
  3. with the same "mounted" render hack, but with an additional 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:

function ChartWithMountHack({ data }: { data: Data }) {
  // initially not mounted
  const [isMounted, setIsMounted] = useState<boolean>(false);

  useEffect(() => {
    // "Now I've been mounted!"
    setIsMounted(true);
  }, []);

  return !isMounted ? <p>Loading</p> : <Chart data={data} />;
}

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 the useEffect where isMounted 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)?

@bennettdams bennettdams added the Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug label Mar 23, 2021
@gaearon
Copy link
Collaborator

gaearon commented Mar 23, 2021

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:

  • Whether this happens in 16.x versions or is new to 17.x
  • What is the actual sequencing here (e.g. check whether we get into commitRoot with the initial loading state or not)

@gaearon
Copy link
Collaborator

gaearon commented Mar 23, 2021

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 useEffect is not enough.)

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.

@ktfth
Copy link

ktfth commented Apr 2, 2021

I give a try to use the older version of react and result is this

@bennettdams
Copy link
Author

  • Whether this happens in 16.x versions or is new to 17.x

Yes, it also happens for v16.14.0:
https://codesandbox.io/s/react-nondeterministic-rendering-react-16140-x02gc?file=/src/App.tsx

@bennettdams bennettdams changed the title Bug: Nondeterministic first render (due to batched updates?) Bug: Nondeterministic first render Apr 6, 2021
@bennettdams
Copy link
Author

Usings React's startTransition with v18.x makes this issue irrelevant:
https://codesandbox.io/s/react-nondeterministic-rendering-react-18xx-pi6kq?file=/src/App.tsx

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.

@gaearon
Copy link
Collaborator

gaearon commented Mar 30, 2022

Looks like this works with 18.

https://codesandbox.io/s/react-nondeterministic-rendering-react-18-x-x-forked-88igm7?file=/src/App.tsx

@gaearon gaearon closed this as completed Mar 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug
Projects
None yet
Development

No branches or pull requests

3 participants