Open bvaughn opened 2 years ago
As a side note, given that React heuristics like this may change between releases (as happened with "click" event behavior) we may need to rethink the source of Timeline warnings. Maybe these warnings belong inside of React itself (DEV and profiling builds) and should be communicated to DevTools using some standardized format?
Bit of an update on this:
I created a Code Sandbox to test the current behavior in alpha: https://codesandbox.io/s/false-positive-long-nested-update-warning-x5zxg?file=/src/App.js:613-618
https://user-images.githubusercontent.com/29597/141007628-1fd47e92-c0c5-4052-8a77-070348128ca7.mp4
Both layout and passive effects are processed synchronously, but while layout effect updates are flushed synchronously– passive effect updates are scheduled with "default" priority and flush after paint.
So the 1st scenario described in the issue above is possible, but the 2nd scenario was a misunderstanding between Dan and I.
Here is a code sandbox that does show a false warning though: https://b75v7.csb.app/
And here is the JSON profile: Profile-20211109T163441.txt
Not sure exactly what's going on here, but we shouldn't be warning about this case since the update isn't actually being processed synchronously. (It's not blocking paint.)
@bvaughn Could you confirm this is still an issue?
Timeline shows the following nested update warning when a synchronous update causes an event handler to run long:
This warning was intended to encourage developers to move heavy updates from layout effects into passive effects so that they did not block paint or stretch event handlers.
Unfortunately this warning currently does not handle a few cases well:
Click events now always flush passive effects synchronously(see https://github.com/facebook/react/issues/20074#issuecomment-811296789).This means that the current warning may be confusing or misleading. We should either update it to ensure that it never fires for updates that were already scheduled inside of a passive effect, or if that is not possible we should remove it entirely.