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

flush only on exiting outermost act() #15682

Merged
merged 1 commit into from
May 21, 2019

Conversation

threepointone
Copy link
Contributor

^ 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.

@sizebot
Copy link

sizebot commented May 20, 2019

ReactDOM: size: 0.0%, gzip: 0.0%

Details of bundled changes.

Comparing: 9c6de71...94d7ffe

react-dom

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-dom.development.js 0.0% -0.0% 837.38 KB 837.38 KB 190.99 KB 190.99 KB UMD_DEV
react-dom.production.min.js 0.0% 0.0% 105.1 KB 105.1 KB 34.12 KB 34.13 KB UMD_PROD
react-dom.development.js 0.0% -0.0% 831.7 KB 831.7 KB 189.43 KB 189.43 KB NODE_DEV
react-dom.profiling.min.js 0.0% 0.0% 108.44 KB 108.44 KB 34.5 KB 34.5 KB NODE_PROFILING
ReactDOM-dev.js 0.0% 0.0% 856.83 KB 856.83 KB 190.96 KB 190.96 KB FB_WWW_DEV
ReactDOM-prod.js 0.0% -0.0% 352.99 KB 352.99 KB 65.3 KB 65.29 KB FB_WWW_PROD
react-dom-unstable-fire.development.js 0.0% -0.0% 837.72 KB 837.72 KB 191.14 KB 191.14 KB UMD_DEV
react-dom-unstable-fire.profiling.min.js 0.0% -0.0% 108.27 KB 108.27 KB 35.16 KB 35.16 KB UMD_PROFILING
react-dom-unstable-fire.production.min.js 0.0% -0.0% 105.11 KB 105.11 KB 33.59 KB 33.59 KB NODE_PROD
ReactFire-prod.js 0.0% 0.0% 340.97 KB 340.97 KB 62.83 KB 62.83 KB FB_WWW_PROD
ReactFire-profiling.js 0.0% -0.0% 345.92 KB 345.92 KB 63.82 KB 63.82 KB FB_WWW_PROFILING
react-dom-test-utils.development.js +0.5% +0.4% 56.93 KB 57.24 KB 15.68 KB 15.75 KB UMD_DEV
react-dom-test-utils.production.min.js 🔺+0.4% 🔺+0.7% 10.76 KB 10.81 KB 3.95 KB 3.97 KB UMD_PROD
react-dom-test-utils.development.js +0.6% +0.5% 55.27 KB 55.58 KB 15.36 KB 15.43 KB NODE_DEV
react-dom-test-utils.production.min.js 🔺+0.5% 🔺+1.1% 10.51 KB 10.56 KB 3.88 KB 3.92 KB NODE_PROD
ReactTestUtils-dev.js +0.6% +0.5% 52.7 KB 53.03 KB 14.21 KB 14.28 KB FB_WWW_DEV
react-dom-unstable-native-dependencies.production.min.js 0.0% -0.0% 10.43 KB 10.43 KB 3.57 KB 3.56 KB NODE_PROD
react-dom-server.browser.development.js 0.0% -0.0% 137.21 KB 137.21 KB 36.17 KB 36.17 KB UMD_DEV
react-dom-server.browser.production.min.js 0.0% 0.0% 19.2 KB 19.2 KB 7.22 KB 7.23 KB UMD_PROD
react-dom-server.browser.development.js 0.0% -0.0% 133.34 KB 133.34 KB 35.23 KB 35.23 KB NODE_DEV
ReactDOMServer-dev.js 0.0% -0.0% 135.54 KB 135.54 KB 34.8 KB 34.8 KB FB_WWW_DEV
react-dom-unstable-fizz.browser.development.js 0.0% +0.2% 3.81 KB 3.81 KB 1.53 KB 1.54 KB UMD_DEV
react-dom-unstable-fizz.browser.production.min.js 0.0% 🔺+0.1% 1.21 KB 1.21 KB 705 B 706 B UMD_PROD
react-dom-unstable-fizz.browser.development.js 0.0% +0.1% 3.64 KB 3.64 KB 1.49 KB 1.49 KB NODE_DEV
react-dom-unstable-fizz.browser.production.min.js 0.0% 🔺+0.2% 1.05 KB 1.05 KB 636 B 637 B NODE_PROD

react-test-renderer

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-test-renderer.development.js +0.1% +0.1% 517.57 KB 517.88 KB 110.92 KB 110.98 KB UMD_DEV
react-test-renderer.production.min.js 🔺+0.1% 🔺+0.2% 63.06 KB 63.11 KB 19.43 KB 19.47 KB UMD_PROD
react-test-renderer.development.js +0.1% +0.1% 513.11 KB 513.42 KB 109.82 KB 109.88 KB NODE_DEV
react-test-renderer.production.min.js 🔺+0.1% 🔺+0.2% 62.75 KB 62.8 KB 19.24 KB 19.27 KB NODE_PROD
ReactTestRenderer-dev.js +0.1% +0.1% 525.18 KB 525.5 KB 109.8 KB 109.87 KB FB_WWW_DEV
react-test-renderer-shallow.development.js 0.0% 0.0% 41.95 KB 41.95 KB 10.83 KB 10.83 KB UMD_DEV

react-noop-renderer

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-noop-renderer.development.js +0.9% +0.7% 37.83 KB 38.16 KB 9.43 KB 9.5 KB NODE_DEV
react-noop-renderer.production.min.js 🔺+0.4% 🔺+1.0% 11.17 KB 11.21 KB 3.57 KB 3.6 KB NODE_PROD
react-noop-renderer-persistent.development.js +0.9% +0.7% 37.95 KB 38.28 KB 9.45 KB 9.51 KB NODE_DEV
react-noop-renderer-persistent.production.min.js 🔺+0.4% 🔺+1.0% 11.19 KB 11.23 KB 3.57 KB 3.61 KB NODE_PROD

Generated by 🚫 dangerJS

@acdlite
Copy link
Collaborator

acdlite commented May 20, 2019

my previous PR (#15519) ended up in merge conflict hell, so I just redid it.

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) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit:

Suggested change
if (actingUpdatesScopeDepth === 1) {
if (actingUpdatesScopeDepth <= 1) {

Copy link
Contributor Author

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.

Copy link
Contributor Author

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

@threepointone threepointone merged commit 9c9ea94 into facebook:master May 21, 2019
robertknight added a commit to preactjs/preact that referenced this pull request Aug 11, 2019
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
robertknight added a commit to preactjs/preact that referenced this pull request Aug 11, 2019
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
NMinhNguyen referenced this pull request in enzymejs/react-shallow-renderer Jan 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants