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: useTransition and uSES do not work together, uT is not resilient to amount-of-render #26814

Open
drcmda opened this issue May 14, 2023 · 12 comments
Assignees
Labels
Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug

Comments

@drcmda
Copy link

drcmda commented May 14, 2023

React version: 18.2.0

Steps To Reproduce

We learn that components should not rely on render. But useTransition does seem to rely on it. If we wrap a set into a useTransition:

const [pending, start] = useTransition()
...
start(() => set("foo"))

everything that now suspends because of it is marked as a transition, it will not go into suspense fallback but instead pending will be true. ✅

But if the suspending component, for whatever reason, happens to render again while still in suspense, giving react the same promise that it already has, then it will unpend because that render wasn't marked a transition. ❌

This at least happens when state managers are built on uSES.

Link to code example:

https://codesandbox.io/s/cool-fast-pcz6bv?file=/src/App.js

The current behavior

1. ❌ Fallback, loading...
2. ⚛️ Content
3. ❌ Fallback, loading...
4. ⚛️ Content

In that sandbox it:

  1. suspends, it correctly goes into suspense fallback
  2. unsuspends, it correctly shows the content
  3. suspends again but wrapped into startTransition, it should not go into fallback but pend, but because the suspending component double renders for whatever reason it throws it's promise again towards react, but nothing marked it a transition this time, so the pending state disappears and it goes back into fallback

The expected behavior

1. ❌ Fallback, loading...
2. ⚛️ Content
3. ⚛️ Content
   ✅ Pending ...
4. ⚛️ Content

startTransition should mark the component that suspended a transition as long as it suspends.

btw, @dai-shi has an extension that lets us use zustand without uSES, it does not happen then: https://codesandbox.io/s/suspicious-hooks-hdvxhl?file=/src/App.js

@drcmda drcmda added the Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug label May 14, 2023
@dai-shi
Copy link
Contributor

dai-shi commented May 14, 2023

I'm not sure if it's described well in react-18 WG discussions (I think there was a tweet about it from Seb.), but the "Sync" is important. It's not useExternalStore. So, it's not a bug per se. It's a designed limitation.

@alexeyraspopov
Copy link
Contributor

alexeyraspopov commented Jun 19, 2023

I hit the same behavior and it was far from obvious in my case, because uSES wasn't the one that supposed to be triggering suspense. I created this sandbox https://codesandbox.io/s/upbeat-flower-2gnhm6 to show that the shim implementation (use-sync-external-store itself, before it got rerouted to the React hook implementation) didn't have this behavior and was working in the way I as a user would expect: when suspending render is triggered via useTransition, I can use pending flag instead of suspense boundary for UI feedback.

Screen.Recording.2023-06-19.at.11.37.48.mov

@mattcarrollcode mattcarrollcode self-assigned this Dec 7, 2023
@markerikson
Copy link
Contributor

I'll tag in a related React-Redux issue:

Copy link

github-actions bot commented Apr 9, 2024

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

@github-actions github-actions bot added the Resolution: Stale Automatically closed due to inactivity label Apr 9, 2024
@alexeyraspopov
Copy link
Contributor

Bump. There was no patch for it and we are yet to discover if v19 fixes the issue.

@github-actions github-actions bot removed the Resolution: Stale Automatically closed due to inactivity label Apr 10, 2024
@alexeyraspopov
Copy link
Contributor

I can confirm that this bug is still present in React 19.0.0-beta-94eed63c49-20240425. I created a repo for reproduction (since my old codesandbox disappeared): https://github.com/alexeyraspopov/react-beta-bugs. Here's the video as well. Disregard the first text being always gray, there's another bug I'll report separately. Should I also create a separate ticket for v19 behavior? @eps1lon

Screen.Recording.2024-04-26.at.08.16.37.mov

Copy link

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

@github-actions github-actions bot added the Resolution: Stale Automatically closed due to inactivity label Jul 25, 2024
@markerikson
Copy link
Contributor

bump

@github-actions github-actions bot removed the Resolution: Stale Automatically closed due to inactivity label Jul 25, 2024
@bastiankistner
Copy link

I'm using @tanstack/router for one of my apps. I just started integrating it and discovered an odd behaviour when I was using the browser's navigation buttons. Every time I go forth or back, the whole router suspends and shows the loading/pending component that can be set. If I don't set it, the whole screen just disappears due to some parts in the react-dom package that set display: none !important. I tried to figure out the reason for it, because this does not happen with react 18.

Then I found this line here : https://github.com/TanStack/router/blob/7d6ebcf5f01febd961ae1bd854205dacacc5938e/packages/react-router/src/Transitioner.tsx#L28

When we're not on the server, tanstack router uses React's useTransition() hook and that seems to lead to a very similar behaviour as @alexeyraspopov shows in the video in #26814 (comment).

It's kinda easy to reproduce. Just checkout one of the @tanstack/router examples (e.g. this one https://github.com/TanStack/router/tree/main/examples/react/kitchen-sink-file-based) and instead of react 18.x use the rc version. Try both when navigating with the browser history / navigation buttons and you'll see that each time the history is being changed, the whole app will disappear.

@motionrus
Copy link

motionrus commented Aug 8, 2024

I got around this problem by wrapping the transition state in a hook, I couldn't find any examples of how to get around this, maybe it's worth recording this somewhere in the documentation

const useTransitionState = () => {
  // useEvent is created store in zustand
  const contentType = useEvent.use.contentType();
  const [type, setType] = useState<'simple' | 'template' | undefined>(contentType);
  const setContentType = useEvent.use.setContentType();

  useEffect(() => {
    if (type !== contentType) {
      setType(contentType);
    }
  }, [contentType]);

  useEffect(() => {
    if (type !== contentType) {
      setContentType(type);
    }
  }, [type]);

  return { type, setType };
};

the code is not clean, my idea is to subscribe to the state change that we store in the store and wrap it in state

if there are any ideas on how to get around this, please write, I will be grateful

Copy link

github-actions bot commented Nov 6, 2024

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

@github-actions github-actions bot added the Resolution: Stale Automatically closed due to inactivity label Nov 6, 2024
@alexeyraspopov
Copy link
Contributor

bump

@github-actions github-actions bot removed the Resolution: Stale Automatically closed due to inactivity label Nov 6, 2024
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

7 participants