reduxjs / react-redux

Official React bindings for Redux
https://react-redux.js.org
MIT License
23.37k stars 3.37k forks source link

Suspense boundary received an update before it finished hydrating #1962

Closed OliverJAsh closed 2 years ago

OliverJAsh commented 2 years ago

What version of React, ReactDOM/React Native, Redux, and React Redux are you using?

What is the current behavior?

Full reduced test case: https://github.com/OliverJAsh/react-redux-suspense-issue

The main code can be found here: https://github.com/OliverJAsh/react-redux-suspense-issue/blob/7540831b4afb8e3811afaad489bd2e456e80e4b1/src/App.js

Replay: https://app.replay.io/recording/untitled--d46adaf3-b286-4dfa-a1a7-ebe5cadd3568

Follow the steps in the README to build the server and client and run the server, then navigate to http://localhost:8080.

In the console we can see this error:

This Suspense boundary received an update before it finished hydrating. This caused the boundary to switch to client rendering. The usual way to fix this is to wrap the original update in startTransition.
image

What is the expected behavior?

There should be no error.

The dispatch call is wrapped in startTransition, as suggested by the error message, but that doesn't seem to help.

I searched existing issues to see if anyone else has reported this error and all I could find was this: https://github.com/reduxjs/react-redux/issues/1797#issuecomment-1163042309

I did also find this issue in the React repository which appears to be describing something very similar. However, the suggested fix to use startTransition doesn't help in this case, so I think there might be more to this.

I was able to workaround the error by wrapping the Suspense boundary with React.memo, but this seems like a hacky workaround rather than a real solution:

diff --git a/src/App.js b/src/App.js
index 2032f1a..e5ab605 100644
--- a/src/App.js
+++ b/src/App.js
@@ -17,13 +17,17 @@ const reducer = (state = initialState, action) => {
 };
 const store = Redux.createStore(reducer);

+const SuspenseBoundaryMemoized = React.memo(() => (
+    <Suspense fallback={<div>Loading…</div>}>
+        <div>Loaded!</div>
+    </Suspense>
+));
+
 const _Inner = ({ count }) => (
     <>
         <div>Count: {count}</div>

-        <Suspense fallback={<div>Loading…</div>}>
-            <div>Loaded!</div>
-        </Suspense>
+        <SuspenseBoundaryMemoized />
     </>
 );
 const Inner = ReactRedux.connect((state) => ({ count: state.count }))(_Inner);

For context also, this is an issue I noticed whilst trying to adopt Suspense at Unsplash.

Which browser and OS are affected by this issue?

No response

Did this work in previous versions of React Redux?

markerikson commented 2 years ago

Hmm. Unfortunately I don't know enough about Suspense or transition behavior to have any idea if this is a bug or something that's expected :(

Don't suppose @Andarist might have a clue?

Andarist commented 2 years ago

The linked replay is private so I can't take a look. However, based on the provided description - I would say that it would be best to first file an issue in the React repo. If there is a suggestion in the warning message about adding startTransition and that doesn't help then it's likely a problem in React (or in how that startTransition gets called).

You could also try to experiment with startTransition and uSES. I'm not sure how those two are supposed to interact. Perhaps the problem is in this combination. If I understand correctly, uSES flushes the updates synchronously (not immediately but in the same sync frame) and startTransition might want to delay the update. This might create a conflict between those two - but I have never actually used startTransition and its semantics are a little bit vague to me. IIRC you can, sort of, time-slice uSES updates if you use useDeferredValue appropriately. But I only recall Sebastian Markbage's comment about this - I have never actually utilized this technique. It might be worth looking into the working group threads to learn more about this.

OliverJAsh commented 2 years ago

@Andarist I've made the Replay public, sorry about that.

I will follow your advice and post an issue in the React repo. Thanks!

OliverJAsh commented 2 years ago

This seems to be the same issue as https://github.com/facebook/react/issues/24810. I reduced my example further to remove react-redux—now I'm just using useSyncExternalStore on its own: https://github.com/OliverJAsh/react-redux-suspense-issue/tree/rm-react-redux.