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

Performance of useSyncExternalStore and React transitions #2086

Open
1 task
OliverJAsh opened this issue Nov 16, 2023 · 5 comments
Open
1 task

Performance of useSyncExternalStore and React transitions #2086

OliverJAsh opened this issue Nov 16, 2023 · 5 comments

Comments

@OliverJAsh
Copy link
Contributor

OliverJAsh commented Nov 16, 2023

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

  • React: 18.2.0 (latest at time of writing)
  • Redux: 4.2.1 (latest at time of writing)
  • React Redux: 8.1.3 (latest at time of writing)

What is the current behavior?

Reduced test case: https://github.com/OliverJAsh/react-redux-useSyncExternalStore-perf

Despite wrapping calls to dispatch with startTransition, React will always render Redux state updates synchronously. This is in contrast to regular React state updates.

I suspect this is due to the behaviour of useSyncExternalStore. However I was wondering if you're aware of any plans for this to be addressed, either in React Redux somehow or React itself?

As it stands I think this is a significant performance issue. For example, here's a performance recording from Unsplash (the application I work on). We're able to benefit from concurrent rendering and "time slicing" for hydration, but then once hydration has finished we need to run some state updates through Redux, and this results in this very long task (hovered):

image

It would be great if we could benefit from transitions / concurrent rendering / time slicing.

In our case at Unsplash, if we could figure out a way to make these states updates non-blocking then we could significantly improve our page load performance and metrics such as TBT, TTI, and ITP.

If it helps, here's a demo of my reduced test case, comparing state updates for React vs Redux (both wrapped in startTransition). Each state update renders 10 components, each taking 100ms.

283576776-7417d43a-0e3a-4193-b93a-ed10a6573277.2.mov

What is the expected behavior?

N/A

Which browser and OS are affected by this issue?

No response

Did this work in previous versions of React Redux?

  • Yes
@phryneas
Copy link
Member

phryneas commented Nov 16, 2023

It is pretty much the React team's decision to deal with all external data sources like this - they created useSyncExternalStore for exactly the purpose of keeping "non-React" updates synchronous (to avoid tearing if a non-React datasource updates during a render), and there is no other way (or only less performant ways) we could implement this without useSyncExternalStore.

Of course, there is also no efficient way of implementing anything like this with only React means - Context is not a solution here :/

@OliverJAsh
Copy link
Contributor Author

That's what I thought. Do you know if there's been any further discussion in the React team about this? I understand the constraints but this seems like a pretty fundamental flaw to me.

@markerikson
Copy link
Contributor

Purely off the top of my head, this is basically the React team's intent. The only way React can fully know that a state update is interruptible / low-pri is if it's built-in React state, in which case React controls when the render pass happens and when that queued state update gets applied. React has no control over any external store, and those update synchronously, so there's no way to tie together a future pending React update with the external store somehow applying the change to itself later when React is ready to render. So, React is forced to treat all external state updates and renders as synchronous and blocking too.

@JAAAAAEMKIM
Copy link

I'm suffering from the same issue too. This sync work blocks rendering for so much ms. Are there at least any recommended workaround? 👀
What I'm trying is using useDeferredValue to defer from original state whenever I need transitions.

@OliverJAsh
Copy link
Contributor Author

In our case we were able to workaround the problem by moving the state out of Redux and into context. This does make me question the long term usage of Redux state in React. 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants
@OliverJAsh @markerikson @phryneas @JAAAAAEMKIM and others