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

[Fizz] Don't flush empty segments #24054

Merged
merged 1 commit into from
Mar 9, 2022

Conversation

sebmarkbage
Copy link
Collaborator

Before this change, we would sometimes write segments without any content in them. For example for a Suspense boundary that immediately suspends we might emit something like:

<div hidden id="123">
  <template id="456"></template>
</div>

Where the outer div is just a temporary wrapper and the inner one is a placeholder for something to be added later.

This serves no purpose.

We should ideally have a heuristic that holds back segments based on byte size and time. However, this is a straight forward clear win for now.

The root most completed segments of a boundary are queued onto it. I simply avoid queuing the segment if it's empty and instead queue its child or waits for it to complete.

@sizebot
Copy link

sizebot commented Mar 9, 2022

Comparing: 11c5bb6...b9a1049

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.21 kB 131.21 kB = 42.00 kB 42.00 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 136.26 kB 136.26 kB = 43.47 kB 43.47 kB
facebook-www/ReactDOM-prod.classic.js = 435.62 kB 435.62 kB = 79.79 kB 79.79 kB
facebook-www/ReactDOM-prod.modern.js = 422.05 kB 422.05 kB = 77.77 kB 77.77 kB
facebook-www/ReactDOMForked-prod.classic.js = 435.62 kB 435.62 kB = 79.79 kB 79.79 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable-semver/react-server/cjs/react-server.production.min.js +0.71% 19.76 kB 19.90 kB +0.53% 6.82 kB 6.85 kB
oss-stable/react-server/cjs/react-server.production.min.js +0.71% 19.76 kB 19.90 kB +0.53% 6.82 kB 6.85 kB
oss-experimental/react-server/cjs/react-server.production.min.js +0.70% 20.09 kB 20.23 kB +0.52% 6.91 kB 6.95 kB
facebook-www/ReactDOMServer-prod.modern.js +0.62% 75.14 kB 75.60 kB +0.45% 15.55 kB 15.62 kB
facebook-www/ReactDOMServerStreaming-prod.modern.js +0.61% 76.96 kB 77.42 kB +0.39% 16.36 kB 16.42 kB
oss-stable-semver/react-server/cjs/react-server.development.js +0.54% 124.13 kB 124.81 kB +0.37% 31.16 kB 31.27 kB
oss-stable/react-server/cjs/react-server.development.js +0.54% 124.13 kB 124.81 kB +0.37% 31.16 kB 31.27 kB
oss-experimental/react-server/cjs/react-server.development.js +0.54% 125.00 kB 125.68 kB +0.38% 31.36 kB 31.48 kB
oss-stable-semver/react-dom/cjs/react-dom-server-legacy.browser.production.min.js +0.44% 32.23 kB 32.37 kB +0.39% 10.74 kB 10.79 kB
oss-stable/react-dom/cjs/react-dom-server-legacy.browser.production.min.js +0.44% 32.23 kB 32.37 kB +0.39% 10.74 kB 10.79 kB
oss-stable-semver/react-dom/umd/react-dom-server-legacy.browser.production.min.js +0.44% 32.38 kB 32.52 kB +0.28% 10.88 kB 10.91 kB
oss-stable/react-dom/umd/react-dom-server-legacy.browser.production.min.js +0.44% 32.38 kB 32.52 kB +0.28% 10.88 kB 10.91 kB
oss-experimental/react-dom/cjs/react-dom-server-legacy.browser.production.min.js +0.44% 32.63 kB 32.77 kB +0.39% 10.88 kB 10.93 kB
oss-experimental/react-dom/umd/react-dom-server-legacy.browser.production.min.js +0.43% 32.78 kB 32.92 kB +0.23% 11.03 kB 11.06 kB
oss-stable-semver/react-dom/cjs/react-dom-server.browser.production.min.js +0.43% 33.39 kB 33.53 kB +0.39% 11.40 kB 11.44 kB
oss-stable/react-dom/cjs/react-dom-server.browser.production.min.js +0.43% 33.39 kB 33.53 kB +0.39% 11.40 kB 11.44 kB
oss-stable-semver/react-dom/umd/react-dom-server.browser.production.min.js +0.42% 33.52 kB 33.66 kB +0.41% 11.52 kB 11.57 kB
oss-stable/react-dom/umd/react-dom-server.browser.production.min.js +0.42% 33.52 kB 33.66 kB +0.41% 11.52 kB 11.57 kB
oss-experimental/react-dom/cjs/react-dom-server.browser.production.min.js +0.42% 33.78 kB 33.93 kB +0.43% 11.53 kB 11.58 kB
oss-experimental/react-dom/umd/react-dom-server.browser.production.min.js +0.42% 33.92 kB 34.06 kB +0.39% 11.66 kB 11.70 kB
oss-stable-semver/react-dom/cjs/react-dom-server.node.production.min.js +0.39% 36.14 kB 36.28 kB +0.32% 12.15 kB 12.19 kB
oss-stable/react-dom/cjs/react-dom-server.node.production.min.js +0.39% 36.14 kB 36.28 kB +0.32% 12.15 kB 12.19 kB
oss-stable-semver/react-dom/cjs/react-dom-server-legacy.node.production.min.js +0.39% 35.91 kB 36.05 kB +0.35% 11.98 kB 12.02 kB
oss-stable/react-dom/cjs/react-dom-server-legacy.node.production.min.js +0.39% 35.91 kB 36.05 kB +0.35% 11.98 kB 12.02 kB
oss-experimental/react-dom/cjs/react-dom-server.node.production.min.js +0.39% 36.60 kB 36.74 kB +0.33% 12.30 kB 12.34 kB
oss-experimental/react-dom/cjs/react-dom-server-legacy.node.production.min.js +0.39% 36.37 kB 36.51 kB +0.33% 12.13 kB 12.17 kB
facebook-www/ReactDOMServerStreaming-dev.modern.js +0.30% 230.67 kB 231.36 kB +0.24% 55.22 kB 55.35 kB
oss-stable-semver/react-dom/cjs/react-dom-server.node.development.js +0.30% 227.33 kB 228.00 kB +0.22% 55.29 kB 55.41 kB
oss-stable/react-dom/cjs/react-dom-server.node.development.js +0.30% 227.33 kB 228.00 kB +0.22% 55.29 kB 55.41 kB
facebook-www/ReactDOMServer-dev.modern.js +0.29% 234.08 kB 234.77 kB +0.24% 56.01 kB 56.15 kB
oss-experimental/react-dom/cjs/react-dom-server.node.development.js +0.29% 228.75 kB 229.42 kB +0.22% 55.65 kB 55.77 kB
oss-stable-semver/react-dom/cjs/react-dom-server.browser.development.js +0.29% 229.33 kB 230.00 kB +0.22% 56.01 kB 56.13 kB
oss-stable/react-dom/cjs/react-dom-server.browser.development.js +0.29% 229.33 kB 230.00 kB +0.22% 56.01 kB 56.13 kB
oss-stable-semver/react-dom/cjs/react-dom-server-legacy.browser.development.js +0.29% 229.69 kB 230.37 kB +0.22% 55.76 kB 55.88 kB
oss-stable/react-dom/cjs/react-dom-server-legacy.browser.development.js +0.29% 229.69 kB 230.37 kB +0.22% 55.76 kB 55.88 kB
oss-stable-semver/react-dom/umd/react-dom-server.browser.development.js +0.29% 240.69 kB 241.39 kB +0.22% 56.60 kB 56.72 kB
oss-stable/react-dom/umd/react-dom-server.browser.development.js +0.29% 240.69 kB 241.39 kB +0.22% 56.60 kB 56.72 kB
oss-experimental/react-dom/cjs/react-dom-server.browser.development.js +0.29% 230.75 kB 231.43 kB +0.21% 56.38 kB 56.50 kB
oss-stable-semver/react-dom/umd/react-dom-server-legacy.browser.development.js +0.29% 241.09 kB 241.79 kB +0.22% 56.39 kB 56.51 kB
oss-stable/react-dom/umd/react-dom-server-legacy.browser.development.js +0.29% 241.09 kB 241.79 kB +0.22% 56.39 kB 56.51 kB
oss-experimental/react-dom/cjs/react-dom-server-legacy.browser.development.js +0.29% 231.12 kB 231.79 kB +0.22% 56.16 kB 56.28 kB
oss-stable-semver/react-dom/cjs/react-dom-server-legacy.node.development.js +0.29% 231.40 kB 232.07 kB +0.21% 56.24 kB 56.35 kB
oss-stable/react-dom/cjs/react-dom-server-legacy.node.development.js +0.29% 231.40 kB 232.07 kB +0.21% 56.24 kB 56.35 kB
oss-experimental/react-dom/umd/react-dom-server.browser.development.js +0.29% 242.19 kB 242.89 kB +0.21% 56.96 kB 57.08 kB
oss-experimental/react-dom/umd/react-dom-server-legacy.browser.development.js +0.29% 242.59 kB 243.29 kB +0.21% 56.76 kB 56.89 kB
oss-experimental/react-dom/cjs/react-dom-server-legacy.node.development.js +0.29% 232.82 kB 233.50 kB +0.21% 56.64 kB 56.76 kB

