Repository: facebook/react
Pull request: [Fizz] Encode external fizz runtime into chunks eagerly

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.

Changed files:
filename: packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js
patch: @@ -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;
 }

filename: packages/react-dom-bindings/src/server/ReactFizzConfigDOMLegacy.js
patch: @@ -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,

filename: packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
patch: @@ -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>