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

A few Suspense related refactors #25556

Merged
merged 3 commits into from
Oct 29, 2022
Merged

Conversation

acdlite
Copy link
Collaborator

@acdlite acdlite commented Oct 24, 2022

Some Suspense-related refactors to prepare for a feature I'm working on. There are no behavioral changes. (Except for the first commit that adds a warning, which I opened as a separate PR: #25543. I can split the warning out if we're not sure we want to enable it yet.)

You can review each one commit-by-commit.

@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Oct 24, 2022
@sizebot
Copy link

sizebot commented Oct 24, 2022

Comparing: 5450dd4...d3705ee

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js +0.16% 152.89 kB 153.13 kB = 48.85 kB 48.84 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js +0.16% 154.81 kB 155.05 kB +0.05% 49.41 kB 49.44 kB
facebook-www/ReactDOM-prod.classic.js +0.21% 528.08 kB 529.21 kB +0.16% 94.33 kB 94.48 kB
facebook-www/ReactDOM-prod.modern.js +0.22% 513.33 kB 514.47 kB +0.18% 92.16 kB 92.32 kB
facebook-www/ReactDOMForked-prod.classic.js +0.21% 528.08 kB 529.21 kB +0.16% 94.33 kB 94.49 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
facebook-react-native/react-test-renderer/cjs/ReactTestRenderer-prod.js +0.35% 285.09 kB 286.07 kB +0.29% 50.82 kB 50.96 kB
react-native/implementations/ReactFabric-profiling.js +0.33% 332.07 kB 333.16 kB +0.24% 58.95 kB 59.09 kB
facebook-react-native/react-test-renderer/cjs/ReactTestRenderer-profiling.js +0.33% 300.68 kB 301.66 kB +0.31% 53.18 kB 53.34 kB
facebook-www/ReactART-prod.modern.js +0.32% 321.12 kB 322.14 kB +0.28% 54.98 kB 55.13 kB
react-native/implementations/ReactNativeRenderer-profiling.js +0.31% 338.85 kB 339.92 kB +0.25% 59.99 kB 60.14 kB
facebook-www/ReactART-prod.classic.js +0.31% 331.91 kB 332.93 kB +0.27% 56.77 kB 56.93 kB
facebook-www/ReactDOMTesting-prod.modern.js +0.30% 475.36 kB 476.78 kB +0.24% 88.05 kB 88.26 kB
facebook-www/ReactDOMTesting-prod.classic.js +0.30% 491.17 kB 492.62 kB +0.24% 90.35 kB 90.56 kB
react-native/implementations/ReactFabric-profiling.fb.js +0.29% 350.49 kB 351.51 kB +0.24% 62.03 kB 62.18 kB
facebook-www/ReactTestRenderer-dev.modern.js +0.28% 767.96 kB 770.15 kB +0.17% 164.50 kB 164.78 kB
facebook-www/ReactTestRenderer-dev.classic.js +0.28% 767.96 kB 770.15 kB +0.17% 164.50 kB 164.78 kB
react-native/implementations/ReactFabric-prod.js +0.28% 312.97 kB 313.85 kB +0.28% 55.79 kB 55.95 kB
react-native/implementations/ReactNativeRenderer-profiling.fb.js +0.28% 357.26 kB 358.25 kB +0.20% 63.11 kB 63.24 kB
react-native/implementations/ReactFabric-prod.fb.js +0.27% 323.72 kB 324.60 kB +0.25% 57.88 kB 58.02 kB
oss-stable-semver/react-test-renderer/umd/react-test-renderer.development.js +0.27% 772.56 kB 774.66 kB +0.19% 161.33 kB 161.63 kB
oss-stable/react-test-renderer/umd/react-test-renderer.development.js +0.27% 772.58 kB 774.69 kB +0.19% 161.36 kB 161.66 kB
oss-experimental/react-test-renderer/umd/react-test-renderer.development.js +0.27% 772.68 kB 774.79 kB +0.19% 161.40 kB 161.69 kB
oss-stable-semver/react-test-renderer/cjs/react-test-renderer.development.js +0.27% 737.43 kB 739.44 kB +0.13% 159.68 kB 159.90 kB
oss-stable/react-test-renderer/cjs/react-test-renderer.development.js +0.27% 737.46 kB 739.46 kB +0.13% 159.71 kB 159.92 kB
oss-experimental/react-test-renderer/cjs/react-test-renderer.development.js +0.27% 737.56 kB 739.56 kB +0.13% 159.74 kB 159.96 kB
facebook-react-native/react-test-renderer/cjs/ReactTestRenderer-dev.js +0.27% 751.72 kB 753.76 kB +0.15% 161.28 kB 161.52 kB
react-native/implementations/ReactNativeRenderer-prod.js +0.27% 319.67 kB 320.53 kB +0.26% 56.85 kB 56.99 kB
oss-stable-semver/react-art/cjs/react-art.production.min.js +0.26% 91.10 kB 91.34 kB +0.26% 28.11 kB 28.18 kB
oss-stable/react-art/cjs/react-art.production.min.js +0.26% 91.13 kB 91.37 kB +0.26% 28.11 kB 28.18 kB
oss-experimental/react-art/cjs/react-art.production.min.js +0.26% 92.54 kB 92.78 kB +0.24% 28.53 kB 28.60 kB
react-native/implementations/ReactNativeRenderer-prod.fb.js +0.26% 330.42 kB 331.26 kB +0.26% 58.91 kB 59.06 kB
oss-stable-semver/react-art/cjs/react-art.development.js +0.25% 758.14 kB 760.01 kB +0.13% 163.78 kB 164.00 kB
oss-stable/react-art/cjs/react-art.development.js +0.25% 758.17 kB 760.03 kB +0.13% 163.81 kB 164.02 kB
react-native/implementations/ReactFabric-dev.js +0.25% 830.32 kB 832.36 kB +0.16% 180.27 kB 180.55 kB
oss-stable-semver/react-test-renderer/cjs/react-test-renderer.production.min.js +0.25% 99.08 kB 99.32 kB +0.25% 30.53 kB 30.61 kB
oss-stable/react-test-renderer/cjs/react-test-renderer.production.min.js +0.25% 99.10 kB 99.34 kB +0.25% 30.53 kB 30.61 kB
oss-experimental/react-test-renderer/cjs/react-test-renderer.production.min.js +0.25% 99.16 kB 99.40 kB +0.25% 30.56 kB 30.64 kB
oss-experimental/react-art/cjs/react-art.development.js +0.24% 765.97 kB 767.83 kB +0.13% 165.26 kB 165.47 kB
oss-stable-semver/react-test-renderer/umd/react-test-renderer.production.min.js +0.24% 99.32 kB 99.56 kB +0.16% 30.93 kB 30.99 kB
facebook-www/ReactART-dev.modern.js +0.24% 858.88 kB 860.96 kB +0.16% 181.44 kB 181.73 kB
oss-stable/react-test-renderer/umd/react-test-renderer.production.min.js +0.24% 99.34 kB 99.58 kB +0.18% 30.94 kB 30.99 kB
react-native/implementations/ReactNativeRenderer-dev.js +0.24% 839.87 kB 841.90 kB +0.15% 182.72 kB 183.00 kB
oss-experimental/react-test-renderer/umd/react-test-renderer.production.min.js +0.24% 99.40 kB 99.64 kB +0.20% 30.97 kB 31.03 kB
facebook-www/ReactART-dev.classic.js +0.24% 869.18 kB 871.27 kB +0.16% 183.51 kB 183.81 kB
facebook-www/ReactDOM-profiling.modern.js +0.23% 543.22 kB 544.49 kB +0.17% 96.71 kB 96.87 kB
facebook-www/ReactDOMForked-profiling.modern.js +0.23% 543.22 kB 544.49 kB +0.17% 96.71 kB 96.87 kB
oss-stable-semver/react-reconciler/cjs/react-reconciler.production.min.js +0.23% 104.89 kB 105.13 kB +0.23% 32.10 kB 32.17 kB
oss-stable/react-reconciler/cjs/react-reconciler.production.min.js +0.23% 104.91 kB 105.15 kB +0.22% 32.12 kB 32.19 kB
facebook-www/ReactDOM-profiling.classic.js +0.23% 558.06 kB 559.33 kB +0.16% 98.87 kB 99.03 kB
facebook-www/ReactDOMForked-profiling.classic.js +0.23% 558.06 kB 559.33 kB +0.16% 98.87 kB 99.03 kB
oss-stable-semver/react-art/umd/react-art.development.js +0.23% 865.03 kB 866.99 kB +0.13% 181.96 kB 182.20 kB
oss-stable/react-art/umd/react-art.development.js +0.23% 865.05 kB 867.01 kB +0.13% 181.98 kB 182.22 kB
oss-experimental/react-reconciler/cjs/react-reconciler.production.min.js +0.23% 106.33 kB 106.57 kB +0.24% 32.51 kB 32.59 kB
oss-experimental/react-art/umd/react-art.development.js +0.22% 873.30 kB 875.26 kB +0.14% 183.40 kB 183.65 kB
react-native/implementations/ReactFabric-dev.fb.js +0.22% 867.16 kB 869.09 kB +0.13% 187.45 kB 187.70 kB
oss-stable-semver/react-reconciler/cjs/react-reconciler.development.js +0.22% 837.67 kB 839.54 kB +0.10% 178.12 kB 178.30 kB
oss-stable/react-reconciler/cjs/react-reconciler.development.js +0.22% 837.70 kB 839.56 kB +0.10% 178.14 kB 178.32 kB
oss-experimental/react-reconciler/cjs/react-reconciler.development.js +0.22% 845.50 kB 847.37 kB +0.10% 179.56 kB 179.75 kB
react-native/implementations/ReactNativeRenderer-dev.fb.js +0.22% 876.69 kB 878.62 kB +0.14% 189.86 kB 190.12 kB
facebook-www/ReactDOM-prod.modern.js +0.22% 513.33 kB 514.47 kB +0.18% 92.16 kB 92.32 kB
facebook-www/ReactDOMForked-prod.modern.js +0.22% 513.33 kB 514.47 kB +0.18% 92.16 kB 92.33 kB
facebook-www/ReactDOM-prod.classic.js +0.21% 528.08 kB 529.21 kB +0.16% 94.33 kB 94.48 kB
facebook-www/ReactDOMForked-prod.classic.js +0.21% 528.08 kB 529.21 kB +0.16% 94.33 kB 94.49 kB

