Optimizing the way external fizz runtime is instantiated so we don't …

…have to carry the nonce around on ResponseState
facebook · May 1, 2023 · 9a0fa75 · 9a0fa75
1 parent 9545e48
commit 9a0fa75
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 30 deletions.
diff --git 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
@@ -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
@@ -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>