Skip to content

Move hydration code out of normal Suspense path - #24532

Merged
acdlite merged 2 commits into
react:mainfrom
acdlite:move-hydration-code
May 25, 2022
Merged

Move hydration code out of normal Suspense path#24532
acdlite merged 2 commits into
react:mainfrom
acdlite:move-hydration-code

Conversation

@acdlite

@acdlite acdlite commented May 10, 2022

Copy link
Copy Markdown
Collaborator

Shuffling some code around to make it easier to follow. The logic for updating a dehydrated Suspense boundary is significantly different from the logic for a client-rendered Suspense boundary. Most of it was already lifted out into a separate function; this moves the remaining hydration-specific logic out of updateSuspenseComponent and into updateDehydratedSuspenseComponent instead.

No expected changes to program behavior.

@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels May 10, 2022
@sizebot

sizebot commented May 10, 2022

Copy link
Copy Markdown

Comparing: 2c68776...01562b7

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 = 131.49 kB 131.28 kB = 42.15 kB 42.13 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 136.75 kB 136.54 kB = 43.70 kB 43.68 kB
facebook-www/ReactDOM-prod.classic.js = 441.13 kB 439.35 kB = 80.41 kB 80.29 kB
facebook-www/ReactDOM-prod.modern.js = 426.42 kB 424.64 kB = 78.24 kB 78.13 kB
facebook-www/ReactDOMForked-prod.classic.js = 441.13 kB 439.35 kB = 80.42 kB 80.29 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
oss-experimental/react-reconciler/cjs/react-reconciler.production.min.js = 97.50 kB 97.30 kB +0.04% 29.89 kB 29.90 kB
oss-stable/react-reconciler/cjs/react-reconciler.production.min.js = 92.65 kB 92.45 kB = 28.50 kB 28.50 kB
oss-stable-semver/react-reconciler/cjs/react-reconciler.production.min.js = 92.63 kB 92.43 kB = 28.48 kB 28.48 kB
oss-experimental/react-test-renderer/umd/react-test-renderer.production.min.js = 91.93 kB 91.73 kB +0.14% 28.61 kB 28.65 kB
oss-experimental/react-test-renderer/cjs/react-test-renderer.production.min.js = 91.65 kB 91.45 kB +0.04% 28.26 kB 28.27 kB
oss-stable/react-test-renderer/umd/react-test-renderer.production.min.js = 87.50 kB 87.30 kB +0.06% 27.30 kB 27.31 kB
oss-stable-semver/react-test-renderer/umd/react-test-renderer.production.min.js = 87.48 kB 87.28 kB +0.06% 27.30 kB 27.31 kB
oss-stable/react-test-renderer/cjs/react-test-renderer.production.min.js = 87.22 kB 87.02 kB +0.05% 26.97 kB 26.98 kB
oss-stable-semver/react-test-renderer/cjs/react-test-renderer.production.min.js = 87.20 kB 87.00 kB +0.05% 26.97 kB 26.98 kB
oss-experimental/react-art/cjs/react-art.production.min.js = 86.52 kB 86.32 kB = 26.77 kB 26.76 kB
oss-stable/react-art/cjs/react-art.production.min.js = 81.75 kB 81.55 kB +0.05% 25.37 kB 25.38 kB
oss-stable-semver/react-art/cjs/react-art.production.min.js = 81.72 kB 81.52 kB +0.10% 25.36 kB 25.38 kB
facebook-www/ReactDOM-profiling.classic.js = 471.82 kB 470.16 kB = 85.01 kB 84.87 kB
facebook-www/ReactDOMForked-profiling.classic.js = 471.82 kB 470.16 kB = 85.01 kB 84.88 kB
facebook-www/ReactDOM-profiling.modern.js = 457.05 kB 455.38 kB = 82.71 kB 82.59 kB
facebook-www/ReactDOMForked-profiling.modern.js = 457.05 kB 455.38 kB = 82.72 kB 82.60 kB
facebook-www/ReactDOMTesting-prod.classic.js = 432.94 kB 431.24 kB = 80.56 kB 80.46 kB
facebook-www/ReactDOM-prod.classic.js = 441.13 kB 439.35 kB = 80.41 kB 80.29 kB
facebook-www/ReactDOMForked-prod.classic.js = 441.13 kB 439.35 kB = 80.42 kB 80.29 kB
facebook-www/ReactDOMTesting-prod.modern.js = 416.83 kB 415.13 kB = 78.10 kB 78.00 kB
facebook-www/ReactDOM-prod.modern.js = 426.42 kB 424.64 kB = 78.24 kB 78.13 kB
facebook-www/ReactDOMForked-prod.modern.js = 426.42 kB 424.64 kB = 78.25 kB 78.14 kB
react-native/implementations/ReactNativeRenderer-profiling.fb.js = 331.07 kB 329.24 kB = 58.95 kB 58.83 kB
react-native/implementations/ReactFabric-profiling.fb.js = 323.90 kB 322.09 kB = 57.90 kB 57.78 kB
react-native/implementations/ReactNativeRenderer-profiling.js = 315.02 kB 313.18 kB = 56.29 kB 56.15 kB
react-native/implementations/ReactFabric-profiling.js = 307.90 kB 306.07 kB = 55.28 kB 55.12 kB
react-native/implementations/ReactNativeRenderer-prod.fb.js = 303.92 kB 302.06 kB = 54.71 kB 54.60 kB
react-native/implementations/ReactFabric-prod.fb.js = 296.83 kB 294.97 kB = 53.63 kB 53.52 kB
facebook-www/ReactART-prod.classic.js = 280.34 kB 278.58 kB = 49.56 kB 49.42 kB
react-native/implementations/ReactNativeRenderer-prod.js = 295.80 kB 293.93 kB = 53.14 kB 53.00 kB
react-native/implementations/ReactFabric-prod.js = 288.76 kB 286.89 kB = 52.06 kB 51.92 kB
facebook-www/ReactART-prod.modern.js = 269.52 kB 267.76 kB = 47.81 kB 47.67 kB
facebook-react-native/react-test-renderer/cjs/ReactTestRenderer-profiling.js = 274.43 kB 272.57 kB = 49.37 kB 49.20 kB
facebook-react-native/react-test-renderer/cjs/ReactTestRenderer-prod.js = 259.28 kB 257.50 kB = 47.13 kB 46.96 kB

