From 8ea96ef84d8f08ed1846dec9e8ed20d2225db0d3 Mon Sep 17 00:00:00 2001
From: Josh Story <story@hey.com>
Date: Mon, 1 May 2023 10:50:36 -0700
Subject: [PATCH] [Fizz] Encode external fizz runtime into chunks eagerly
 (#26752)

in https://github.com/facebook/react/pull/26738 we added nonce to the
ResponseState. Initially it was used in a variety of places but the
version that got merged only included it with the external fizz runtime.
This PR updates the config for the external fizz runtime so that the
nonce is encoded into the script chunks at request creation time.

The rationale is that for live-requests, streaming is more likely than
not so doing the encoding work at the start is better than during flush.
For cases such as SSG where the runtime is not required the extra
encoding is tolerable (not a live request). Bots are an interesting case
because if you want fastest TTFB you will end up requiring the runtime
but if you are withholding until the stream is done you have already
sacrificed fastest TTFB and the marginal slowdown of the extraneous
encoding is hopefully neglibible

I'm writing this so later if we learn that this tradeoff isn't worth it
we at least understand why I made the change in the first place.
---
 .../src/server/ReactFizzConfigDOM.js          | 56 ++++++++++---------
 .../src/server/ReactFizzConfigDOMLegacy.js    |  7 +--
 .../src/__tests__/ReactDOMFizzServer-test.js  |  2 +-
 3 files changed, 35 insertions(+), 30 deletions(-)

diff --git a/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js b/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js
index f4fae4d307..646e61608a 100644
--- a/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js
+++ b/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js
@@ -130,11 +130,8 @@ export type ResponseState = {
   startInlineScript: PrecomputedChunk,
   instructions: InstructionState,
 
-  // state for outputting CSP nonce
-  nonce: string | void,
-
   // state for data streaming format
-  externalRuntimeConfig: BootstrapScriptDescriptor | null,
+  externalRuntimeScript: null | ExternalRuntimeScript,
 
   // preamble and postamble chunks and state
   htmlChunks: null | Array<Chunk | PrecomputedChunk>,
@@ -196,6 +193,10 @@ export type BootstrapScriptDescriptor = {
   src: string,
   integrity?: string,
 };
+export type ExternalRuntimeScript = {
+  src: string,
+  chunks: Array<Chunk | PrecomputedChunk>,
+};
 // Allows us to keep track of what we've already written so we can refer back to it.
 // if passed externalRuntimeConfig and the enableFizzExternalRuntime feature flag
 // is set, the server will send instructions via data attributes (instead of inline scripts)
@@ -215,7 +216,7 @@ export function createResponseState(
           '<script nonce="' + escapeTextForBrowser(nonce) + '">',
         );
   const bootstrapChunks: Array<Chunk | PrecomputedChunk> = [];
-  let externalRuntimeDesc = null;
+  let externalRuntimeScript: null | ExternalRuntimeScript = null;
   let streamingFormat = ScriptStreamingFormat;
   if (bootstrapScriptContent !== undefined) {
     bootstrapChunks.push(
@@ -233,12 +234,27 @@ export function createResponseState(
     if (externalRuntimeConfig !== undefined) {
       streamingFormat = DataStreamingFormat;
       if (typeof externalRuntimeConfig === 'string') {
-        externalRuntimeDesc = {
+        externalRuntimeScript = {
           src: externalRuntimeConfig,
-          integrity: undefined,
+          chunks: [],
         };
+        pushScriptImpl(externalRuntimeScript.chunks, {
+          src: externalRuntimeConfig,
+          async: true,
+          integrity: undefined,
+          nonce: nonce,
+        });
       } else {
-        externalRuntimeDesc = externalRuntimeConfig;
+        externalRuntimeScript = {
+          src: externalRuntimeConfig.src,
+          chunks: [],
+        };
+        pushScriptImpl(externalRuntimeScript.chunks, {
+          src: externalRuntimeConfig.src,
+          async: true,
+          integrity: externalRuntimeConfig.integrity,
+          nonce: nonce,
+        });
       }
     }
   }
@@ -307,7 +323,7 @@ export function createResponseState(
     streamingFormat,
     startInlineScript: inlineScriptWithNonce,
     instructions: NothingSent,
-    externalRuntimeConfig: externalRuntimeDesc,
+    externalRuntimeScript,
     htmlChunks: null,
     headChunks: null,
     hasBody: false,
@@ -1293,7 +1309,7 @@ function injectFormReplayingRuntime(responseState: ResponseState): void {
   // to emit anything. It's always used.
   if (
     (responseState.instructions & SentFormReplayingRuntime) === NothingSent &&
-    (!enableFizzExternalRuntime || !responseState.externalRuntimeConfig)
+    (!enableFizzExternalRuntime || !responseState.externalRuntimeScript)
   ) {
     responseState.instructions |= SentFormReplayingRuntime;
     responseState.bootstrapChunks.unshift(
@@ -4078,15 +4094,15 @@ export function writePreamble(
   if (
     enableFizzExternalRuntime &&
     !willFlushAllSegments &&
-    responseState.externalRuntimeConfig
+    responseState.externalRuntimeScript
   ) {
     // If the root segment is incomplete due to suspended tasks
     // (e.g. willFlushAllSegments = false) and we are using data
     // streaming format, ensure the external runtime is sent.
     // (User code could choose to send this even earlier by calling
     //  preinit(...), if they know they will suspend).
-    const {src, integrity} = responseState.externalRuntimeConfig;
-    internalPreinitScript(resources, src, integrity, responseState.nonce);
+    const {src, chunks} = responseState.externalRuntimeScript;
+    internalPreinitScript(resources, src, chunks);
   }
 
   const htmlChunks = responseState.htmlChunks;
@@ -5362,32 +5378,22 @@ function preinit(href: string, options: PreinitOptions): void {
   }
 }
 
-// This method is trusted. It must only be called from within this codebase and it assumes the arguments
-// conform to the types because no user input is being passed in. It also assumes that it is being called as
-// part of a work or flush loop and therefore does not need to request Fizz to flush Resources.
 function internalPreinitScript(
   resources: Resources,
   src: string,
-  integrity: ?string,
-  nonce: ?string,
+  chunks: Array<Chunk | PrecomputedChunk>,
 ): void {
   const key = getResourceKey('script', src);
   let resource = resources.scriptsMap.get(key);
   if (!resource) {
     resource = {
       type: 'script',
-      chunks: [],
+      chunks,
       state: NoState,
       props: null,
     };
     resources.scriptsMap.set(key, resource);
     resources.scripts.add(resource);
-    pushScriptImpl(resource.chunks, {
-      async: true,
-      src,
-      integrity,
-      nonce,
-    });
   }
   return;
 }
diff --git a/packages/react-dom-bindings/src/server/ReactFizzConfigDOMLegacy.js b/packages/react-dom-bindings/src/server/ReactFizzConfigDOMLegacy.js
index bc2e93706c..5e1be3c91b 100644
--- a/packages/react-dom-bindings/src/server/ReactFizzConfigDOMLegacy.js
+++ b/packages/react-dom-bindings/src/server/ReactFizzConfigDOMLegacy.js
@@ -9,6 +9,7 @@
 
 import type {
   BootstrapScriptDescriptor,
+  ExternalRuntimeScript,
   FormatContext,
   StreamingFormat,
   InstructionState,
@@ -48,7 +49,7 @@ export type ResponseState = {
   streamingFormat: StreamingFormat,
   startInlineScript: PrecomputedChunk,
   instructions: InstructionState,
-  externalRuntimeConfig: BootstrapScriptDescriptor | null,
+  externalRuntimeScript: null | ExternalRuntimeScript,
   htmlChunks: null | Array<Chunk | PrecomputedChunk>,
   headChunks: null | Array<Chunk | PrecomputedChunk>,
   hasBody: boolean,
@@ -57,7 +58,6 @@ export type ResponseState = {
   preloadChunks: Array<Chunk | PrecomputedChunk>,
   hoistableChunks: Array<Chunk | PrecomputedChunk>,
   stylesToHoist: boolean,
-  nonce: string | void,
   // This is an extra field for the legacy renderer
   generateStaticMarkup: boolean,
 };
@@ -86,7 +86,7 @@ export function createResponseState(
     streamingFormat: responseState.streamingFormat,
     startInlineScript: responseState.startInlineScript,
     instructions: responseState.instructions,
-    externalRuntimeConfig: responseState.externalRuntimeConfig,
+    externalRuntimeScript: responseState.externalRuntimeScript,
     htmlChunks: responseState.htmlChunks,
     headChunks: responseState.headChunks,
     hasBody: responseState.hasBody,
@@ -95,7 +95,6 @@ export function createResponseState(
     preloadChunks: responseState.preloadChunks,
     hoistableChunks: responseState.hoistableChunks,
     stylesToHoist: responseState.stylesToHoist,
-    nonce: responseState.nonce,
 
     // This is an extra field for the legacy renderer
     generateStaticMarkup,
diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
index 1ba04fb044..f4efd7959e 100644
--- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
+++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
@@ -3767,7 +3767,7 @@ describe('ReactDOMFizzServer', () => {
       Array.from(document.head.getElementsByTagName('script')).map(
         n => n.outerHTML,
       ),
-    ).toEqual(['<script async="" src="src-of-external-runtime"></script>']);
+    ).toEqual(['<script src="src-of-external-runtime" async=""></script>']);
 
     expect(getVisibleChildren(document)).toEqual(
       <html>