From d19472b65b34d313f8f0d45ce1e3db29d0f26300 Mon Sep 17 00:00:00 2001 From: Mofei Zhang Date: Fri, 9 Dec 2022 17:51:23 -0500 Subject: [PATCH] [Fizz] Readability refactor (follow up to #25437) --- .../src/server/ReactDOMServerFormatConfig.js | 389 +++++++++++------- .../src/__tests__/ReactDOMFizzServer-test.js | 9 +- .../src/__tests__/ReactDOMFloat-test.js | 9 +- .../react-dom/src/test-utils/FizzTestUtils.js | 8 - 4 files changed, 253 insertions(+), 162 deletions(-) diff --git a/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js b/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js index a7f1a20cc1f15..2dfecabfe0f93 100644 --- a/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js +++ b/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js @@ -2421,40 +2421,65 @@ export function writeCompletedSegmentInstruction( responseState: ResponseState, contentSegmentID: number, ): boolean { - const scriptFormat = + if ( !enableFizzExternalRuntime || - responseState.streamingFormat === ScriptStreamingFormat; - if (scriptFormat) { - writeChunk(destination, responseState.startInlineScript); - if (!responseState.sentCompleteSegmentFunction) { - // The first time we write this, we'll need to include the full implementation. - responseState.sentCompleteSegmentFunction = true; - writeChunk(destination, completeSegmentScript1Full); - } else { - // Future calls can just reuse the same function. - writeChunk(destination, completeSegmentScript1Partial); - } + responseState.streamingFormat === ScriptStreamingFormat + ) { + return writeCompletedSegmentInstructionScript( + destination, + responseState, + contentSegmentID, + ); + } else { + return writeCompletedSegmentInstructionData( + destination, + responseState, + contentSegmentID, + ); + } +} +function writeCompletedSegmentInstructionScript( + destination: Destination, + responseState: ResponseState, + contentSegmentID: number, +): boolean { + writeChunk(destination, responseState.startInlineScript); + if (!responseState.sentCompleteSegmentFunction) { + // The first time we write this, we'll need to include the full implementation. + responseState.sentCompleteSegmentFunction = true; + writeChunk(destination, completeSegmentScript1Full); } else { - writeChunk(destination, completeSegmentData1); + // Future calls can just reuse the same function. + writeChunk(destination, completeSegmentScript1Partial); } // Write function arguments, which are string literals writeChunk(destination, responseState.segmentPrefix); const formattedID = stringToChunk(contentSegmentID.toString(16)); writeChunk(destination, formattedID); - if (scriptFormat) { - writeChunk(destination, completeSegmentScript2); - } else { - writeChunk(destination, completeSegmentData2); - } + writeChunk(destination, completeSegmentScript2); + writeChunk(destination, responseState.placeholderPrefix); writeChunk(destination, formattedID); + return writeChunkAndReturn(destination, completeSegmentScriptEnd); +} - if (scriptFormat) { - return writeChunkAndReturn(destination, completeSegmentScriptEnd); - } else { - return writeChunkAndReturn(destination, completeSegmentDataEnd); - } +function writeCompletedSegmentInstructionData( + destination: Destination, + responseState: ResponseState, + contentSegmentID: number, +): boolean { + writeChunk(destination, completeSegmentData1); + + // Write function arguments, which are string literals + writeChunk(destination, responseState.segmentPrefix); + const formattedID = stringToChunk(contentSegmentID.toString(16)); + writeChunk(destination, formattedID); + writeChunk(destination, completeSegmentData2); + + writeChunk(destination, responseState.placeholderPrefix); + writeChunk(destination, formattedID); + return writeChunkAndReturn(destination, completeSegmentDataEnd); } const completeBoundaryScript1Full = stringToPrecomputedChunk( @@ -2492,43 +2517,62 @@ export function writeCompletedBoundaryInstruction( boundaryID: SuspenseBoundaryID, contentSegmentID: number, boundaryResources: BoundaryResources, +): boolean { + if ( + !enableFizzExternalRuntime || + responseState.streamingFormat === ScriptStreamingFormat + ) { + return writeCompletedBoundaryInstructionScript( + destination, + responseState, + boundaryID, + contentSegmentID, + boundaryResources, + ); + } else { + return writeCompletedBoundaryInstructionData( + destination, + responseState, + boundaryID, + contentSegmentID, + boundaryResources, + ); + } +} + +function writeCompletedBoundaryInstructionScript( + destination: Destination, + responseState: ResponseState, + boundaryID: SuspenseBoundaryID, + contentSegmentID: number, + boundaryResources: BoundaryResources, ): boolean { let hasStyleDependencies; if (enableFloat) { hasStyleDependencies = hasStyleResourceDependencies(boundaryResources); } - const scriptFormat = - !enableFizzExternalRuntime || - responseState.streamingFormat === ScriptStreamingFormat; - if (scriptFormat) { - writeChunk(destination, responseState.startInlineScript); - if (enableFloat && hasStyleDependencies) { - if (!responseState.sentCompleteBoundaryFunction) { - responseState.sentCompleteBoundaryFunction = true; - responseState.sentStyleInsertionFunction = true; - writeChunk( - destination, - clonePrecomputedChunk(completeBoundaryWithStylesScript1FullBoth), - ); - } else if (!responseState.sentStyleInsertionFunction) { - responseState.sentStyleInsertionFunction = true; - writeChunk(destination, completeBoundaryWithStylesScript1FullPartial); - } else { - writeChunk(destination, completeBoundaryWithStylesScript1Partial); - } + + writeChunk(destination, responseState.startInlineScript); + if (enableFloat && hasStyleDependencies) { + if (!responseState.sentCompleteBoundaryFunction) { + responseState.sentCompleteBoundaryFunction = true; + responseState.sentStyleInsertionFunction = true; + writeChunk( + destination, + clonePrecomputedChunk(completeBoundaryWithStylesScript1FullBoth), + ); + } else if (!responseState.sentStyleInsertionFunction) { + responseState.sentStyleInsertionFunction = true; + writeChunk(destination, completeBoundaryWithStylesScript1FullPartial); } else { - if (!responseState.sentCompleteBoundaryFunction) { - responseState.sentCompleteBoundaryFunction = true; - writeChunk(destination, completeBoundaryScript1Full); - } else { - writeChunk(destination, completeBoundaryScript1Partial); - } + writeChunk(destination, completeBoundaryWithStylesScript1Partial); } } else { - if (enableFloat && hasStyleDependencies) { - writeChunk(destination, completeBoundaryWithStylesData1); + if (!responseState.sentCompleteBoundaryFunction) { + responseState.sentCompleteBoundaryFunction = true; + writeChunk(destination, completeBoundaryScript1Full); } else { - writeChunk(destination, completeBoundaryData1); + writeChunk(destination, completeBoundaryScript1Partial); } } @@ -2541,37 +2585,61 @@ export function writeCompletedBoundaryInstruction( // Write function arguments, which are string and array literals const formattedContentID = stringToChunk(contentSegmentID.toString(16)); writeChunk(destination, boundaryID); - if (scriptFormat) { - writeChunk(destination, completeBoundaryScript2); - } else { - writeChunk(destination, completeBoundaryData2); - } + writeChunk(destination, completeBoundaryScript2); + writeChunk(destination, responseState.segmentPrefix); writeChunk(destination, formattedContentID); + if (enableFloat && hasStyleDependencies) { - // Script and data writers must format this differently: // - script writer emits an array literal, whose string elements are // escaped for javascript e.g. ["A", "B"] - // - data writer emits a string literal, which is escaped as html - // e.g. ["A", "B"] - if (scriptFormat) { - writeChunk(destination, completeBoundaryScript3a); - // boundaryResources encodes an array literal - writeStyleResourceDependenciesInJS(destination, boundaryResources); - } else { - writeChunk(destination, completeBoundaryData3a); - writeStyleResourceDependenciesInAttr(destination, boundaryResources); - } + writeChunk(destination, completeBoundaryScript3a); + // boundaryResources encodes an array literal + writeStyleResourceDependenciesInJS(destination, boundaryResources); } else { - if (scriptFormat) { - writeChunk(destination, completeBoundaryScript3b); - } + writeChunk(destination, completeBoundaryScript3b); } - if (scriptFormat) { - return writeChunkAndReturn(destination, completeBoundaryScriptEnd); + return writeChunkAndReturn(destination, completeBoundaryScriptEnd); +} + +function writeCompletedBoundaryInstructionData( + destination: Destination, + responseState: ResponseState, + boundaryID: SuspenseBoundaryID, + contentSegmentID: number, + boundaryResources: BoundaryResources, +): boolean { + let hasStyleDependencies; + if (enableFloat) { + hasStyleDependencies = hasStyleResourceDependencies(boundaryResources); + } + if (enableFloat && hasStyleDependencies) { + writeChunk(destination, completeBoundaryWithStylesData1); } else { - return writeChunkAndReturn(destination, completeBoundaryDataEnd); + writeChunk(destination, completeBoundaryData1); + } + + if (boundaryID === null) { + throw new Error( + 'An ID must have been assigned before we can complete the boundary.', + ); + } + + // Write function arguments, which are string and array literals + const formattedContentID = stringToChunk(contentSegmentID.toString(16)); + writeChunk(destination, boundaryID); + writeChunk(destination, completeBoundaryData2); + + writeChunk(destination, responseState.segmentPrefix); + writeChunk(destination, formattedContentID); + + if (enableFloat && hasStyleDependencies) { + // - data writer emits a string literal, which is escaped as html + // e.g. ["A", "B"] + writeChunk(destination, completeBoundaryData3a); + writeStyleResourceDependenciesInAttr(destination, boundaryResources); } + return writeChunkAndReturn(destination, completeBoundaryDataEnd); } const clientRenderScript1Full = stringToPrecomputedChunk( @@ -2598,22 +2666,46 @@ export function writeClientRenderBoundaryInstruction( errorMessage?: string, errorComponentStack?: string, ): boolean { - const scriptFormat = + if ( !enableFizzExternalRuntime || - responseState.streamingFormat === ScriptStreamingFormat; - if (scriptFormat) { - writeChunk(destination, responseState.startInlineScript); - if (!responseState.sentClientRenderFunction) { - // The first time we write this, we'll need to include the full implementation. - responseState.sentClientRenderFunction = true; - writeChunk(destination, clientRenderScript1Full); - } else { - // Future calls can just reuse the same function. - writeChunk(destination, clientRenderScript1Partial); - } + responseState.streamingFormat === ScriptStreamingFormat + ) { + return writeClientRenderBoundaryInstructionScript( + destination, + responseState, + boundaryID, + errorDigest, + errorMessage, + errorComponentStack, + ); } else { - // - return writeChunkAndReturn(destination, clientRenderDataEnd); +function writeClientRenderBoundaryInstructionData( + destination: Destination, + responseState: ResponseState, + boundaryID: SuspenseBoundaryID, + errorDigest: ?string, + errorMessage?: string, + errorComponentStack?: string, +): boolean { + // + return writeChunkAndReturn(destination, clientRenderDataEnd); } const regexForJSStringsInInstructionScripts = /[<\u2028\u2029]/g; diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index 79455a7225df8..38705e7d843ae 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -10,7 +10,6 @@ 'use strict'; import { replaceScriptsAndMove, - mergeOptions, stripExternalRuntimeInNodes, withLoadingReadyState, } from '../test-utils/FizzTestUtils'; @@ -300,10 +299,10 @@ describe('ReactDOMFizzServer', () => { } function renderToPipeableStream(jsx, options) { // Merge options with renderOptions, which may contain featureFlag specific behavior - return ReactDOMFizzServer.renderToPipeableStream( - jsx, - mergeOptions(options, renderOptions), - ); + return ReactDOMFizzServer.renderToPipeableStream(jsx, { + ...renderOptions, + ...options, + }); } it('should asynchronously load a lazy component', async () => { diff --git a/packages/react-dom/src/__tests__/ReactDOMFloat-test.js b/packages/react-dom/src/__tests__/ReactDOMFloat-test.js index b216700671cca..74e2417f998a6 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFloat-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFloat-test.js @@ -10,7 +10,6 @@ 'use strict'; import { replaceScriptsAndMove, - mergeOptions, withLoadingReadyState, } from '../test-utils/FizzTestUtils'; @@ -251,10 +250,10 @@ describe('ReactDOMFloat', () => { function renderToPipeableStream(jsx, options) { // Merge options with renderOptions, which may contain featureFlag specific behavior - return ReactDOMFizzServer.renderToPipeableStream( - jsx, - mergeOptions(options, renderOptions), - ); + return ReactDOMFizzServer.renderToPipeableStream(jsx, { + ...renderOptions, + ...options, + }); } // @gate enableFloat diff --git a/packages/react-dom/src/test-utils/FizzTestUtils.js b/packages/react-dom/src/test-utils/FizzTestUtils.js index 1df5ca947e7c0..c27cb396fdb66 100644 --- a/packages/react-dom/src/test-utils/FizzTestUtils.js +++ b/packages/react-dom/src/test-utils/FizzTestUtils.js @@ -133,13 +133,6 @@ async function replaceScriptsAndMove( } } -function mergeOptions(options: Object, defaultOptions: Object): Object { - return { - ...defaultOptions, - ...options, - }; -} - function stripExternalRuntimeInNodes( nodes: HTMLElement[] | HTMLCollection, externalRuntimeSrc: string | null, @@ -192,7 +185,6 @@ async function withLoadingReadyState( export { replaceScriptsAndMove, - mergeOptions, stripExternalRuntimeInNodes, withLoadingReadyState, };