diff --git a/.gitignore b/.gitignore index 438d125b47a0..16982164c421 100644 --- a/.gitignore +++ b/.gitignore @@ -21,6 +21,7 @@ chrome-user-data .idea *.iml .vscode +.zed *.swp *.swo /tmp @@ -40,4 +41,3 @@ packages/react-devtools-fusebox/dist packages/react-devtools-inline/dist packages/react-devtools-shell/dist packages/react-devtools-timeline/dist - diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index 6ff107786f3b..efb7e887cf18 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -6289,6 +6289,100 @@ describe('ReactDOMFizzServer', () => { expect(getVisibleChildren(container)).toEqual('Hi'); }); + // Regression: finishedTask aborting remaining fallback tasks from a + // completed boundary could reenter itself via abortTaskSoft and fire + // onAllReady twice (the inner call drained allPendingTasks to 0 and + // called completeAll, then the outer call re-observed the same 0). + it('only fires onAllReady once when a boundary with an instrumented sync-resolving thenable completes', async () => { + // Mirrors Flight-client chunk behavior: the status-probe .then() in + // trackUsedThenable stays pending, but the ping-attaching .then() in + // renderNode's catch resolves synchronously. This reorders the work + // queue so the fallback task is still in fallbackAbortableTasks when + // the content task completes. + function createDeferredSyncThenable(value) { + let thenCallCount = 0; + return { + status: 'pending', + value: undefined, + then(resolve) { + thenCallCount++; + if (thenCallCount > 1) { + this.status = 'fulfilled'; + this.value = value; + resolve(value); + } + }, + }; + } + + const thenable = createDeferredSyncThenable('hello'); + function AsyncContent() { + return ; + } + + let allReadyCount = 0; + await act(() => { + const {pipe} = renderToPipeableStream( + }> + + , + { + onAllReady() { + allReadyCount++; + }, + }, + ); + pipe(writable); + }); + + expect(allReadyCount).toBe(1); + expect(getVisibleChildren(container)).toEqual('hello'); + }); + + // Same bug, hit without any sync-thenable trickery: if the fallback + // also suspends, its spawned sub-task lives in fallbackAbortableTasks + // and can still be there when the content task completes first. + it('only fires onAllReady once when both content and fallback suspend on real promises', async () => { + let resolveContent; + const contentPromise = new Promise(r => (resolveContent = r)); + // The fallback promise never resolves — the fallback-sub-task gets + // soft-aborted when the content completes, so we never need it. + const fallbackPromise = new Promise(() => {}); + + function AsyncContent() { + return ; + } + function AsyncFallback() { + return ; + } + + let allReadyCount = 0; + await act(() => { + const {pipe} = renderToPipeableStream( + }> + + , + { + onAllReady() { + allReadyCount++; + }, + }, + ); + pipe(writable); + }); + + // Resolving content alone is enough: the fallback-sub-task is still + // in fallbackAbortableTasks when the content task completes, and + // abortTaskSoft on it reenters finishedTask. + await act(async () => { + resolveContent('hello'); + await contentPromise; + }); + + expect(allReadyCount).toBe(1); + expect(getVisibleChildren(container)).toEqual('hello'); + }); + it('promise as node', async () => { const promise = Promise.resolve('Hi'); await act(async () => { diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index 0b115199fc3b..c1fc1531d3d9 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -4955,8 +4955,14 @@ function finishedTask( hoistHoistables(boundaryRow.hoistables, boundary.contentState); } if (!isEligibleForOutlining(request, boundary)) { + // abortTaskSoft reenters finishedTask for each aborted task, which + // decrements allPendingTasks. Ensure that these reentrant finsihedTask + // calls do not call `completeAll` too early by forcing the task counter + // above zero for their duration. + request.allPendingTasks++; boundary.fallbackAbortableTasks.forEach(abortTaskSoft, request); boundary.fallbackAbortableTasks.clear(); + request.allPendingTasks--; if (boundaryRow !== null) { // If we aren't eligible for outlining, we don't have to wait until we flush it. if (--boundaryRow.pendingTasks === 0) {