Generated by 🚫 dangerJS against d3705ee

@acdlite acdlite marked this pull request as ready for review October 24, 2022 17:35
@acdlite acdlite changed the title In work loop, add enum of reasons for suspending A few Suspense related refactors Oct 24, 2022
@acdlite acdlite force-pushed the suspense-reason-enum branch 2 times, most recently from 9e4b112 to 1a70d38 Compare October 24, 2022 19:18
@acdlite acdlite force-pushed the suspense-reason-enum branch from 1a70d38 to 3d20fb6 Compare October 24, 2022 20:24
@acdlite acdlite force-pushed the suspense-reason-enum branch 2 times, most recently from b54bb03 to c11c136 Compare October 25, 2022 15:09
This is a pure refactor, no change to behavior.

When a component throws, the work loop can handle that in one of several
ways — unwind immediately, wait for microtasks, and so on. I'm about
to add another one, too. So I've changed the variable that tracks
whether the work loop is suspended from a boolean
(workInProgressIsSuspended) to an enum (workInProgressSuspendedReason).
Refactors the logic for handling when the work loop is suspended into
separate functions for replaying versus unwinding. This allows us to
hoist certain checks into the caller.

For example, when rendering due to flushSync, we will always unwind the
stack without yielding the microtasks.

No intentional behavior change in this commit.
Pure refactor, no change in behavior.

Extracts the logic for detecting whether a suspended component will
result in a "bad" Suspense fallback into a helper function. An example
of a bad Suspense fallback is one that causes already-visible content
to disappear.

I want to reuse this same logic in the work loop, too.
@acdlite acdlite force-pushed the suspense-reason-enum branch from c11c136 to d3705ee Compare October 29, 2022 00:02
@acdlite acdlite merged commit bdd3d08 into facebook:main Oct 29, 2022
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.

5 participants