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

Bugfix: Legacy Mode + DevTools "force fallback" #19164

Merged
merged 1 commit into from
Jun 19, 2020

Conversation

acdlite
Copy link
Collaborator

@acdlite acdlite commented Jun 19, 2020

DevTools has a feature to force a Suspense boundary to show a fallback. This feature causes us to skip the first render pass (where we render the primary children) and go straight to rendering the fallback.

There's a Legacy Mode-only codepath that failed to take this scenario into account, instead assuming that whenever a fallback is being rendered, it was preceded by an attempt to render the primary children.

SuspenseList can also cause us to skip the first pass, but the relevant branch is Legacy Mode only, and SuspenseList is not supported in Legacy Mode.

Fixes a test that I had temporarily disabled when upstreaming the Lanes implementation in #19108.

DevTools has a feature to force a Suspense boundary to show a fallback.
This feature causes us to skip the first render pass (where we render
the primary children) and go straight to rendering the fallback.

There's a Legacy Mode-only codepath that failed to take this scenario
into account, instead assuming that whenever a fallback is being
rendered, it was preceded by an attempt to render the primary children.

SuspenseList can also cause us to skip the first pass, but the relevant
branch is Legacy Mode-only, and SuspenseList is not supported in
Legacy Mode.

Fixes a test that I had temporarily disabled when upstreaming the Lanes
implementation in facebook#19108.
@acdlite acdlite requested a review from bvaughn June 19, 2020 17:21
@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Jun 19, 2020
@acdlite acdlite force-pushed the bugfix-legacy-mode-devtools branch from ffab048 to 63cb523 Compare June 19, 2020 17:23
@sizebot
Copy link

sizebot commented Jun 19, 2020

Details of bundled changes.

Comparing: d1d9054...fa930db

react-dom

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-dom.development.js 0.0% +0.1% 923.16 KB 923.6 KB 209.82 KB 209.97 KB NODE_DEV
ReactDOMForked-prod.js 0.0% 0.0% 395.61 KB 395.67 KB 73.4 KB 73.41 KB FB_WWW_PROD
react-dom-server.node.development.js 0.0% -0.0% 138.25 KB 138.25 KB 36.58 KB 36.58 KB NODE_DEV
react-dom.production.min.js 0.0% 0.0% 126.77 KB 126.78 KB 40.33 KB 40.33 KB NODE_PROD
ReactDOMForked-profiling.js 0.0% 0.0% 406.15 KB 406.2 KB 75.16 KB 75.17 KB FB_WWW_PROFILING
react-dom-server.browser.development.js 0.0% -0.0% 144.38 KB 144.38 KB 36.78 KB 36.78 KB UMD_DEV
react-dom-server.node.production.min.js 0.0% -0.0% 20.68 KB 20.68 KB 7.66 KB 7.66 KB NODE_PROD
react-dom-test-utils.production.min.js 0.0% -0.0% 13.18 KB 13.18 KB 4.9 KB 4.9 KB UMD_PROD
ReactDOMTesting-dev.js 0.0% +0.1% 953.06 KB 953.53 KB 214.18 KB 214.32 KB FB_WWW_DEV
react-dom-test-utils.development.js 0.0% -0.0% 70.09 KB 70.09 KB 19.6 KB 19.6 KB NODE_DEV
ReactDOMTesting-prod.js 0.0% 0.0% 394.87 KB 394.92 KB 74.51 KB 74.52 KB FB_WWW_PROD
react-dom-unstable-fizz.node.development.js 0.0% -0.1% 5.61 KB 5.61 KB 1.87 KB 1.86 KB NODE_DEV
react-dom-test-utils.production.min.js 0.0% -0.0% 13.06 KB 13.06 KB 4.81 KB 4.8 KB NODE_PROD
react-dom-unstable-fizz.browser.development.js 0.0% +0.1% 5.36 KB 5.36 KB 1.8 KB 1.8 KB UMD_DEV
react-dom-unstable-fizz.node.production.min.js 0.0% -0.1% 1.17 KB 1.17 KB 667 B 666 B NODE_PROD
react-dom-unstable-fizz.browser.production.min.js 0.0% -0.3% 1.2 KB 1.2 KB 706 B 704 B UMD_PROD
react-dom-unstable-fizz.browser.development.js 0.0% -0.1% 4.87 KB 4.87 KB 1.7 KB 1.7 KB NODE_DEV
react-dom.development.js 0.0% +0.1% 970.03 KB 970.49 KB 212.58 KB 212.75 KB UMD_DEV
react-dom-unstable-fizz.browser.production.min.js 0.0% -0.2% 1.01 KB 1.01 KB 616 B 615 B NODE_PROD
react-dom.production.min.js 0.0% 0.0% 126.55 KB 126.56 KB 41.2 KB 41.22 KB UMD_PROD
react-dom.profiling.min.js 0.0% 0.0% 130.51 KB 130.53 KB 42.38 KB 42.38 KB UMD_PROFILING
ReactDOMForked-dev.js 0.0% +0.1% 972.47 KB 972.94 KB 217.11 KB 217.26 KB FB_WWW_DEV
react-dom.profiling.min.js 0.0% 0.0% 130.9 KB 130.91 KB 41.51 KB 41.51 KB NODE_PROFILING
react-dom-server.browser.production.min.js 0.0% -0.0% 20.34 KB 20.34 KB 7.52 KB 7.52 KB UMD_PROD
ReactDOM-dev.js 0.0% +0.1% 973.62 KB 974.09 KB 217.22 KB 217.37 KB FB_WWW_DEV
ReactDOM-prod.js 0.0% 0.0% 395.81 KB 395.86 KB 73.42 KB 73.43 KB FB_WWW_PROD
react-dom-server.browser.development.js 0.0% -0.0% 136.98 KB 136.98 KB 36.33 KB 36.33 KB NODE_DEV
ReactDOM-profiling.js 0.0% 0.0% 406.34 KB 406.4 KB 75.18 KB 75.19 KB FB_WWW_PROFILING
react-dom-server.browser.production.min.js 0.0% -0.0% 20.26 KB 20.26 KB 7.51 KB 7.51 KB NODE_PROD
ReactDOMServer-dev.js 0.0% -0.0% 142.51 KB 142.51 KB 36.26 KB 36.26 KB FB_WWW_DEV
react-dom-test-utils.development.js 0.0% -0.0% 75.26 KB 75.26 KB 20.1 KB 20.1 KB UMD_DEV

react-native-renderer

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
ReactNativeRenderer-dev.js +0.1% +0.1% 682.71 KB 683.18 KB 147.5 KB 147.66 KB RN_FB_DEV
ReactFabric-dev.js +0.1% +0.1% 663.43 KB 663.9 KB 142.92 KB 143.07 KB RN_FB_DEV
ReactNativeRenderer-dev.js +0.1% +0.1% 678.25 KB 678.72 KB 146.83 KB 146.99 KB RN_OSS_DEV
ReactFabric-prod.js 0.0% 0.0% 265.59 KB 265.64 KB 46.98 KB 46.98 KB RN_FB_PROD
ReactNativeRenderer-prod.js 0.0% 0.0% 271.91 KB 271.97 KB 48.26 KB 48.27 KB RN_OSS_PROD
ReactFabric-profiling.js 0.0% 0.0% 277.12 KB 277.18 KB 49.2 KB 49.2 KB RN_FB_PROFILING
ReactNativeRenderer-profiling.js 0.0% 0.0% 283.42 KB 283.47 KB 50.46 KB 50.46 KB RN_OSS_PROFILING
ReactNativeRenderer-prod.js 0.0% 0.0% 271.87 KB 271.92 KB 48.24 KB 48.25 KB RN_FB_PROD
ReactNativeRenderer-profiling.js 0.0% 0.0% 283.37 KB 283.42 KB 50.44 KB 50.44 KB RN_FB_PROFILING
ReactFabric-dev.js +0.1% +0.1% 658.97 KB 659.44 KB 142.23 KB 142.38 KB RN_OSS_DEV
ReactFabric-prod.js 0.0% 0.0% 265.62 KB 265.68 KB 46.99 KB 47 KB RN_OSS_PROD
ReactFabric-profiling.js 0.0% 0.0% 277.16 KB 277.22 KB 49.21 KB 49.22 KB RN_OSS_PROFILING

react-art

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
ReactART-prod.js 0.0% 0.0% 240.22 KB 240.28 KB 42.46 KB 42.46 KB FB_WWW_PROD
react-art.development.js +0.1% +0.1% 685.9 KB 686.36 KB 145.19 KB 145.35 KB UMD_DEV
react-art.production.min.js 0.0% 0.0% 112.35 KB 112.36 KB 34.75 KB 34.76 KB UMD_PROD
react-art.development.js +0.1% +0.1% 587.52 KB 587.97 KB 127.39 KB 127.53 KB NODE_DEV
react-art.production.min.js 0.0% 0.0% 77.31 KB 77.32 KB 23.89 KB 23.9 KB NODE_PROD
ReactART-dev.js +0.1% +0.1% 610.26 KB 610.73 KB 130 KB 130.17 KB FB_WWW_DEV

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% 602.3 KB 602.76 KB 126.46 KB 126.61 KB UMD_DEV
react-test-renderer.production.min.js 0.0% -0.0% 76.6 KB 76.61 KB 23.97 KB 23.97 KB UMD_PROD
react-test-renderer.development.js +0.1% +0.1% 573.85 KB 574.3 KB 125 KB 125.15 KB NODE_DEV
react-test-renderer.production.min.js 0.0% 0.0% 76.42 KB 76.44 KB 23.67 KB 23.67 KB NODE_PROD
ReactTestRenderer-dev.js +0.1% +0.1% 597.11 KB 597.58 KB 127.86 KB 128.02 KB FB_WWW_DEV
ReactTestRenderer-dev.js +0.1% +0.1% 590.18 KB 590.65 KB 127.58 KB 127.74 KB RN_FB_DEV
react-test-renderer-shallow.development.js 0.0% -0.0% 39.17 KB 39.17 KB 9.58 KB 9.58 KB UMD_DEV

react-reconciler

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-reconciler.development.js +0.1% +0.1% 645.35 KB 645.79 KB 137.51 KB 137.66 KB NODE_DEV
react-reconciler-reflection.development.js 0.0% -0.0% 16.7 KB 16.7 KB 4.98 KB 4.98 KB NODE_DEV
react-reconciler.production.min.js 0.0% 0.0% 87.09 KB 87.11 KB 26.63 KB 26.63 KB NODE_PROD
react-reconciler-reflection.production.min.js 0.0% -0.1% 2.81 KB 2.81 KB 1.16 KB 1.16 KB NODE_PROD

ReactDOM: size: 0.0%, gzip: -0.0%

Size changes (experimental)

Generated by 🚫 dangerJS against fa930db

@sizebot
Copy link

sizebot commented Jun 19, 2020

Details of bundled changes.

Comparing: d1d9054...fa930db

react-dom

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-dom.development.js +0.1% +0.1% 892.73 KB 893.17 KB 204.25 KB 204.4 KB NODE_DEV
ReactDOMForked-prod.js 0.0% 0.0% 406.8 KB 406.85 KB 75.24 KB 75.25 KB FB_WWW_PROD
react-dom-server.node.development.js 0.0% -0.0% 136.74 KB 136.74 KB 36.38 KB 36.38 KB NODE_DEV
react-dom.production.min.js 0.0% 0.0% 122.3 KB 122.31 KB 39.03 KB 39.04 KB NODE_PROD
ReactDOMForked-profiling.js 0.0% 0.0% 417.39 KB 417.45 KB 77.01 KB 77.02 KB FB_WWW_PROFILING
react-dom-server.node.production.min.js 0.0% -0.0% 20.22 KB 20.22 KB 7.59 KB 7.58 KB NODE_PROD
react-dom-test-utils.production.min.js 0.0% -0.0% 13.17 KB 13.17 KB 4.89 KB 4.89 KB UMD_PROD
ReactDOMTesting-dev.js 0.0% +0.1% 978.85 KB 979.33 KB 219.83 KB 219.98 KB FB_WWW_DEV
react-dom-test-utils.development.js 0.0% -0.0% 70.08 KB 70.08 KB 19.59 KB 19.59 KB NODE_DEV
ReactDOMTesting-prod.js 0.0% 0.0% 406.84 KB 406.89 KB 76.47 KB 76.48 KB FB_WWW_PROD
react-dom-test-utils.production.min.js 0.0% -0.0% 13.05 KB 13.05 KB 4.8 KB 4.8 KB NODE_PROD
react-dom.development.js 0.0% +0.1% 938.25 KB 938.71 KB 206.9 KB 207.05 KB UMD_DEV
react-dom.production.min.js 0.0% 0.0% 122.16 KB 122.17 KB 39.92 KB 39.92 KB UMD_PROD
react-dom.profiling.min.js 0.0% -0.0% 126.07 KB 126.08 KB 41.09 KB 41.08 KB UMD_PROFILING
ReactDOMForked-dev.js 0.0% +0.1% 998.02 KB 998.49 KB 222.93 KB 223.08 KB FB_WWW_DEV
react-dom.profiling.min.js 0.0% 0.0% 126.38 KB 126.39 KB 40.25 KB 40.25 KB NODE_PROFILING
react-dom-server.browser.production.min.js 0.0% -0.0% 19.88 KB 19.88 KB 7.44 KB 7.44 KB UMD_PROD
ReactDOM-dev.js 0.0% +0.1% 999.16 KB 999.63 KB 223.04 KB 223.19 KB FB_WWW_DEV
ReactDOM-prod.js 0.0% 0.0% 406.99 KB 407.05 KB 75.25 KB 75.26 KB FB_WWW_PROD
react-dom-server.browser.development.js 0.0% -0.0% 135.46 KB 135.46 KB 36.13 KB 36.12 KB NODE_DEV
ReactDOM-profiling.js 0.0% 0.0% 417.59 KB 417.64 KB 77.03 KB 77.04 KB FB_WWW_PROFILING
react-dom-server.browser.production.min.js 0.0% -0.0% 19.8 KB 19.8 KB 7.43 KB 7.43 KB NODE_PROD
ReactDOMServer-dev.js 0.0% -0.0% 146.54 KB 146.54 KB 37.26 KB 37.26 KB FB_WWW_DEV
react-dom-test-utils.development.js 0.0% -0.0% 75.25 KB 75.25 KB 20.09 KB 20.09 KB UMD_DEV

react-art

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
ReactART-prod.js 0.0% 0.0% 247.36 KB 247.42 KB 43.74 KB 43.74 KB FB_WWW_PROD
react-art.development.js +0.1% +0.1% 663.74 KB 664.2 KB 141.11 KB 141.27 KB UMD_DEV
react-art.production.min.js 0.0% 0.0% 109.4 KB 109.41 KB 33.85 KB 33.85 KB UMD_PROD
react-art.development.js +0.1% +0.1% 566.27 KB 566.72 KB 123.35 KB 123.51 KB NODE_DEV
react-art.production.min.js 0.0% -0.0% 74.42 KB 74.43 KB 23.03 KB 23.02 KB NODE_PROD
ReactART-dev.js +0.1% +0.1% 620.28 KB 620.75 KB 132.03 KB 132.19 KB FB_WWW_DEV

react-test-renderer

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-test-renderer-shallow.production.min.js 0.0% -0.0% 12.73 KB 12.73 KB 3.96 KB 3.96 KB UMD_PROD
react-test-renderer.development.js +0.1% +0.1% 602.27 KB 602.73 KB 126.44 KB 126.6 KB UMD_DEV
react-test-renderer.production.min.js 0.0% -0.0% 76.58 KB 76.59 KB 23.96 KB 23.95 KB UMD_PROD
react-test-renderer.development.js +0.1% +0.1% 573.83 KB 574.28 KB 124.99 KB 125.14 KB NODE_DEV
react-test-renderer.production.min.js 0.0% 0.0% 76.4 KB 76.41 KB 23.65 KB 23.65 KB NODE_PROD
ReactTestRenderer-dev.js +0.1% +0.1% 597.1 KB 597.57 KB 127.85 KB 128.01 KB FB_WWW_DEV
ReactTestRenderer-dev.js +0.1% +0.1% 590.16 KB 590.63 KB 127.57 KB 127.74 KB RN_FB_DEV
react-test-renderer-shallow.development.js 0.0% -0.0% 39.16 KB 39.16 KB 9.57 KB 9.57 KB UMD_DEV

react-native-renderer

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
ReactNativeRenderer-dev.js +0.1% +0.1% 678.24 KB 678.71 KB 146.83 KB 146.99 KB RN_OSS_DEV
ReactNativeRenderer-prod.js 0.0% 0.0% 271.9 KB 271.95 KB 48.25 KB 48.26 KB RN_OSS_PROD
ReactNativeRenderer-profiling.js 0.0% 0.0% 283.4 KB 283.46 KB 50.45 KB 50.45 KB RN_OSS_PROFILING
ReactFabric-dev.js +0.1% +0.1% 658.95 KB 659.42 KB 142.22 KB 142.38 KB RN_OSS_DEV
ReactFabric-prod.js 0.0% 0.0% 265.61 KB 265.67 KB 46.99 KB 46.99 KB RN_OSS_PROD
ReactFabric-profiling.js 0.0% 0.0% 277.15 KB 277.2 KB 49.21 KB 49.21 KB RN_OSS_PROFILING

react-reconciler

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-reconciler.development.js +0.1% +0.1% 621.73 KB 622.18 KB 133.01 KB 133.16 KB NODE_DEV
react-reconciler-reflection.development.js 0.0% -0.0% 16.68 KB 16.68 KB 4.98 KB 4.97 KB NODE_DEV
react-reconciler.production.min.js 0.0% 0.0% 83.77 KB 83.78 KB 25.73 KB 25.73 KB NODE_PROD
react-reconciler-reflection.production.min.js 0.0% -0.1% 2.8 KB 2.8 KB 1.15 KB 1.15 KB NODE_PROD

ReactDOM: size: 0.0%, gzip: -0.0%

Size changes (stable)

Generated by 🚫 dangerJS against fa930db

@acdlite acdlite force-pushed the bugfix-legacy-mode-devtools branch from 63cb523 to fa930db Compare June 19, 2020 17:33
@codesandbox-ci
Copy link

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 63cb523:

Sandbox Source
trusting-forest-u5h36 Configuration

@acdlite acdlite merged commit 6ba25b9 into facebook:master Jun 19, 2020
trueadm pushed a commit to trueadm/react that referenced this pull request Jun 22, 2020
DevTools has a feature to force a Suspense boundary to show a fallback.
This feature causes us to skip the first render pass (where we render
the primary children) and go straight to rendering the fallback.

There's a Legacy Mode-only codepath that failed to take this scenario
into account, instead assuming that whenever a fallback is being
rendered, it was preceded by an attempt to render the primary children.

SuspenseList can also cause us to skip the first pass, but the relevant
branch is Legacy Mode-only, and SuspenseList is not supported in
Legacy Mode.

Fixes a test that I had temporarily disabled when upstreaming the Lanes
implementation in facebook#19108.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants