-
Notifications
You must be signed in to change notification settings - Fork 47.7k
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
flush only on exiting outermost act() #15682
Conversation
ReactDOM: size: 0.0%, gzip: 0.0% Details of bundled changes.Comparing: 9c6de71...94d7ffe react-dom
react-test-renderer
react-noop-renderer
Generated by 🚫 dangerJS |
Thanks! That makes it much easier for me to review, too. (Personally I find rebasing on master makes it easier to maintain stacked PRs, compared to merging, but if you prefer merging keep doing that.) |
@@ -171,7 +177,11 @@ function act(callback: () => Thenable) { | |||
|
|||
// flush effects until none remain, and cleanup | |||
try { | |||
flushWork(); | |||
if (actingUpdatesScopeDepth === 1) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit:
if (actingUpdatesScopeDepth === 1) { | |
if (actingUpdatesScopeDepth <= 1) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
really? it shouldn't do that if it's 0 (and it ideally should never be 0 here). going to push back on this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tbh, it should probably throw an error if it ever gets there
When calls to act are nested, effects and component updates are only flushed when the outer `act` call returns, as per [1] and [2]. This is convenient for creating helper functions which may invoke `act` themselves. [1] facebook/react#15682 [2] facebook/react#15472
When calls to act are nested, effects and component updates are only flushed when the outer `act` call returns, as per [1] and [2]. This is convenient for creating helper functions which may invoke `act` themselves. [1] facebook/react#15682 [2] facebook/react#15472
^ like it says. we flush only on exiting the outermost act().
my previous PR (#15519) ended up in merge conflict hell, so I just redid it.