Import maps need to be emitted before any scripts or preloads so the browser can properly locate these resources. This change makes React aware of the concept of import maps and emits them before scripts and modules and their preloads.

gnoff · gnoff · commit 53dcbb26baf5 · 2023-08-21T16:51:32.000-07:00
diff --git a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js
@@ -1968,7 +1968,7 @@ export function clearSingleton(instance: Instance): void {
 
 export const supportsResources = true;
 
-type HoistableTagType = 'link' | 'meta' | 'title';
+type HoistableTagType = 'link' | 'meta' | 'title' | 'script';
 type TResource<
   T: 'stylesheet' | 'style' | 'script' | 'void',
   S: null | {...},
@@ -2759,7 +2759,12 @@ export function getResource(
       return null;
     }
     case 'script': {
-      if (typeof pendingProps.src === 'string' && pendingProps.async === true) {
+      if (pendingProps.type === 'importmap') {
+        return null;
+      } else if (
+        typeof pendingProps.src === 'string' &&
+        pendingProps.async === true
+      ) {
         const scriptProps: ScriptProps = pendingProps;
         const key = getScriptKey(scriptProps.src);
         const scripts = getResourcesFromRoot(resourceRoot).hoistableScripts;
@@ -3110,21 +3115,21 @@ export function hydrateHoistable(
     case 'title': {
       instance = ownerDocument.getElementsByTagName('title')[0];
       if (
-        !instance ||
-        isOwnedInstance(instance) ||
-        instance.namespaceURI === SVG_NAMESPACE ||
-        instance.hasAttribute('itemprop')
+        instance &&
+        !isOwnedInstance(instance) &&
+        instance.namespaceURI !== SVG_NAMESPACE &&
+        !instance.hasAttribute('itemprop')
       ) {
-        instance = ownerDocument.createElement(type);
-        (ownerDocument.head: any).insertBefore(
-          instance,
-          ownerDocument.querySelector('head > title'),
-        );
+        setInitialProperties(instance, type, props);
+        break;
       }
+      instance = ownerDocument.createElement(type);
       setInitialProperties(instance, type, props);
-      precacheFiberNode(internalInstanceHandle, instance);
-      markNodeAsHoistable(instance);
-      return instance;
+      (ownerDocument.head: any).insertBefore(
+        instance,
+        ownerDocument.querySelector('head > title'),
+      );
+      break;
     }
     case 'link': {
       const cache = getHydratableHoistableCache('link', 'href', ownerDocument);
@@ -3201,6 +3206,17 @@ export function hydrateHoistable(
       (ownerDocument.head: any).appendChild(instance);
       break;
     }
+    case 'script': {
+      // the only hoistable script is type="importmap" and there should only be 1
+      instance = ownerDocument.querySelector('script[type="importmap"]');
+      if (instance && !isOwnedInstance(instance)) {
+        break;
+      }
+      instance = ownerDocument.createElement(type);
+      setInitialProperties(instance, type, props);
+      (ownerDocument.head: any).appendChild(instance);
+      break;
+    }
     default:
       throw new Error(
         `getNodesForType encountered a type it did not expect: "${type}". This is a bug in React.`,
@@ -3406,7 +3422,9 @@ export function isHostHoistableType(
       }
     }
     case 'script': {
-      if (
+      if (props.type === 'importmap') {
+        return true;
+      } else if (
         props.async !== true ||
         props.onLoad ||
         props.onError ||
@@ -3436,8 +3454,9 @@ export function isHostHoistableType(
           }
         }
         return false;
+      } else {
+        return true;
       }
-      return true;
     }
     case 'noscript':
     case 'template': {
diff --git a/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js b/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js
@@ -142,6 +142,7 @@ export type ResponseState = {
   // Hoistable chunks
   charsetChunks: Array<Chunk | PrecomputedChunk>,
   preconnectChunks: Array<Chunk | PrecomputedChunk>,
+  importMapChunks: Array<Chunk | PrecomputedChunk>,
   preloadChunks: Array<Chunk | PrecomputedChunk>,
   hoistableChunks: Array<Chunk | PrecomputedChunk>,
 
@@ -361,6 +362,7 @@ export function createResponseState(
     hasBody: false,
     charsetChunks: [],
     preconnectChunks: [],
+    importMapChunks: [],
     preloadChunks: [],
     hoistableChunks: [],
     stylesToHoist: false,
@@ -2701,12 +2703,17 @@ function pushStartHtml(
 function pushScript(
   target: Array<Chunk | PrecomputedChunk>,
   props: Object,
+  responseState: ResponseState,
   resources: Resources,
   textEmbedded: boolean,
   insertionMode: InsertionMode,
   noscriptTagInScope: boolean,
 ): null {
   if (enableFloat) {
+    if (props.type === 'importmap') {
+      return pushScriptImpl(responseState.importMapChunks, props);
+    }
+
     const asyncProp = props.async;
     if (
       typeof props.src !== 'string' ||
@@ -3122,6 +3129,7 @@ export function pushStartInstance(
         ? pushScript(
             target,
             props,
+            responseState,
             resources,
             textEmbedded,
             formatContext.insertionMode,
@@ -4231,6 +4239,12 @@ export function writePreamble(
   // Flush unblocked stylesheets by precedence
   resources.precedences.forEach(flushAllStylesInPreamble, destination);
 
+  const importMapChunks = responseState.importMapChunks;
+  for (i = 0; i < importMapChunks.length; i++) {
+    writeChunk(destination, importMapChunks[i]);
+  }
+  importMapChunks.length = 0;
+
   resources.bootstrapScripts.forEach(flushResourceInPreamble, destination);
 
   resources.scripts.forEach(flushResourceInPreamble, destination);
@@ -4301,6 +4315,13 @@ export function writeHoistables(
   // but we want to kick off preloading as soon as possible
   resources.precedences.forEach(preloadLateStyles, destination);
 
+  // Import maps should have already flushed. If they haven't there is a chance we still haven't emitted any preloads for scripts
+  // or modules so we will emit them now
+  const importMapChunks = responseState.importMapChunks;
+  for (i = 0; i < importMapChunks.length; i++) {
+    writeChunk(destination, importMapChunks[i]);
+  }
+
   // bootstrap scripts should flush above script priority but these can only flush in the preamble
   // so we elide the code here for performance
 
diff --git a/packages/react-dom-bindings/src/server/ReactFizzConfigDOMLegacy.js b/packages/react-dom-bindings/src/server/ReactFizzConfigDOMLegacy.js
@@ -56,6 +56,7 @@ export type ResponseState = {
   hasBody: boolean,
   charsetChunks: Array<Chunk | PrecomputedChunk>,
   preconnectChunks: Array<Chunk | PrecomputedChunk>,
+  importMapChunks: Array<Chunk | PrecomputedChunk>,
   preloadChunks: Array<Chunk | PrecomputedChunk>,
   hoistableChunks: Array<Chunk | PrecomputedChunk>,
   stylesToHoist: boolean,
@@ -95,6 +96,7 @@ export function createResponseState(
     hasBody: responseState.hasBody,
     charsetChunks: responseState.charsetChunks,
     preconnectChunks: responseState.preconnectChunks,
+    importMapChunks: responseState.importMapChunks,
     preloadChunks: responseState.preloadChunks,
     hoistableChunks: responseState.hoistableChunks,
     stylesToHoist: responseState.stylesToHoist,
diff --git a/packages/react-dom/src/__tests__/ReactDOMFloat-test.js b/packages/react-dom/src/__tests__/ReactDOMFloat-test.js
@@ -3999,6 +3999,96 @@ body {
     );
   });
 
+  it('hoists <script type="importmap"> tags above scripts and preloads', async () => {
+    function App() {
+      return (
+        <html>
+          <body>
+            hello
+            <script async={true} src="foo" />
+            <script type="importmap" data-meaningful="">
+              first map
+            </script>
+            <script type="importmap" data-meaningful="">
+              second map (two maps do not make sense but we are asserting they
+              flush anyway)
+            </script>
+            <script src="sync script" data-meaningful="" />
+          </body>
+        </html>
+      );
+    }
+
+    await act(() => {
+      renderToPipeableStream(<App />).pipe(writable);
+    });
+
+    expect(getMeaningfulChildren(document)).toEqual(
+      <html>
+        <head>
+          <script type="importmap" data-meaningful="">
+            first map
+          </script>
+          <script type="importmap" data-meaningful="">
+            second map (two maps do not make sense but we are asserting they
+            flush anyway)
+          </script>
+          <script async="" src="foo" />
+        </head>
+        <body>
+          hello
+          <script src="sync script" data-meaningful="" />
+        </body>
+      </html>,
+    );
+
+    const root = ReactDOMClient.hydrateRoot(document, <App />);
+    await waitForAll([]);
+
+    expect(getMeaningfulChildren(document)).toEqual(
+      <html>
+        <head>
+          <script type="importmap" data-meaningful="">
+            first map
+          </script>
+          <script type="importmap" data-meaningful="">
+            second map (two maps do not make sense but we are asserting they
+            flush anyway)
+          </script>
+          <script async="" src="foo" />
+          {/* The reason we see a second copy of this map is we assume there
+          will only be one and thus only hydrate the first one we encounter.
+          All subsequent importmap hoistables will be considered new on the client
+          This is similar to how <title> hydrates as well */}
+          <script type="importmap" data-meaningful="">
+            second map (two maps do not make sense but we are asserting they
+            flush anyway)
+          </script>
+        </head>
+        <body>
+          hello
+          <script src="sync script" data-meaningful="" />
+        </body>
+      </html>,
+    );
+
+    root.unmount();
+
+    expect(getMeaningfulChildren(document)).toEqual(
+      <html>
+        <head>
+          {/* This one never hydrated so React leaves it behind */}
+          <script type="importmap" data-meaningful="">
+            second map (two maps do not make sense but we are asserting they
+            flush anyway)
+          </script>
+          <script async="" src="foo" />
+        </head>
+        <body />
+      </html>,
+    );
+  });
+
   describe('ReactDOM.prefetchDNS(href)', () => {
     it('creates a dns-prefetch resource when called', async () => {
       function App({url}) {
diff --git a/packages/react-dom/src/test-utils/FizzTestUtils.js b/packages/react-dom/src/test-utils/FizzTestUtils.js
@@ -104,9 +104,9 @@ async function executeScript(script: Element) {
     const newScript = ownerDocument.createElement('script');
     newScript.textContent = script.textContent;
     // make sure to add nonce back to script if it exists
-    const scriptNonce = script.getAttribute('nonce');
-    if (scriptNonce) {
-      newScript.setAttribute('nonce', scriptNonce);
+    for (let i = 0; i < script.attributes.length; i++) {
+      const attribute = script.attributes[i];
+      newScript.setAttribute(attribute.name, attribute.value);
     }
 
     parent.insertBefore(newScript, script);
