Skip to content

Commit

Permalink
[Fizz] Expose a method to explicitly start writing to a Node stream (f…
Browse files Browse the repository at this point in the history
…acebook#21028)

* Expose an explicit point when to start writing in the Node API

* Add a previously failing test
  • Loading branch information
sebmarkbage authored and koto committed Jun 15, 2021
1 parent 0243cf4 commit b8765f8
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 10 deletions.
10 changes: 8 additions & 2 deletions packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -196,14 +196,15 @@ describe('ReactDOMFizzServer', () => {
// @gate experimental
it('should asynchronously load the suspense boundary', async () => {
await act(async () => {
ReactDOMFizzServer.pipeToNodeWritable(
const {startWriting} = ReactDOMFizzServer.pipeToNodeWritable(
<div>
<Suspense fallback={<Text text="Loading..." />}>
<AsyncText text="Hello World" />
</Suspense>
</div>,
writable,
);
startWriting();
});
expect(getVisibleChildren(container)).toEqual(<div>Loading...</div>);
await act(async () => {
Expand All @@ -229,7 +230,11 @@ describe('ReactDOMFizzServer', () => {
}

await act(async () => {
ReactDOMFizzServer.pipeToNodeWritable(<App />, writable);
const {startWriting} = ReactDOMFizzServer.pipeToNodeWritable(
<App />,
writable,
);
startWriting();
});

// We're still showing a fallback.
Expand Down Expand Up @@ -281,6 +286,7 @@ describe('ReactDOMFizzServer', () => {
let controls;
await act(async () => {
controls = ReactDOMFizzServer.pipeToNodeWritable(<App />, writable);
controls.startWriting();
});

// We're still showing a fallback.
Expand Down
38 changes: 33 additions & 5 deletions packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,33 @@ describe('ReactDOMFizzServer', () => {
// @gate experimental
it('should call pipeToNodeWritable', () => {
const {writable, output} = getTestWritable();
ReactDOMFizzServer.pipeToNodeWritable(<div>hello world</div>, writable);
const {startWriting} = ReactDOMFizzServer.pipeToNodeWritable(
<div>hello world</div>,
writable,
);
startWriting();
jest.runAllTimers();
expect(output.result).toBe('<div>hello world</div>');
});

// @gate experimental
it('should start writing after startWriting', () => {
const {writable, output} = getTestWritable();
const {startWriting} = ReactDOMFizzServer.pipeToNodeWritable(
<div>hello world</div>,
writable,
);
jest.runAllTimers();
// First we write our header.
output.result +=
'<!doctype html><html><head><title>test</title><head><body>';
// Then React starts writing.
startWriting();
expect(output.result).toBe(
'<!doctype html><html><head><title>test</title><head><body><div>hello world</div>',
);
});

// @gate experimental
it('should error the stream when an error is thrown at the root', async () => {
const {writable, output, completed} = getTestWritable();
Expand All @@ -74,6 +96,8 @@ describe('ReactDOMFizzServer', () => {
writable,
);

// The stream is errored even if we haven't started writing.

await completed;

expect(output.error).toBe(theError);
Expand All @@ -83,14 +107,15 @@ 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(
<div>
<Suspense fallback={<Throw />}>
<InfiniteSuspend />
</Suspense>
</div>,
writable,
);
startWriting();

await completed;

Expand All @@ -101,14 +126,15 @@ 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(
<div>
<Suspense fallback={<div>Loading</div>}>
<Throw />
</Suspense>
</div>,
writable,
);
startWriting();

await completed;

Expand All @@ -128,12 +154,13 @@ describe('ReactDOMFizzServer', () => {
function Content() {
return 'Hi';
}
ReactDOMFizzServer.pipeToNodeWritable(
const {startWriting} = ReactDOMFizzServer.pipeToNodeWritable(
<Suspense fallback={<Fallback />}>
<Content />
</Suspense>,
writable,
);
startWriting();

await completed;

Expand All @@ -145,14 +172,15 @@ 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(
<div>
<Suspense fallback={<div>Loading</div>}>
<InfiniteSuspend />
</Suspense>
</div>,
writable,
);
startWriting();

jest.runAllTimers();

Expand Down
10 changes: 9 additions & 1 deletion packages/react-dom/src/server/ReactDOMFizzServerNode.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
},
Expand Down
1 change: 1 addition & 0 deletions packages/react-noop-renderer/src/ReactNoopServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,7 @@ function render(children: React$Element<any>): Destination {
};
const request = ReactNoopServer.createRequest(children, destination);
ReactNoopServer.startWork(request);
ReactNoopServer.startFlowing(request);
return destination;
}

Expand Down
2 changes: 0 additions & 2 deletions packages/react-server/src/ReactFizzServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}

Expand Down

0 comments on commit b8765f8

Please sign in to comment.