Generated by 🚫 dangerJS against b9a1049

Before this change, we would sometimes write segments without any content
in them. For example for a Suspense boundary that immediately suspends
we might emit something like:

<div hidden id="123">
  <template id="456"></template>
</div>

Where the outer div is just a temporary wrapper and the inner one is a
placeholder for something to be added later.

This serves no purpose.

We should ideally have a heuristic that holds back segments based on byte
size and time. However, this is a straight forward clear win for now.
@sebmarkbage
Copy link
Collaborator Author

Notably, if two children suspends, it'll still emit:

<div hidden id="123">
  <template id="456"></template>
  <template id="789"></template>
</div>

That way each of those can be filled in independently.

@sebmarkbage sebmarkbage merged commit c91892e into facebook:main Mar 9, 2022
facebook-github-bot pushed a commit to facebook/react-native that referenced this pull request Mar 24, 2022
Summary:
This sync includes the following changes:
- **[3f8990898](facebook/react@3f8990898 )**: Fix test-build-devtools if build was generated by build-for-devtools ([#24088](facebook/react#24088)) //<Sebastian Silbermann>//
- **[577f2de46](facebook/react@577f2de46 )**: enableCacheElement flag ([#24131](facebook/react#24131)) //<David McCabe>//
- **[2e0d86d22](facebook/react@2e0d86d22 )**: Allow updating dehydrated root at lower priority without forcing client render ([#24082](facebook/react#24082)) //<Andrew Clark>//
- **[dbe9e732a](facebook/react@dbe9e732a )**: Avoid conditions where control flow is sufficient ([#24126](facebook/react#24126)) //<Sebastian Markbåge>//
- **[b075f9742](facebook/react@b075f9742 )**: Fix dispatch config type for skipBubbling ([#24109](facebook/react#24109)) //<Luna>//
- **[ef23a9ee8](facebook/react@ef23a9ee8 )**: Flag for text hydration mismatch ([#24107](facebook/react#24107)) //<salazarm>//
- **[0412f0c1a](facebook/react@0412f0c1a )**: add offscreen state node ([#24026](facebook/react#24026)) //<Luna Ruan>//
- **[43eb28339](facebook/react@43eb28339 )**: Add skipBubbling property to dispatch config ([#23366](facebook/react#23366)) //<Luna>//
- **[832e2987e](facebook/react@832e2987e )**: Revert accdientally merged PR ([#24081](facebook/react#24081)) //<Andrew Clark>//
- **[02b65fd8c](facebook/react@02b65fd8c )**: Allow updates at lower pri without forcing client render //<Andrew Clark>//
- **[83b941a51](facebook/react@83b941a51 )**: Add isRootDehydrated function //<Andrew Clark>//
- **[c8e4789e2](facebook/react@c8e4789e2 )**: Pass children to hydration root constructor //<Andrew Clark>//
- **[581f0c42e](facebook/react@581f0c42e )**: [Flight] add support for Lazy components in Flight server ([#24068](facebook/react#24068)) //<Josh Story>//
- **[72a933d28](facebook/react@72a933d28 )**: Gate legacy hidden ([#24047](facebook/react#24047)) //<Sebastian Markbåge>//
- **[b9de50d2f](facebook/react@b9de50d2f )**: Update test to reset modules instead of using private state ([#24055](facebook/react#24055)) //<Sebastian Markbåge>//
- **[c91892ec3](facebook/react@c91892ec3 )**: [Fizz] Don't flush empty segments ([#24054](facebook/react#24054)) //<Sebastian Markbåge>//
- **[d5f1b067c](facebook/react@d5f1b067c )**: [ServerContext] Flight support for ServerContext ([#23244](facebook/react#23244)) //<salazarm>//
- **[6edd55a3f](facebook/react@6edd55a3f )**: Gate unstable_expectedLoadTime on enableCPUSuspense ([#24038](facebook/react#24038)) //<Sebastian Markbåge>//
- **[57799b912](facebook/react@57799b912 )**: Add more feature flag checks ([#24037](facebook/react#24037)) //<Sebastian Markbåge>//
- **[e09518e5b](facebook/react@e09518e5b )**: [Fizz] write chunks to a buffer with no re-use ([#24034](facebook/react#24034)) //<Josh Story>//
- **[14c2be8da](facebook/react@14c2be8da )**: Rename Node SSR Callbacks to onShellReady/onAllReady and Other Fixes ([#24030](facebook/react#24030)) //<Sebastian Markbåge>//
- **[cb1e7b1c6](facebook/react@cb1e7b1c6 )**: Move onCompleteAll to .allReady Promise ([#24025](facebook/react#24025)) //<Sebastian Markbåge>//
- **[566285761](facebook/react@566285761 )**: [Fizz] Export debug function for FB ([#24024](facebook/react#24024)) //<salazarm>//
- **[05c283c3c](facebook/react@05c283c3c )**: Fabric HostComponent as EventEmitter: support add/removeEventListener (unstable only) ([#23386](facebook/react#23386)) //<Joshua Gross>//
- **[08644348b](facebook/react@08644348b )**: Added unit Tests in the ReactART, increasing the code coverage ([#23195](facebook/react#23195)) //<BIKI DAS>//
- **[feefe437f](facebook/react@feefe437f )**: Refactor Cache Code ([#23393](facebook/react#23393)) //<Luna Ruan>//

Changelog:
[General][Changed] - React Native sync for revisions 1780659...1159ff6

jest_e2e[run_all_tests]

Reviewed By: lunaleaps

Differential Revision: D34928167

fbshipit-source-id: 8c386f2be5871981d217ab9a514892ed88eafcfb
zhengjitf pushed a commit to zhengjitf/react that referenced this pull request Apr 15, 2022
Before this change, we would sometimes write segments without any content
in them. For example for a Suspense boundary that immediately suspends
we might emit something like:

<div hidden id="123">
  <template id="456"></template>
</div>

Where the outer div is just a temporary wrapper and the inner one is a
placeholder for something to be added later.

This serves no purpose.

We should ideally have a heuristic that holds back segments based on byte
size and time. However, this is a straight forward clear win for now.
Saadnajmi pushed a commit to Saadnajmi/react-native-macos that referenced this pull request Jan 15, 2023
Summary:
This sync includes the following changes:
- **[3f8990898](facebook/react@3f8990898 )**: Fix test-build-devtools if build was generated by build-for-devtools ([facebook#24088](facebook/react#24088)) //<Sebastian Silbermann>//
- **[577f2de46](facebook/react@577f2de46 )**: enableCacheElement flag ([facebook#24131](facebook/react#24131)) //<David McCabe>//
- **[2e0d86d22](facebook/react@2e0d86d22 )**: Allow updating dehydrated root at lower priority without forcing client render ([facebook#24082](facebook/react#24082)) //<Andrew Clark>//
- **[dbe9e732a](facebook/react@dbe9e732a )**: Avoid conditions where control flow is sufficient ([facebook#24126](facebook/react#24126)) //<Sebastian Markbåge>//
- **[b075f9742](facebook/react@b075f9742 )**: Fix dispatch config type for skipBubbling ([facebook#24109](facebook/react#24109)) //<Luna>//
- **[ef23a9ee8](facebook/react@ef23a9ee8 )**: Flag for text hydration mismatch ([facebook#24107](facebook/react#24107)) //<salazarm>//
- **[0412f0c1a](facebook/react@0412f0c1a )**: add offscreen state node ([facebook#24026](facebook/react#24026)) //<Luna Ruan>//
- **[43eb28339](facebook/react@43eb28339 )**: Add skipBubbling property to dispatch config ([facebook#23366](facebook/react#23366)) //<Luna>//
- **[832e2987e](facebook/react@832e2987e )**: Revert accdientally merged PR ([facebook#24081](facebook/react#24081)) //<Andrew Clark>//
- **[02b65fd8c](facebook/react@02b65fd8c )**: Allow updates at lower pri without forcing client render //<Andrew Clark>//
- **[83b941a51](facebook/react@83b941a51 )**: Add isRootDehydrated function //<Andrew Clark>//
- **[c8e4789e2](facebook/react@c8e4789e2 )**: Pass children to hydration root constructor //<Andrew Clark>//
- **[581f0c42e](facebook/react@581f0c42e )**: [Flight] add support for Lazy components in Flight server ([facebook#24068](facebook/react#24068)) //<Josh Story>//
- **[72a933d28](facebook/react@72a933d28 )**: Gate legacy hidden ([facebook#24047](facebook/react#24047)) //<Sebastian Markbåge>//
- **[b9de50d2f](facebook/react@b9de50d2f )**: Update test to reset modules instead of using private state ([facebook#24055](facebook/react#24055)) //<Sebastian Markbåge>//
- **[c91892ec3](facebook/react@c91892ec3 )**: [Fizz] Don't flush empty segments ([facebook#24054](facebook/react#24054)) //<Sebastian Markbåge>//
- **[d5f1b067c](facebook/react@d5f1b067c )**: [ServerContext] Flight support for ServerContext ([facebook#23244](facebook/react#23244)) //<salazarm>//
- **[6edd55a3f](facebook/react@6edd55a3f )**: Gate unstable_expectedLoadTime on enableCPUSuspense ([facebook#24038](facebook/react#24038)) //<Sebastian Markbåge>//
- **[57799b912](facebook/react@57799b912 )**: Add more feature flag checks ([facebook#24037](facebook/react#24037)) //<Sebastian Markbåge>//
- **[e09518e5b](facebook/react@e09518e5b )**: [Fizz] write chunks to a buffer with no re-use ([facebook#24034](facebook/react#24034)) //<Josh Story>//
- **[14c2be8da](facebook/react@14c2be8da )**: Rename Node SSR Callbacks to onShellReady/onAllReady and Other Fixes ([facebook#24030](facebook/react#24030)) //<Sebastian Markbåge>//
- **[cb1e7b1c6](facebook/react@cb1e7b1c6 )**: Move onCompleteAll to .allReady Promise ([facebook#24025](facebook/react#24025)) //<Sebastian Markbåge>//
- **[566285761](facebook/react@566285761 )**: [Fizz] Export debug function for FB ([facebook#24024](facebook/react#24024)) //<salazarm>//
- **[05c283c3c](facebook/react@05c283c3c )**: Fabric HostComponent as EventEmitter: support add/removeEventListener (unstable only) ([facebook#23386](facebook/react#23386)) //<Joshua Gross>//
- **[08644348b](facebook/react@08644348b )**: Added unit Tests in the ReactART, increasing the code coverage ([facebook#23195](facebook/react#23195)) //<BIKI DAS>//
- **[feefe437f](facebook/react@feefe437f )**: Refactor Cache Code ([facebook#23393](facebook/react#23393)) //<Luna Ruan>//

Changelog:
[General][Changed] - React Native sync for revisions 1780659...1159ff6

jest_e2e[run_all_tests]

Reviewed By: lunaleaps

Differential Revision: D34928167

fbshipit-source-id: 8c386f2be5871981d217ab9a514892ed88eafcfb
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.

4 participants