retejs / rete

JavaScript framework for visual programming
https://retejs.org
MIT License
10.04k stars 652 forks source link

Warning spam: flushSync was called from inside a lifecycle method. React cannot flush when React is already rendering #669

Closed iXab3r closed 11 months ago

iXab3r commented 11 months ago

🕵️‍♂️ Problem Description

When a user interacts with connections (e.g., lifting up an existing connection), the browser console displays repetitive warning messages. While these warnings don't break any functionality, they can flood the console, especially during batch operations.

🌍 Environment

🐾 Steps to Reproduce

  1. Visit any React-related example:
  2. Interact with an input or output port by clicking on it.

    Note: You don't need to reestablish the connection; just lifting it is enough.

  3. Observe the console for the following warning:

Warning: flushSync was called from inside a lifecycle method. React cannot flush when React is already rendering. Consider moving this call to a scheduler task or micro task.
    at ConnectionWrapper (https://yrzxe5.csb.app/node_modules/rete-react-plugin/rete-react-plugin.esm.js:176:24)
    at Root (https://yrzxe5.csb.app/node_modules/rete-react-plugin/rete-react-plugin.esm.js:109:23) 
    in ConnectionWrapper
Ni55aN commented 11 months ago

https://github.com/retejs/react-plugin/issues/38#issuecomment-1553225290

iXab3r commented 11 months ago

Understood, thanks for the quick reply 😄

TL;DR; Known issue, no working solution yet

Ni55aN commented 11 months ago

Known issue, no working solution yet

not even issue :) React.js doesn't have an option to disable this warning, so in this particular case it's not an issue

Ni55aN commented 11 months ago

btw, thanks for raising this topic.

So, if we wouldn't have flushSync, we can see this (if there are a lot of nodes e.i. JS uses CPU close to 100%.):

http://127.0.0.1:5173/?template=perf&rows=100

perf1

I just got some time and checked possible solutions

  1. flushSync is implemented in react-reconciler, but it is bundled (not peer dependency) into react-dom package and isn't exposed ❌
  2. the warning recommends using microtask ✔️
queueMicrotask(() => flushSync(f))

Luckly, it doesn't cause a gap between node and connections when flushSync is called as a separate microtask

perf12

Ni55aN commented 11 months ago

Released in v2.0.4

iXab3r commented 11 months ago

Thanks a lot for spending your time on fixing this! :)

Just for context, that is how browser console looked like :D rider64_Srp1FiegNb

Necmttn commented 11 months ago

Thank you ❤️