Skip to content

Commit

Permalink
Re-add reentrancy avoidance
Browse files Browse the repository at this point in the history
I removed the equivalency of this in #22446. However, I didn't fully
understand the intended semantics of the spec but I understand this better
now.

The spec is not actually recursive. It won't call pull again inside of a
pull. It might not call it inside startWork neither which the original
issue avoided. However, it will call pull if you enqueue to the controller
without filling up the desired size outside any call.

We could avoid that by returning a Promise from pull that we wait to
resolve until we've performed all our pending tasks. That would be the
more idiomatic solution. That's a bit more involved but since we know
understand it, we can readd the reentrancy hack since we have an easy place
to detect it. If anything, it should probably throw or log here otherwise.

I believe this fixes #22772.

This includes the test from #22889 but should ideally have one for Fizz.
  • Loading branch information
sebmarkbage committed Feb 23, 2022
1 parent 5619560 commit 8ecc536
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 0 deletions.
4 changes: 4 additions & 0 deletions packages/react-server/src/ReactFizzServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -1950,6 +1950,10 @@ export function startFlowing(request: Request, destination: Destination): void {
if (request.status === CLOSED) {
return;
}
if (request.destination !== null) {
// We're already flowing.
return;
}
request.destination = destination;
try {
flushCompletedQueues(request, destination);
Expand Down
4 changes: 4 additions & 0 deletions packages/react-server/src/ReactFlightServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -790,6 +790,10 @@ export function startFlowing(request: Request, destination: Destination): void {
if (request.status === CLOSED) {
return;
}
if (request.destination !== null) {
// We're already flowing.
return;
}
request.destination = destination;
try {
flushCompletedChunks(request, destination);
Expand Down

0 comments on commit 8ecc536

Please sign in to comment.