From 7d1675c09da89baafbd3b4662ccae50ed37d4573 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 17 Mar 2021 22:17:57 -0400 Subject: [PATCH 1/2] Expose an explicit point when to start writing in the Node API --- .../src/__tests__/ReactDOMFizzServer-test.js | 10 ++++++++-- .../__tests__/ReactDOMFizzServerNode-test.js | 20 ++++++++++++++----- .../src/server/ReactDOMFizzServerNode.js | 10 +++++++++- .../src/ReactNoopServer.js | 1 + packages/react-server/src/ReactFizzServer.js | 2 -- 5 files changed, 33 insertions(+), 10 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index b971874d62dc6..9f139dd3baf15 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -196,7 +196,7 @@ describe('ReactDOMFizzServer', () => { // @gate experimental it('should asynchronously load the suspense boundary', async () => { await act(async () => { - ReactDOMFizzServer.pipeToNodeWritable( + const {startWriting} = ReactDOMFizzServer.pipeToNodeWritable(
}> @@ -204,6 +204,7 @@ describe('ReactDOMFizzServer', () => {
, writable, ); + startWriting(); }); expect(getVisibleChildren(container)).toEqual(
Loading...
); await act(async () => { @@ -229,7 +230,11 @@ describe('ReactDOMFizzServer', () => { } await act(async () => { - ReactDOMFizzServer.pipeToNodeWritable(, writable); + const {startWriting} = ReactDOMFizzServer.pipeToNodeWritable( + , + writable, + ); + startWriting(); }); // We're still showing a fallback. @@ -281,6 +286,7 @@ describe('ReactDOMFizzServer', () => { let controls; await act(async () => { controls = ReactDOMFizzServer.pipeToNodeWritable(, writable); + controls.startWriting(); }); // We're still showing a fallback. diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js index b5f6d0bfa83ad..569868ce6f119 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js @@ -59,7 +59,11 @@ describe('ReactDOMFizzServer', () => { // @gate experimental it('should call pipeToNodeWritable', () => { const {writable, output} = getTestWritable(); - ReactDOMFizzServer.pipeToNodeWritable(
hello world
, writable); + const {startWriting} = ReactDOMFizzServer.pipeToNodeWritable( +
hello world
, + writable, + ); + startWriting(); jest.runAllTimers(); expect(output.result).toBe('
hello world
'); }); @@ -74,6 +78,8 @@ describe('ReactDOMFizzServer', () => { writable, ); + // The stream is errored even if we haven't started writing. + await completed; expect(output.error).toBe(theError); @@ -83,7 +89,7 @@ describe('ReactDOMFizzServer', () => { // @gate experimental it('should error the stream when an error is thrown inside a fallback', async () => { const {writable, output, completed} = getTestWritable(); - ReactDOMFizzServer.pipeToNodeWritable( + const {startWriting} = ReactDOMFizzServer.pipeToNodeWritable(
}> @@ -91,6 +97,7 @@ describe('ReactDOMFizzServer', () => {
, writable, ); + startWriting(); await completed; @@ -101,7 +108,7 @@ describe('ReactDOMFizzServer', () => { // @gate experimental it('should not error the stream when an error is thrown inside suspense boundary', async () => { const {writable, output, completed} = getTestWritable(); - ReactDOMFizzServer.pipeToNodeWritable( + const {startWriting} = ReactDOMFizzServer.pipeToNodeWritable(
Loading
}> @@ -109,6 +116,7 @@ describe('ReactDOMFizzServer', () => { , writable, ); + startWriting(); await completed; @@ -128,12 +136,13 @@ describe('ReactDOMFizzServer', () => { function Content() { return 'Hi'; } - ReactDOMFizzServer.pipeToNodeWritable( + const {startWriting} = ReactDOMFizzServer.pipeToNodeWritable( }> , writable, ); + startWriting(); await completed; @@ -145,7 +154,7 @@ describe('ReactDOMFizzServer', () => { // @gate experimental it('should be able to complete by aborting even if the promise never resolves', async () => { const {writable, output, completed} = getTestWritable(); - const {abort} = ReactDOMFizzServer.pipeToNodeWritable( + const {startWriting, abort} = ReactDOMFizzServer.pipeToNodeWritable(
Loading
}> @@ -153,6 +162,7 @@ describe('ReactDOMFizzServer', () => { , writable, ); + startWriting(); jest.runAllTimers(); diff --git a/packages/react-dom/src/server/ReactDOMFizzServerNode.js b/packages/react-dom/src/server/ReactDOMFizzServerNode.js index cb15bd197300e..e79cbf0986912 100644 --- a/packages/react-dom/src/server/ReactDOMFizzServerNode.js +++ b/packages/react-dom/src/server/ReactDOMFizzServerNode.js @@ -32,9 +32,17 @@ function pipeToNodeWritable( destination: Writable, ): Controls { const request = createRequest(children, destination); - destination.on('drain', createDrainHandler(destination, request)); + let hasStartedFlowing = false; startWork(request); return { + startWriting() { + if (hasStartedFlowing) { + return; + } + hasStartedFlowing = true; + startFlowing(request); + destination.on('drain', createDrainHandler(destination, request)); + }, abort() { abort(request); }, diff --git a/packages/react-noop-renderer/src/ReactNoopServer.js b/packages/react-noop-renderer/src/ReactNoopServer.js index 4553fa43cd4f9..9e0c712661146 100644 --- a/packages/react-noop-renderer/src/ReactNoopServer.js +++ b/packages/react-noop-renderer/src/ReactNoopServer.js @@ -222,6 +222,7 @@ function render(children: React$Element): Destination { }; const request = ReactNoopServer.createRequest(children, destination); ReactNoopServer.startWork(request); + ReactNoopServer.startFlowing(request); return destination; } diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index aff4c50374618..f03da08ab3e30 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -894,8 +894,6 @@ function flushCompletedQueues(request: Request): void { // This would put all waiting boundaries into client-only mode. export function startWork(request: Request): void { - // TODO: Don't automatically start flowing. Expose an explicit signal. Auto-start once everything is done. - request.status = FLOWING; scheduleWork(() => performWork(request)); } From 3036225bcf7fe1a716072027997ef7c2162e3eee Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Thu, 18 Mar 2021 12:24:00 -0400 Subject: [PATCH 2/2] Add a previously failing test --- .../__tests__/ReactDOMFizzServerNode-test.js | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js index 569868ce6f119..a7ea970bece51 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js @@ -68,6 +68,24 @@ describe('ReactDOMFizzServer', () => { expect(output.result).toBe('
hello world
'); }); + // @gate experimental + it('should start writing after startWriting', () => { + const {writable, output} = getTestWritable(); + const {startWriting} = ReactDOMFizzServer.pipeToNodeWritable( +
hello world
, + writable, + ); + jest.runAllTimers(); + // First we write our header. + output.result += + 'test'; + // Then React starts writing. + startWriting(); + expect(output.result).toBe( + 'test
hello world
', + ); + }); + // @gate experimental it('should error the stream when an error is thrown at the root', async () => { const {writable, output, completed} = getTestWritable();