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

Concurrent rendering problem in React 18 #177

Closed
dhilt opened this issue Mar 1, 2023 · 4 comments · Fixed by #180
Closed

Concurrent rendering problem in React 18 #177

dhilt opened this issue Mar 1, 2023 · 4 comments · Fixed by #180

Comments

@dhilt
Copy link

dhilt commented Mar 1, 2023

I forked minimal example on codesandbox and sligthly changed it: https://codesandbox.io/s/damp-sun-0bhfb1?file=/src/index.js. My goal is to make sure that the lib does prevent re-renders, so I separated render counters for TextBox and Counter components. I would expect their counters to increment independently as I change different parts of the state. But they increase, as if it were a single counter. Also, console.log shows that both componentes are rendered every time the state changes. Am I doing it wrong?

Repro:

  • run https://codesandbox.io/s/damp-sun-0bhfb1?file=/src/index.js
  • see numRendered1: 2 and numRendered2: 2 (not sure why they are doubled, but let's skip it)
  • update Counter by clicking "+1" 3 times
  • see numRendered1: 10 and numRendered2: 2 (the 1st counter should be 8, and this is also because they depend on each other)
  • update TextBox by adding 1 symbol to "hello" (like "hello+")

Expectation:

  • see numRendered1: 10 and numRendered2: 4 (I would say it should be 8/4, but let's pay attention to 4-12 difference)

Result:

  • numRendered1: 10 and numRendered2: 12
@dai-shi
Copy link
Owner

dai-shi commented Mar 2, 2023

Does it work as expected if you update numRendered in useEffect or use React 17 instead of React 18?

If that's the case, it's a behavioral change in React 18 useReducer, which is somewhat reasonable. It's how it works now to be full compatible for concurrent rendering. If it causes serious performance problems, like heavy computation, we need to consider wrapping the heavy computation with useMemo.

ref: https://twitter.com/dai_shi/status/1534170089981100032

@dhilt
Copy link
Author

dhilt commented Mar 2, 2023

@dai-shi You are right! When I switched demo to React v17 (link), it started working as expected. Do you plan to support React v18? otherwise, as I guess, using react-tracking library becomes redundant for R18 users, and useMemo solves the whole problem (in a very awkward way, of course)

@dai-shi
Copy link
Owner

dai-shi commented Mar 2, 2023

This is really not solvable unless we give up concurrency support.
ref: https://github.com/dai-shi/will-this-react-global-state-work-in-concurrent-rendering

If React provides native useContextSelector, react-tracked can migrate to it and everything will work nicely.

So, what is your real issue? Do you see any performance drop? Double rendering is an expected outcome as of now.

@dhilt
Copy link
Author

dhilt commented Mar 3, 2023

@dai-shi Thanks for answering. I really didn't dig much into this, I just see extra renders in a component that doesn't depend on a sub-context that was affected by some outer update, and I don't like it. I see that useMemo (and other techniques like splitting/wrapping you mentioned here) can fix it, while react-tracked can't (in React 18 App). Maybe these extra renders don't matter with react-tracked, I haven't done any performance testing, but they obviousely happen at the App level. I think there should be a special note regarding React 18 in the lib documnetation, as it really confuses.

@dhilt dhilt changed the title Understanding codesandbox sample Concurrent rendering problem in React 18 Mar 3, 2023
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

Successfully merging a pull request may close this issue.

2 participants