Skip to content

Conversation

@gnoff
Copy link
Collaborator

@gnoff gnoff commented Jul 24, 2025

There is an edge case when prerendering where if you have nothing to write you can end up in a state where the prerender is in status closed before you can provide a destination. In this case the destination is never closed becuase it assumes it already would have been.

This condition can happen now because of the introduction of the deubg stream. Before this a request would never entere closed status if there was no active destination. When a destination was added it would perform a flush and possibly close the stream. Now, it is possible to flush without a destination because you might have debug chunks to stream and you can end up closing the stream independent of an active destination.

There are a number of ways we can solve this but the one that seems to adhere best to the original design is to only set the status to CLOSED when a destination is active. This means that if you don't have an active destination when the pendingChunks count hits zero it will not enter CLOSED status until you startFlowing.

@gnoff gnoff force-pushed the fix-close-before-read branch from f89939d to 62f79c7 Compare July 24, 2025 19:33
@gnoff gnoff requested a review from sebmarkbage July 24, 2025 19:33
@github-actions github-actions bot added the React Core Team Opened by a member of the React Core Team label Jul 24, 2025
@react-sizebot
Copy link

react-sizebot commented Jul 24, 2025

Comparing: 4f34cc4...52221d4

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.js = 6.68 kB 6.68 kB = 1.83 kB 1.83 kB
oss-stable/react-dom/cjs/react-dom-client.production.js = 530.70 kB 530.70 kB = 93.70 kB 93.70 kB
oss-experimental/react-dom/cjs/react-dom.production.js = 6.69 kB 6.69 kB = 1.83 kB 1.83 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js = 655.25 kB 655.25 kB = 115.40 kB 115.40 kB
facebook-www/ReactDOM-prod.classic.js = 675.13 kB 675.13 kB = 118.75 kB 118.75 kB
facebook-www/ReactDOM-prod.modern.js = 665.56 kB 665.56 kB = 117.11 kB 117.12 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against 52221d4

@gnoff gnoff force-pushed the fix-close-before-read branch 4 times, most recently from 8e88cfa to 29e9d99 Compare July 24, 2025 19:52
…r prerender

There is an edge case when prerendering where if you have nothing to write you can end up in a state where the prerender is in status closed before you can provide a destination. In this case the destination is never closed becuase it assumes it already would have been.

This condition can happen now because of the introduction of the deubg stream. Before this a request would never entere closed status if there was no active destination. When a destination was added it would perform a flush and possibly close the stream. Now, it is possible to flush without a destination because you might have debug chunks to stream and you can end up closing the stream independent of an active destination.

There are a number of ways we can solve this but the one that seems to adhere best to the original design is to only set the status to CLOSED when a destination is active. This means that if you don't have an active destination when the pendingChunks count hits zero it will not enter CLOSED status until you startFlowing.
@gnoff gnoff force-pushed the fix-close-before-read branch from 29e9d99 to 52221d4 Compare July 24, 2025 19:59
@gnoff gnoff merged commit 5a04619 into facebook:main Jul 25, 2025
241 checks passed
@gnoff gnoff deleted the fix-close-before-read branch July 25, 2025 02:38
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