TanStack / router

🤖 Fully typesafe Router for React (and friends) w/ built-in caching, 1st class search-param APIs, client-side cache integration and isomorphic rendering.
https://tanstack.com/router
MIT License
7.77k stars 565 forks source link

browser history changes lead to suspended router in react 19 #2082

Open bastiankistner opened 1 month ago

bastiankistner commented 1 month ago

Describe the bug

Apologies for coming up with an issue that I discovered in React 19 (rc). But after digging into the react/react-dom codebase and debugging tanstack router, I'm not fully convinced that this is actually an issue with react but rather something that "still" works but will break anyways once React 19 is no longer a release candidate.

When I use the current rc versions with tanstack router, any change to the browser's history, e.g. through clicking the forward/back buttons, lead to a flash of content. I figured that this is related to the router becoming suspended whilte a transition is being executed.

However, this bug does not exist, when I click on the links and navigate through the app directly

At first I thought this might anyways not be considered due to a non final release of react. But when I checked the router's codebase, a few things looked suspicious and I got the impression that the useTransition integration might still be a bit buggy.

  1. the isTransitioning property from the router state here is never being used.

  2. isTransitioning from the React.useTransition() hook is only true when I click the links manually, not when I navigate with the browser controls (see here)

  3. unsetting the reassignment of startReactTransition in the router easily fixes the problem (see here) (but that would probably also lead to a case where a click on a link might behave unintentional

  4. according to react's documentation, the actual transition should never be async but it is being passed an async function here

I couldn't figure out how to fix this properly. But at least I was able to find a way to not break too much (I hope) and leave the changes to the code that targets browser history changes:

In the Transitioner in line 34, instead of just passing router.load to the subscribe function, we can either set the router.startReactTransition back to (fn) => fn()) and reset it back again to the hook's startTransition_ function right after, or we could use the startTransition function from react, which should at least inform react about the update. Although I'm not sure if that helps in any way.

Here's how I have currently changed it to get it working:

import { startTransition } from 'react';

// ...

React.useEffect(() => {
  const unsub = router.history.subscribe(() => {
        router.startReactTransition = startTransition;
        router.load();
        router.startReactTransition = startReactTransition_;
      });
// ....
});

At least this way the browser navigation is working as expected. I'll keep this with a patch for now and will report if I encounter any unforeseen problems.

I'm also fine if you close this issue or leave it uncommented. I just wanted to point out that this seems to be something that won't just go away when react 19 will be released officially. And maybe this helps you a little to fix it.

I also found an open issue in the react repo that shows a similar behaviour: https://github.com/facebook/react/issues/26814#issuecomment-2079275889 (I added a comment).

Your Example Website or App

https://stackblitz.com/edit/github-sekuqn?file=package.json

Steps to Reproduce the Bug or Issue

  1. Run any current example
  2. click links and navigate with the browser controls (in stackblitz cmd+← and cmd+→ will work fine)

-> You won't see any content flashes

  1. change react and react-dom version to rc
  2. re-run the app
  3. click links, works fine
  4. navigate with browser controls -> content flash because the whole router gets suspended on each history event

Hint: you can also add the following line to your createRouter call to make it more obvious what's happening:

defaultPendingComponent: () => <div>loading...</div>,

Expected behavior

As a user, I expected browser navigation to behave in the same way as manual link clicks, but instead the whole app disappears when navigating forth and back.

Screenshots or Videos

No response

Platform

Additional context

No response

damianstasik commented 1 month ago

according to react's documentation, the actual transition should never be async but it is being passed an async function here

With React 19, you can pass async functions to startTransition in both its standalone form and when used with useTransition.