facebook / react

The library for web and native user interfaces.
https://react.dev
MIT License
228.15k stars 46.67k forks source link

Question: Transition starvation in React 18? #21668

Open Ephem opened 3 years ago

Ephem commented 3 years ago

Main question:

What happens if a transition gets constantly interrupted before it has time to finish, and are there any cases where we need to worry about this in userland?

Subquestion:

What exactly interrupts a transition? Is it only user events, or also for example when a new transition happens and if so, is it only if a new transition happens in the same component or an ancestor, or is it app-wide?

I've seen discussions around the starvation problem now and then (can't remember where) so I'm curious about if you have ended up with a specific approach in React 18 or if this is yet undecided? It struck me that since interrupted work is no longer reused, maybe this has the potential for happening more often now?

The heuristics is an internal detail that can be tweaked of course, so I'm mostly interested if this is something I'll need to ever care about as a developer?

Perhaps more concretely:

What happens if I call startTransition in a requestAnimationFrame and it never has time to finish?

joshribakoff-sm commented 2 years ago

I have 3 transitions defined in the same component:

import { startTransition } from 'react';
const [isAutocompletePending, startAutocompleteTransition] = useTransition();
const [isApplyPending, startApplyTransition] = useTransition();

By wrapping things in transitions, I expected my app to become faster (generally speaking), however it actually becomes slower, it won't even start a network request in a 2nd transition whenever an unrelated network request in a 1st transition is pending. These network requests did not depend on each other at all, ironically, until we opt in to "concurrency".

If it makes a difference, all these transitions set some React state that is being passed to react-query to trigger a network call "fetch on render" style. I guess that the issue is that React cannot yet prepare two different versions of the "universe" for this suspended component simultaneously. I suspect that doing my transitions within separate suspense boundaries may be the key to solving this.

gaearon commented 2 years ago

Can I ask you to prepare a repro so we can have a closer look? There are likely some limitations but we may be able to tweak the heuristics if there is a clear demo of the problem.

all these transitions set some React state that is being passed to react-query to trigger a network call "fetch on render" style

If you create a repro please don’t use React Query or similar libraries since we can’t easily see what they’re doing. Ideally for repro cases you would use the experimental react-fetch or a hand-rolled Promise throwing cache. Thanks!

joshribakoff-sm commented 2 years ago

Hey Dan, thanks for the reply. In my "real app", I got rid of the multiple transitions and replaced it with a single useDeferredValue at the top of my app to fix my issue.

I've tried to create a demo of the transition starvation issue, however I'm having trouble even getting a single transition to work as expected. I have tried a bunch of things, such as awaiting the fake delay inside the start(()=>{ ... }), or setting state that runs an effect that has delay, and I have tried moving that effect both above and below the suspense boundary. In all 3 cases, I never see my "pending" rendered on the screen:

function App() {
  const [state, setState] = useState();
  const [isPending, start] = useTransition();

  useEffect(() => {
   // I guess this is not part of the transition since it's a separate microtask?
    (async () => {
      console.log("before delay");
      await new Promise((r) => setTimeout(r, 5000));
      console.log("after delay");
    })();
  }, [state]);

  console.log(isPending);
  return (
    <>
      <Child {...{ setState, start }} />
      <br />
      {isPending ? "pending" : "not pending"}
      <br />
      <Suspense fallback={<h1>loading</h1>}>{state}</Suspense>
    </>
  );
}

function Child({ setState, start, state }) {
  return (
    <>
      <button
        onClick={() =>
          start(async () => {
      console.log("before delay");
      await new Promise((r) => setTimeout(r, 5000));
      console.log("after delay");
            console.log("before setting state");
            setState(Math.random());
            console.log("after setting state");
          })
        }
      >
        start
      </button>
    </>
  );
}

Because I'm stumbling to get even a basic example of a transition even working, I don't have a clear path forward to build you an example. Keep in mind part of the "transition starvation" issue I'm seeing may be specific to "fetch on render", and React team has not provided any examples of this pattern working with transitions (from what I'm aware of). One sentiment that I have is that it will be hard for the community to give reliable feedback about (or even learn) these features if it is hard for us to know whether we're using the features as intended or not.

Sorry in advance if I'm just doing something stupid here, or letting my confusion get in the way of communicating this clearly. Perhaps once the React team has examples of "fetch on render", it will be easier for me to modify to show the issue?

gaearon commented 2 years ago

Right, it doesn't wait for async/await or useEffect or similar things. The low-level mechanism for integrating with Suspense is throwing a Promise, but I'd suggest using an experimental react-fetch package for easy testing.

Here is an example you can build on top of: https://codesandbox.io/s/vibrant-kare-p2s2qi?file=/src/App.js

joshribakoff-sm commented 2 years ago

Hey Dan, I think I managed to replicate the issue I was describing in my "real app". I have here an example using the experimental react cache. There are two unrelated states, and two unrelated transitions (from my perspective).

