Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fizz] Expose a method to explicitly start writing to a Node stream #21028

Merged
merged 2 commits into from
Mar 18, 2021
Merged

[Fizz] Expose a method to explicitly start writing to a Node stream #21028

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7d1675c
Expose an explicit point when to start writing in the Node API
sebmarkbage Mar 18, 2021
3036225
Add a previously failing test
sebmarkbage Mar 18, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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