Generated by 🚫 dangerJS against 01562b7

@acdlite
acdlite force-pushed the move-hydration-code branch 2 times, most recently from 3d0367e to f7c9bf2 Compare May 20, 2022 00:33

ghost left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Makes sense and fixed described in #24578

Andrew Clark added 2 commits May 25, 2022 15:33
Shuffling some code around to make it easier to follow. The logic for
updating a dehydrated Suspense boundary is significantly different
from the logic for a client-rendered Suspense boundary. Most of it was
already lifted out into a separate function; this moves the remaining
hydration-specific logic out of updateSuspenseComponent and into
updateDehydratedSuspenseComponent instead.

No expected changes to program behavior.
Same as previous step but for the complete phase. This is a separate
commit to make bisecting easier in case something breaks. The logic
is very subtle but mostly all I've done is extract it to
another function.
@sebmarkbage

ghost commented May 25, 2022

Copy link
Copy Markdown
Contributor

If this fixes something, then there's a change to behavior. What's the change?

@gnoff

ghost commented May 25, 2022

Copy link
Copy Markdown
Collaborator

hydration errors from an inner boundary can be upgraded to recoverable by an outer boundary. This moves the upgrade call to the code path that only runs for the boundary that is being hydrated

@acdlite

ghost commented May 25, 2022

Copy link
Copy Markdown
Collaborator Author

If this fixes something, then there's a change to behavior. What's the change?

This upgradeHydrationErrorsToRecoverable call is only meant to be called when going from dehydrated -> not dehydrated. Which was just a mistake in the old code that my refactoring happened to fix. I didn't realize this when I said "no expected changes" but as you can see I hedged with "expected" :)

https://github.com/acdlite/react/blob/01562b73ccd3eefdc5c0f3ebf028138576c6d9bf/packages/react-reconciler/src/ReactFiberCompleteWork.new.js#L838-L842

@acdlite

ghost commented May 25, 2022

Copy link
Copy Markdown
Collaborator Author

I didn't add a test because @gnoff already had one, here: #24618

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