When I start transition 1, and then wait 2-3 seconds and start transition 2, I see them both switch from "pending" to their completed state simultaneously, but actually what I expected was to first see the first transition complete, and then see the second transition complete 2-3 seconds later.

I don't think it's necessarily "wrong", but I do think it is an area where it would be great to get more clarity surrounding the expected semantics:

import { useTransition, Suspense, useState, useEffect } from "react";
import react from "react";

var Pending = 0;
var Resolved = 1;
var Rejected = 2;

function createRecordFromThenable(thenable) {
  var record = {
    status: Pending,
    value: thenable,
  };
  thenable.then(
    function (value) {
      if (record.status === Pending) {
        var resolvedRecord = record;
        resolvedRecord.status = Resolved;
        resolvedRecord.value = value;
      }
    },
    function (err) {
      if (record.status === Pending) {
        var rejectedRecord = record;
        rejectedRecord.status = Rejected;
        rejectedRecord.value = err;
      }
    }
  );
  return record;
}

function createRecordMap() {
  return new Map();
}

function getRecordMap() {
  return react.unstable_getCacheForType(createRecordMap);
}

function preloadRecord(url, options) {
  var map = getRecordMap();
  var record = map.get(url);
  if (!record) {
    var thenable = new Promise((res) => setTimeout(() => res(url), 5000));
    record = createRecordFromThenable(thenable);
    map.set(url, record);
  }
  return record;
}

function readRecordValue(record) {
  if (record.status === Resolved) {
    return record.value;
  } else {
    throw record.value;
  }
}

function App() {
  const [url, setUrl] = useState();
  const [url2, setUrl2] = useState();
  const [isPending, start] = useTransition();
  const [isPending2, start2] = useTransition();
  return (
    <>
      <br />
      <button
        onClick={() => {
          start(() => {
            setUrl(Math.random());
          });
        }}
      >
        start
      </button>
      <br />
      <button
        onClick={() => {
          start2(() => {
            setUrl2(Math.random());
          });
        }}
      >
        start2
      </button>
      <br />
      {isPending ? "pending" : "not pending"}
      <br />
      {isPending2 ? "pending2" : "not pending2"}
      <br />
      <Suspense fallback={<h1>loading</h1>}>
        val1: {url && readRecordValue(preloadRecord(url))}
      </Suspense>
      <br />
      <Suspense fallback={<h1>loading2</h1>}>
        {/* it doesn't even matter if this one suspends, it still becomes entangled with the first transition */ }
        {/* val2: {url2 && readRecordValue(preloadRecord(url2))} */}
        url2: {url2}
      </Suspense>
    </>
  );
}

export default App;
gaearon commented 2 years ago

Thanks, this is very helpful! We'll take a look.

joshribakoff-sm commented 2 years ago

Thanks!

Just a quick update I confirmed this appears to still happen even if the transitions are in totally separate components, with totally separate suspense boundaries. This makes me think this has to change to make concurrent React useful to users [beyond using it in a single place in the whole app?].

For example I have some charts, and also a search with an autocomplete.

I am using transitions on both of these so that I can still type in the search when the search's autocomplete is rendering, and I can still interact with my "stale" charts while I'm fetching data or rendering a new chart (and in general, the main thread is not blocked while I render my charts).

However, I unfortunately cannot use my search at all while I am fetching data for my charts. So the impact to us as a consumer of React is that our search is always blocked whenever the chart data is being fetched (which takes a long time).

Right now our app is unreleased, so it is okay, but this will be a total blocker for us in releasing our app with concurrent features enabled (at least in more than one place) 😢 .

The charts and the search are unrelated, decoupled features, and in previous versions of React we could have parallel network requests. When doing a "CPU Demo", React 18 is for sure more concurrent than React 17, but when doing a "network demo" we're less concurrent than we were in React 17. Thanks in advance for looking into it 🙏

gaearon commented 2 years ago

