From 8b47a3cd90d7e9a0091018b7bcc48bb9ecb1fda8 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Thu, 23 Sep 2021 21:18:09 -0400 Subject: [PATCH 1/5] Remove pushEmpty This is only used to support the assignID mechanism. --- .../src/server/ReactDOMServerFormatConfig.js | 10 ---- .../ReactDOMServerLegacyFormatConfig.js | 1 - .../server/ReactNativeServerFormatConfig.js | 8 ---- packages/react-server/src/ReactFizzServer.js | 48 +++---------------- .../forks/ReactServerFormatConfig.custom.js | 1 - 5 files changed, 6 insertions(+), 62 deletions(-) diff --git a/packages/react-dom/src/server/ReactDOMServerFormatConfig.js b/packages/react-dom/src/server/ReactDOMServerFormatConfig.js index 1b2bbc28f6669..b02b6cc085caf 100644 --- a/packages/react-dom/src/server/ReactDOMServerFormatConfig.js +++ b/packages/react-dom/src/server/ReactDOMServerFormatConfig.js @@ -224,16 +224,6 @@ function pushDummyNodeWithID( target.push(dummyNode1, id, dummyNode2); } -export function pushEmpty( - target: Array, - responseState: ResponseState, - assignID: null | SuspenseBoundaryID, -): void { - if (assignID !== null) { - pushDummyNodeWithID(target, responseState, assignID); - } -} - const textSeparator = stringToPrecomputedChunk(''); export function pushTextInstance( diff --git a/packages/react-dom/src/server/ReactDOMServerLegacyFormatConfig.js b/packages/react-dom/src/server/ReactDOMServerLegacyFormatConfig.js index 60ab7544d88fa..caca8988f8643 100644 --- a/packages/react-dom/src/server/ReactDOMServerLegacyFormatConfig.js +++ b/packages/react-dom/src/server/ReactDOMServerLegacyFormatConfig.js @@ -83,7 +83,6 @@ export { getChildFormatContext, createSuspenseBoundaryID, makeServerID, - pushEmpty, pushStartInstance, pushEndInstance, pushStartCompletedSuspenseBoundary, diff --git a/packages/react-native-renderer/src/server/ReactNativeServerFormatConfig.js b/packages/react-native-renderer/src/server/ReactNativeServerFormatConfig.js index 24e9da45423d2..8b16e37667395 100644 --- a/packages/react-native-renderer/src/server/ReactNativeServerFormatConfig.js +++ b/packages/react-native-renderer/src/server/ReactNativeServerFormatConfig.js @@ -125,14 +125,6 @@ export function makeServerID( const RAW_TEXT = stringToPrecomputedChunk('RCTRawText'); -export function pushEmpty( - target: Array, - responseState: ResponseState, - assignID: null | SuspenseBoundaryID, -): void { - // This is not used since we don't need to assign any IDs. -} - export function pushTextInstance( target: Array, text: string, diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index f19f45e41f182..c61133e2d84d8 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -48,7 +48,6 @@ import { writeClientRenderBoundaryInstruction, writeCompletedBoundaryInstruction, writeCompletedSegmentInstruction, - pushEmpty, pushTextInstance, pushStartInstance, pushEndInstance, @@ -421,9 +420,6 @@ function renderSuspenseBoundary( const parentBoundary = task.blockedBoundary; const parentSegment = task.blockedSegment; - // We need to push an "empty" thing here to identify the parent suspense boundary. - pushEmpty(parentSegment.chunks, request.responseState, task.assignID); - task.assignID = null; // Each time we enter a suspense boundary, we split out into a new segment for // the fallback so that we can later replace that segment with the content. // This also lets us split out the main content even if it doesn't suspend, @@ -488,26 +484,13 @@ function renderSuspenseBoundary( task.blockedSegment = parentSegment; } - // This injects an extra segment just to contain an empty tag with an ID. - // This means that we're not actually using the assignID anywhere. - // TODO: Rethink the assignID approach. - pushEmpty(boundarySegment.chunks, request.responseState, newBoundary.id); - const innerSegment = createPendingSegment( - request, - boundarySegment.chunks.length, - null, - boundarySegment.formatContext, - ); - boundarySegment.status = COMPLETED; - boundarySegment.children.push(innerSegment); - // We create suspended task for the fallback because we don't want to actually work // on it yet in case we finish the main content, so we queue for later. const suspendedFallbackTask = createTask( request, fallback, parentBoundary, - innerSegment, + boundarySegment, fallbackAbortSet, task.legacyContext, task.context, @@ -1143,20 +1126,11 @@ function renderNodeDestructive( } if (isArray(node)) { - if (node.length > 0) { - for (let i = 0; i < node.length; i++) { - // Recursively render the rest. We need to use the non-destructive form - // so that we can safely pop back up and render the sibling if something - // suspends. - renderNode(request, task, node[i]); - } - } else { - pushEmpty( - task.blockedSegment.chunks, - request.responseState, - task.assignID, - ); - task.assignID = null; + for (let i = 0; i < node.length; i++) { + // Recursively render the rest. We need to use the non-destructive form + // so that we can safely pop back up and render the sibling if something + // suspends. + renderNode(request, task, node[i]); } return; } @@ -1181,12 +1155,6 @@ function renderNodeDestructive( return; } } - pushEmpty( - task.blockedSegment.chunks, - request.responseState, - task.assignID, - ); - task.assignID = null; } const childString = Object.prototype.toString.call(node); @@ -1232,10 +1200,6 @@ function renderNodeDestructive( ); } } - - // Any other type is assumed to be empty. - pushEmpty(task.blockedSegment.chunks, request.responseState, task.assignID); - task.assignID = null; } function spawnNewSuspendedTask( diff --git a/packages/react-server/src/forks/ReactServerFormatConfig.custom.js b/packages/react-server/src/forks/ReactServerFormatConfig.custom.js index 22539f3aa86bf..92e209f75e2b6 100644 --- a/packages/react-server/src/forks/ReactServerFormatConfig.custom.js +++ b/packages/react-server/src/forks/ReactServerFormatConfig.custom.js @@ -35,7 +35,6 @@ export const isPrimaryRenderer = false; export const getChildFormatContext = $$$hostConfig.getChildFormatContext; export const createSuspenseBoundaryID = $$$hostConfig.createSuspenseBoundaryID; export const makeServerID = $$$hostConfig.makeServerID; -export const pushEmpty = $$$hostConfig.pushEmpty; export const pushTextInstance = $$$hostConfig.pushTextInstance; export const pushStartInstance = $$$hostConfig.pushStartInstance; export const pushEndInstance = $$$hostConfig.pushEndInstance; From f0bb021d062d10c6a6b157e3511b35021c12c2fd Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Thu, 23 Sep 2021 21:25:51 -0400 Subject: [PATCH 2/5] Remove assignID mechanism This effectively isn't used anyway because we always insert a dummy tag into the fallback. --- .../src/server/ReactDOMServerFormatConfig.js | 102 ++---------------- .../ReactDOMServerLegacyFormatConfig.js | 8 +- .../server/ReactNativeServerFormatConfig.js | 2 - packages/react-server/src/ReactFizzServer.js | 21 +--- 4 files changed, 14 insertions(+), 119 deletions(-) diff --git a/packages/react-dom/src/server/ReactDOMServerFormatConfig.js b/packages/react-dom/src/server/ReactDOMServerFormatConfig.js index b02b6cc085caf..6c0bf17c5bcda 100644 --- a/packages/react-dom/src/server/ReactDOMServerFormatConfig.js +++ b/packages/react-dom/src/server/ReactDOMServerFormatConfig.js @@ -230,11 +230,7 @@ export function pushTextInstance( target: Array, text: string, responseState: ResponseState, - assignID: null | SuspenseBoundaryID, ): void { - if (assignID !== null) { - pushDummyNodeWithID(target, responseState, assignID); - } if (text === '') { // Empty text doesn't have a DOM node representation and the hydration is aware of this. return; @@ -588,7 +584,6 @@ function pushStartSelect( target: Array, props: Object, responseState: ResponseState, - assignID: null | SuspenseBoundaryID, ): ReactNodeList { if (__DEV__) { checkControlledValueProps('select', props); @@ -641,9 +636,6 @@ function pushStartSelect( } } } - if (assignID !== null) { - pushID(target, responseState, assignID, props.id); - } target.push(endOfStartTag); pushInnerHTML(target, innerHTML, children); @@ -683,7 +675,6 @@ function pushStartOption( props: Object, responseState: ResponseState, formatContext: FormatContext, - assignID: null | SuspenseBoundaryID, ): ReactNodeList { const selectedValue = formatContext.selectedValue; @@ -766,10 +757,6 @@ function pushStartOption( target.push(selectedMarkerAttribute); } - if (assignID !== null) { - pushID(target, responseState, assignID, props.id); - } - target.push(endOfStartTag); pushInnerHTML(target, innerHTML, children); return children; @@ -779,7 +766,6 @@ function pushInput( target: Array, props: Object, responseState: ResponseState, - assignID: null | SuspenseBoundaryID, ): ReactNodeList { if (__DEV__) { checkControlledValueProps('input', props); @@ -873,10 +859,6 @@ function pushInput( pushAttribute(target, responseState, 'value', defaultValue); } - if (assignID !== null) { - pushID(target, responseState, assignID, props.id); - } - target.push(endOfStartTagSelfClosing); return null; } @@ -885,7 +867,6 @@ function pushStartTextArea( target: Array, props: Object, responseState: ResponseState, - assignID: null | SuspenseBoundaryID, ): ReactNodeList { if (__DEV__) { checkControlledValueProps('textarea', props); @@ -942,10 +923,6 @@ function pushStartTextArea( value = defaultValue; } - if (assignID !== null) { - pushID(target, responseState, assignID, props.id); - } - target.push(endOfStartTag); // TODO (yungsters): Remove support for children content in