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.

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/Flight] Pass in Destination lazily to startFlowing instead of in createRequest #22449

Merged
merged 4 commits into from
Sep 28, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ describe('ReactDOMFizzServer', () => {
it('should error the stream when an error is thrown at the root', async () => {
const reportedErrors = [];
const {writable, output, completed} = getTestWritable();
ReactDOMFizzServer.pipeToNodeWritable(
const {startWriting} = ReactDOMFizzServer.pipeToNodeWritable(
<div>
<Throw />
</div>,
Expand All @@ -166,7 +166,8 @@ describe('ReactDOMFizzServer', () => {
},
);

// The stream is errored even if we haven't started writing.
// The stream is errored once we start writing.
startWriting();

await completed;

Expand Down
22 changes: 10 additions & 12 deletions packages/react-dom/src/server/ReactDOMFizzServerBrowser.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,15 @@ function renderToReadableStream(
children: ReactNodeList,
options?: Options,
): ReadableStream {
let request;
const request = createRequest(
children,
createResponseState(options ? options.identifierPrefix : undefined),
createRootFormatContext(options ? options.namespaceURI : undefined),
options ? options.progressiveChunkSize : undefined,
options ? options.onError : undefined,
options ? options.onCompleteAll : undefined,
options ? options.onCompleteShell : undefined,
);
if (options && options.signal) {
const signal = options.signal;
const listener = () => {
Expand All @@ -48,16 +56,6 @@ function renderToReadableStream(
}
const stream = new ReadableStream({
start(controller) {
request = createRequest(
children,
controller,
createResponseState(options ? options.identifierPrefix : undefined),
createRootFormatContext(options ? options.namespaceURI : undefined),
options ? options.progressiveChunkSize : undefined,
options ? options.onError : undefined,
options ? options.onCompleteAll : undefined,
options ? options.onCompleteShell : undefined,
);
startWork(request);
},
pull(controller) {
Expand All @@ -66,7 +64,7 @@ function renderToReadableStream(
// is actually used by something so we can give it the best result possible
// at that point.
if (stream.locked) {
startFlowing(request);
startFlowing(request, controller);
}
},
cancel(reason) {},
Expand Down
13 changes: 4 additions & 9 deletions packages/react-dom/src/server/ReactDOMFizzServerNode.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import {
} from './ReactDOMServerFormatConfig';

function createDrainHandler(destination, request) {
return () => startFlowing(request);
return () => startFlowing(request, destination);
}

type Options = {|
Expand All @@ -44,14 +44,9 @@ type Controls = {|
startWriting(): void,
|};

function createRequestImpl(
children: ReactNodeList,
destination: Writable,
options: void | Options,
) {
function createRequestImpl(children: ReactNodeList, options: void | Options) {
return createRequest(
children,
destination,
createResponseState(options ? options.identifierPrefix : undefined),
createRootFormatContext(options ? options.namespaceURI : undefined),
options ? options.progressiveChunkSize : undefined,
Expand All @@ -66,7 +61,7 @@ function pipeToNodeWritable(
destination: Writable,
options?: Options,
): Controls {
const request = createRequestImpl(children, destination, options);
const request = createRequestImpl(children, options);
let hasStartedFlowing = false;
startWork(request);
return {
Expand All @@ -75,7 +70,7 @@ function pipeToNodeWritable(
return;
}
hasStartedFlowing = true;
startFlowing(request);
startFlowing(request, destination);
destination.on('drain', createDrainHandler(destination, request));
},
abort() {
Expand Down
3 changes: 1 addition & 2 deletions packages/react-dom/src/server/ReactDOMLegacyServerBrowser.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ function renderToStringImpl(
}
const request = createRequest(
children,
destination,
createResponseState(
generateStaticMarkup,
options ? options.identifierPrefix : undefined,
Expand All @@ -74,7 +73,7 @@ function renderToStringImpl(
// If anything suspended and is still pending, we'll abort it before writing.
// That way we write only client-rendered boundaries from the start.
abort(request);
startFlowing(request);
startFlowing(request, destination);
if (didFatal) {
throw fatalError;
}
Expand Down
5 changes: 2 additions & 3 deletions packages/react-dom/src/server/ReactDOMLegacyServerNode.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ class ReactMarkupReadableStream extends Readable {

_read(size) {
if (this.startedFlowing) {
startFlowing(this.request);
startFlowing(this.request, this);
}
}
}
Expand All @@ -72,12 +72,11 @@ function renderToNodeStreamImpl(
// We wait until everything has loaded before starting to write.
// That way we only end up with fully resolved HTML even if we suspend.
destination.startedFlowing = true;
startFlowing(request);
startFlowing(request, destination);
}
const destination = new ReactMarkupReadableStream();
const request = createRequest(
children,
destination,
createResponseState(false, options ? options.identifierPrefix : undefined),
createRootFormatContext(),
Infinity,
Expand Down
3 changes: 1 addition & 2 deletions packages/react-noop-renderer/src/ReactNoopFlightServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,12 +63,11 @@ function render(model: ReactModel, options?: Options): Destination {
const bundlerConfig = undefined;
const request = ReactNoopFlightServer.createRequest(
model,
destination,
bundlerConfig,
options ? options.onError : undefined,
);
ReactNoopFlightServer.startWork(request);
ReactNoopFlightServer.startFlowing(request);
ReactNoopFlightServer.startFlowing(request, destination);
return destination;
}

Expand Down
3 changes: 1 addition & 2 deletions packages/react-noop-renderer/src/ReactNoopServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,6 @@ function render(children: React$Element<any>, options?: Options): Destination {
};
const request = ReactNoopServer.createRequest(
children,
destination,
null,
null,
options ? options.progressiveChunkSize : undefined,
Expand All @@ -268,7 +267,7 @@ function render(children: React$Element<any>, options?: Options): Destination {
options ? options.onCompleteShell : undefined,
);
ReactNoopServer.startWork(request);
ReactNoopServer.startFlowing(request);
ReactNoopServer.startFlowing(request, destination);
return destination;
}

Expand Down
3 changes: 1 addition & 2 deletions packages/react-server-dom-relay/src/ReactDOMServerFB.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ function renderToStream(children: ReactNodeList, options: Options): Stream {
};
const request = createRequest(
children,
destination,
createResponseState(options ? options.identifierPrefix : undefined),
createRootFormatContext(undefined),
options ? options.progressiveChunkSize : undefined,
Expand All @@ -71,7 +70,7 @@ function abortStream(stream: Stream): void {
function renderNextChunk(stream: Stream): string {
const {request, destination} = stream;
performWork(request);
startFlowing(request);
startFlowing(request, destination);
if (destination.fatal) {
throw destination.error;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,11 @@ function render(
): void {
const request = createRequest(
model,
destination,
config,
options ? options.onError : undefined,
);
startWork(request);
startFlowing(request);
startFlowing(request, destination);
}

export {render};
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,13 @@ function renderToReadableStream(
webpackMap: BundlerConfig,
options?: Options,
): ReadableStream {
let request;
const request = createRequest(
model,
webpackMap,
options ? options.onError : undefined,
);
const stream = new ReadableStream({
start(controller) {
request = createRequest(
model,
controller,
webpackMap,
options ? options.onError : undefined,
);
startWork(request);
},
pull(controller) {
Expand All @@ -42,7 +40,7 @@ function renderToReadableStream(
// is actually used by something so we can give it the best result possible
// at that point.
if (stream.locked) {
startFlowing(request);
startFlowing(request, controller);
}
},
cancel(reason) {},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {
} from 'react-server/src/ReactFlightServer';

function createDrainHandler(destination, request) {
return () => startFlowing(request);
return () => startFlowing(request, destination);
}

type Options = {
Expand All @@ -33,12 +33,11 @@ function pipeToNodeWritable(
): void {
const request = createRequest(
model,
destination,
webpackMap,
options ? options.onError : undefined,
);
startWork(request);
startFlowing(request);
startFlowing(request, destination);
destination.on('drain', createDrainHandler(destination, request));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ function render(
destination: Destination,
config: BundlerConfig,
): void {
const request = createRequest(model, destination, config);
const request = createRequest(model, config);
startWork(request);
startFlowing(request);
startFlowing(request, destination);
}

export {render};
Loading