It takes some time to shake out the heuristics and bugs, and since we've only used these features in a limited way (refetching with Relay) it's likely we're going to be running into limitations that need to be fixed. In that sense, you can think of 18.0.0 as the MVP on top of which we're working now. Some will be easy fixes, some may be more more complex. As long as we have good reproducing cases, we can make progress. (Keep in mind you don't have to use startTransition if it doesn't yet cover your use case well — we don't even have an officially recommended data fetching setup for it yet!)

However, I unfortunately cannot use my search at all while I am fetching data for my charts.

This part sounds a bit strange to me though. (Even assuming the behavior that currently exists.) Urgent renders for the input should be able to happen outside of transitions, so why doesn't it work? Can you make a demo of this as well?

joshribakoff-sm commented 2 years ago

To clarify, typing in my search's input works, but it's similar to a MUI autocomplete https://mui.com/components/autocomplete/ and rendering the suggestions / results is blocked (which I'm doing in a transition).

We'll need to not only remove the transition from our search, but also remove suspense completely, otherwise we get a flicker without the transition. We'll just use it on our charts in the interim.

Please let me know if I can help out with testing any unreleased fixes ❤️

gaearon commented 2 years ago

Checked with the team. All transitions get "entangled" in the current implementation. There is a planned project to solve this, but there are more urgent things to do post-release so we expect to get back to it not earlier than H2 of this year. Obviously, it will become more pressing as concurrent features start getting adoption.

joshribakoff-sm commented 2 years ago

Thanks for confirming, it would be good to document these semantics sooner than H2. Let me know if I can help with contributing anything here like a PR or helping with docs. Your feedback has been very helpful in vindicating our approach to use just on our charts, since they are the one slow thing we have [from a network and CPU perspective]. I think another workaround if we really wanted an escape hatch would be to just mount two React roots, I'd be surprised if they were still entangled after that :)

joshribakoff commented 2 years ago

One other use case we want to call out is consider that we start a transition to fetch data and render our chart (useTransition), and it takes a long time due to the network. Now while that transition is pending (due to network) the user resizes their browser and the resize event triggers the chart to update with new state (new width and height), but with stale data (React.startTransition). Ideally we think these transitions are also decoupled :)

in other words we would presumably expect the chart to rerender even though there is already a previous transition pending for the chart.

Perhaps one heuristic is that transitions that don’t suspend ought to be decoupled from transitions that suspend. Or React.startTransition could take precedent. Not sure if it works this way now, but it’s another case for the team to consider if there’s not already explicit test cases for this :)

Also consider the resize events could fire so often the chart never updates (if each resize event interrupts the work from the previous resize event), as opposed to a “throttle” with a hard coded delay which may block the main thread “sometimes” but guarantees the charts won’t lag too far behind. There is a staleness vs responsiveness tradeoff, and there is potentially a use case to be able to configure this

gaearon commented 2 years ago

I know it's a tough ask but minimal sandboxes are always very helpful for these types of issues.

christian-lechner commented 2 years ago

Hi everyone,

I finally found an explanation for the strange behavior of useTransition() I encountered, here in this issue. The UI does not update until all running transitions are complete, even if they are in their own suspense block.

All transitions get "entangled" in the current implementation. -- @gaearon

This explains a lot, thanks @gaearon.

To illustrate this problem, I have put together a small example: https://codesandbox.io/s/react-suspense-transition-o2sm9d?file=/src/App.js

The initial loading works as expected, the contents of the four boxes appear at different (random) times, but if you click the buttons one after the other while the others are still pending, the UI update of all four boxes is delayed until all transitions are complete.

It would be nice to see this behavior changed as soon as possible.

Best regards!

manelio commented 2 years ago

I know it's a tough ask but minimal sandboxes are always very helpful for these types of issues.

I just found this question and I think is related to another one I asked yesterday in SO here

This is the relevant part:

Briefly: I'm building a multipage app with React 18, react-location and react-query. Each route requires retrieving information from an API. Some requests could take too long for the users, who should be able to navigate to other page if they want.

I crated a CodeSandbox to replicate the situation. There are 3 routes. Requests for routes /0 and /1 are instant. Query for route /2 takes 5 seconds:

https://codesandbox.io/s/busy-vaughan-20lcfz?file=/src/App.js

Click sequentially the links for routes 0 (instant), 2 (slow) and 1 (instant).

Click on page 0: page 0 is loaded instantly

Click on page 2: since navigate method (from react-location) is called from startTransition and query uses suspense, nothing changes until page 2 query ends (3 seconds). That's correct and expected (in real app there's a global activity indicator).

But the problem comes here. If the user decides not to wait until request ends and clicks on page 1, the expected behaviour is to instantly navigate to page 1 and abandon or cancel whatever ongoing activity. But the current behaviour is the user has to wait until page 2 request ends.

joshribakoff-sm commented 2 years ago

There would probably at least be a better experience for consumers of React if the method were renamed back to _unstable_startTransition or startBLOCKINGtransition or something... Or if it were documented...

I understand its hard to change the behavior, but right now the behavior is misleading users into making their apps worse.

joshribakoff-sm commented 2 months ago

FYI I'm running into this again, where I'm using startTransition() when a user types to update some state. This state update triggers expensive rendering that otherwise blocks typing. The app was working fine until I changed something subtle (likely triggering network requests inside the transition, I think) and now a bunch of "unrelated" e2e tests fail (one of these tests unmounts the component after the user types into it, and remounts it, and now fails because the state update was not applied). If I simply remove the startTransition(), the tests pass (but that causes a performance regression)

It would be tedious to change all consumers to defer, but that's likely the work around I'll try. Curious to hear if there's any update. Seems like startTransition() is a footgun.

clemp6r commented 3 weeks ago

Is there any plans to fix the transition entangling issue?

I'm running a similar issue with a Next.js app where multiple transitions can occur on the same page. A slow transition starting before a fast one will keep the fast one in loading state until the slow one is finished.