[Flight] Fallback to importing the whole module instead of encoding every name (#26624)

sebmarkbage · web-flow · commit da6c23a45c62 · 2023-04-14T21:09:09.000-04:00
We currently don't just "require" a module by its module id/path. We encode the pair of module id/path AND its export name. That's because with module splitting, a single original module can end up in two or more separate modules by name. Therefore the manifest files need to encode how to require the whole module as well as how to require each export name. In practice, we don't currently use this because we end up forcing Webpack to deopt and keep it together as a single module, and we don't even have the code in the Webpack plugin to write separate values for each export name. The problem is with CJS we don't statically know what all the export names will be. Since these cases will never be module split, we don't really need to know. This changes the Flight requires to first look for the specific name we're loading and then if that name doesn't exist in the manifest we fallback to looking for the `"*"` name containing the entire module and look for the name in there at runtime. We could probably optimize this a bit if we assume that CJS modules on the server never get built with a name. That way we don't have to do the failed lookup. Additionally, since we've recently merged filepath + name into a single string instead of two values, we now have to split those back out by parsing the string. This is especially unfortunate for server references since those should really not reveal what they are but be a hash or something. The solution might just be to split them back out into two separate fields again. cc @shuding
diff --git a/packages/react-server-dom-webpack/src/ReactFlightClientConfigNodeBundler.js b/packages/react-server-dom-webpack/src/ReactFlightClientConfigNodeBundler.js
@@ -39,8 +39,29 @@ export function resolveClientReference<T>(
   bundlerConfig: SSRManifest,
   metadata: ClientReferenceMetadata,
 ): ClientReference<T> {
-  const resolvedModuleData = bundlerConfig[metadata.id][metadata.name];
-  return resolvedModuleData;
+  const moduleExports = bundlerConfig[metadata.id];
+  let resolvedModuleData = moduleExports[metadata.name];
+  let name;
+  if (resolvedModuleData) {
+    // The potentially aliased name.
+    name = resolvedModuleData.name;
+  } else {
+    // If we don't have this specific name, we might have the full module.
+    resolvedModuleData = moduleExports['*'];
+    if (!resolvedModuleData) {
+      throw new Error(
+        'Could not find the module "' +
+          metadata.id +
+          '" in the React SSR Manifest. ' +
+          'This is probably a bug in the React Server Components bundler.',
+      );
+    }
+    name = metadata.name;
+  }
+  return {
+    specifier: resolvedModuleData.specifier,
+    name: name,
+  };
 }
 
 export function resolveServerReference<T>(
diff --git a/packages/react-server-dom-webpack/src/ReactFlightClientConfigWebpackBundler.js b/packages/react-server-dom-webpack/src/ReactFlightClientConfigWebpackBundler.js
@@ -40,17 +40,31 @@ export function resolveClientReference<T>(
   metadata: ClientReferenceMetadata,
 ): ClientReference<T> {
   if (bundlerConfig) {
-    const resolvedModuleData = bundlerConfig[metadata.id][metadata.name];
-    if (metadata.async) {
-      return {
-        id: resolvedModuleData.id,
-        chunks: resolvedModuleData.chunks,
-        name: resolvedModuleData.name,
-        async: true,
-      };
+    const moduleExports = bundlerConfig[metadata.id];
+    let resolvedModuleData = moduleExports[metadata.name];
+    let name;
+    if (resolvedModuleData) {
+      // The potentially aliased name.
+      name = resolvedModuleData.name;
     } else {
-      return resolvedModuleData;
+      // If we don't have this specific name, we might have the full module.
+      resolvedModuleData = moduleExports['*'];
+      if (!resolvedModuleData) {
+        throw new Error(
+          'Could not find the module "' +
+            metadata.id +
+            '" in the React SSR Manifest. ' +
+            'This is probably a bug in the React Server Components bundler.',
+        );
+      }
+      name = metadata.name;
     }
+    return {
+      id: resolvedModuleData.id,
+      chunks: resolvedModuleData.chunks,
+      name: name,
+      async: !!metadata.async,
+    };
   }
   return metadata;
 }
@@ -59,8 +73,37 @@ export function resolveServerReference<T>(
   bundlerConfig: ServerManifest,
   id: ServerReferenceId,
 ): ClientReference<T> {
-  // This needs to return async: true if it's an async module.
-  return bundlerConfig[id];
+  let name = '';
+  let resolvedModuleData = bundlerConfig[id];
+  if (resolvedModuleData) {
+    // The potentially aliased name.
+    name = resolvedModuleData.name;
+  } else {
+    // We didn't find this specific export name but we might have the * export
+    // which contains this name as well.
+    // TODO: It's unfortunate that we now have to parse this string. We should
+    // probably go back to encoding path and name separately on the client reference.
+    const idx = id.lastIndexOf('#');
+    if (idx !== -1) {
+      name = id.substr(idx + 1);
+      resolvedModuleData = bundlerConfig[id.substr(0, idx)];
+    }
+    if (!resolvedModuleData) {
+      throw new Error(
+        'Could not find the module "' +
+          id +
+          '" in the React Server Manifest. ' +
+          'This is probably a bug in the React Server Components bundler.',
+      );
+    }
+  }
+  // TODO: This needs to return async: true if it's an async module.
+  return {
+    id: resolvedModuleData.id,
+    chunks: resolvedModuleData.chunks,
+    name: name,
+    async: false,
+  };
 }
 
 // The chunk cache contains all the chunks we've preloaded so far.
diff --git a/packages/react-server-dom-webpack/src/ReactFlightServerConfigWebpackBundler.js b/packages/react-server-dom-webpack/src/ReactFlightServerConfigWebpackBundler.js
@@ -58,17 +58,37 @@ export function resolveClientReferenceMetadata<T>(
   config: ClientManifest,
   clientReference: ClientReference<T>,
 ): ClientReferenceMetadata {
-  const resolvedModuleData = config[clientReference.$$id];
-  if (clientReference.$$async) {
-    return {
-      id: resolvedModuleData.id,
-      chunks: resolvedModuleData.chunks,
-      name: resolvedModuleData.name,
-      async: true,
-    };
+  const modulePath = clientReference.$$id;
+  let name = '';
+  let resolvedModuleData = config[modulePath];
+  if (resolvedModuleData) {
+    // The potentially aliased name.
+    name = resolvedModuleData.name;
   } else {
-    return resolvedModuleData;
+    // We didn't find this specific export name but we might have the * export
+    // which contains this name as well.
+    // TODO: It's unfortunate that we now have to parse this string. We should
+    // probably go back to encoding path and name separately on the client reference.
+    const idx = modulePath.lastIndexOf('#');
+    if (idx !== -1) {
+      name = modulePath.substr(idx + 1);
+      resolvedModuleData = config[modulePath.substr(0, idx)];
+    }
+    if (!resolvedModuleData) {
+      throw new Error(
+        'Could not find the module "' +
+          modulePath +
+          '" in the React Client Manifest. ' +
+          'This is probably a bug in the React Server Components bundler.',
+      );
+    }
   }
+  return {
+    id: resolvedModuleData.id,
+    chunks: resolvedModuleData.chunks,
+    name: name,
+    async: !!clientReference.$$async,
+  };
 }
 
 export function getServerReferenceId<T>(
diff --git a/packages/react-server-dom-webpack/src/ReactFlightWebpackPlugin.js b/packages/react-server-dom-webpack/src/ReactFlightWebpackPlugin.js
@@ -247,10 +247,6 @@ export default class ReactFlightWebpackPlugin {
                 return;
               }
 
-              const moduleProvidedExports = compilation.moduleGraph
-                .getExportsInfo(module)
-                .getProvidedExports();
-
               const href = pathToFileURL(module.resource).href;
 
               if (href !== undefined) {
@@ -267,6 +263,13 @@ export default class ReactFlightWebpackPlugin {
                   specifier: href,
                   name: '*',
                 };
+
+                // TODO: If this module ends up split into multiple modules, then
+                // we should encode each the chunks needed for the specific export.
+                // When the module isn't split, it doesn't matter and we can just
+                // encode the id of the whole module. This code doesn't currently
+                // deal with module splitting so is likely broken from ESM anyway.
+                /*
                 clientManifest[href + '#'] = {
                   id,
                   chunks: chunkIds,
@@ -277,6 +280,10 @@ export default class ReactFlightWebpackPlugin {
                   name: '',
                 };
 
+                const moduleProvidedExports = compilation.moduleGraph
+                  .getExportsInfo(module)
+                  .getProvidedExports();
+
                 if (Array.isArray(moduleProvidedExports)) {
                   moduleProvidedExports.forEach(function (name) {
                     clientManifest[href + '#' + name] = {
@@ -290,6 +297,7 @@ export default class ReactFlightWebpackPlugin {
                     };
                   });
                 }
+                */
 
                 ssrManifest[id] = ssrExports;
               }
diff --git a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOM-test.js b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOM-test.js
@@ -334,6 +334,45 @@ describe('ReactFlightDOM', () => {
     expect(container.innerHTML).toBe('<p>Hello World</p>');
   });
 
+  // @gate enableUseHook
+  it('should be able to render a module split named component export', async () => {
+    const Module = {
+      // This gets split into a separate module from the original one.
+      split: function ({greeting}) {
+        return greeting + ' World';
+      },
+    };
+
+    function Print({response}) {
+      return <p>{use(response)}</p>;
+    }
+
+    function App({response}) {
+      return (
+        <Suspense fallback={<h1>Loading...</h1>}>
+          <Print response={response} />
+        </Suspense>
+      );
+    }
+
+    const {split: Component} = clientExports(Module);
+
+    const {writable, readable} = getTestStream();
+    const {pipe} = ReactServerDOMServer.renderToPipeableStream(
+      <Component greeting={'Hello'} />,
+      webpackMap,
+    );
+    pipe(writable);
+    const response = ReactServerDOMClient.createFromReadableStream(readable);
+
+    const container = document.createElement('div');
+    const root = ReactDOMClient.createRoot(container);
+    await act(() => {
+      root.render(<App response={response} />);
+    });
+    expect(container.innerHTML).toBe('<p>Hello World</p>');
+  });
+
   // @gate enableUseHook
   it('should unwrap async module references', async () => {
     const AsyncModule = Promise.resolve(function AsyncModule({text}) {
diff --git a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js
@@ -75,12 +75,35 @@ describe('ReactFlightDOMBrowser', () => {
   }
 
   function requireServerRef(ref) {
-    const metaData = webpackServerMap[ref];
-    const mod = __webpack_require__(metaData.id);
-    if (metaData.name === '*') {
+    let name = '';
+    let resolvedModuleData = webpackServerMap[ref];
+    if (resolvedModuleData) {
+      // The potentially aliased name.
+      name = resolvedModuleData.name;
+    } else {
+      // We didn't find this specific export name but we might have the * export
+      // which contains this name as well.
+      // TODO: It's unfortunate that we now have to parse this string. We should
+      // probably go back to encoding path and name separately on the client reference.
+      const idx = ref.lastIndexOf('#');
+      if (idx !== -1) {
+        name = ref.substr(idx + 1);
+        resolvedModuleData = webpackServerMap[ref.substr(0, idx)];
+      }
+      if (!resolvedModuleData) {
+        throw new Error(
+          'Could not find the module "' +
+            ref +
+            '" in the React Client Manifest. ' +
+            'This is probably a bug in the React Server Components bundler.',
+        );
+      }
+    }
+    const mod = __webpack_require__(resolvedModuleData.id);
+    if (name === '*') {
       return mod;
     }
-    return mod[metaData.name];
+    return mod[name];
   }
 
   async function callServer(actionId, body) {
@@ -824,6 +847,52 @@ describe('ReactFlightDOMBrowser', () => {
     expect(result).toBe('Hello HI');
   });
 
+  it('can call a module split server function', async () => {
+    let actionProxy;
+
+    function Client({action}) {
+      actionProxy = action;
+      return 'Click Me';
+    }
+
+    function greet(text) {
+      return 'Hello ' + text;
+    }
+
+    const ServerModule = serverExports({
+      // This gets split into another module
+      split: greet,
+    });
+    const ClientRef = clientExports(Client);
+
+    const stream = ReactServerDOMServer.renderToReadableStream(
+      <ClientRef action={ServerModule.split} />,
+      webpackMap,
+    );
+
+    const response = ReactServerDOMClient.createFromReadableStream(stream, {
+      async callServer(ref, args) {
+        const body = await ReactServerDOMClient.encodeReply(args);
+        return callServer(ref, body);
+      },
+    });
+
+    function App() {
+      return use(response);
+    }
+
+    const container = document.createElement('div');
+    const root = ReactDOMClient.createRoot(container);
+    await act(() => {
+      root.render(<App />);
+    });
+    expect(container.innerHTML).toBe('Click Me');
+    expect(typeof actionProxy).toBe('function');
+
+    const result = await actionProxy('Split');
+    expect(result).toBe('Hello Split');
+  });
+
   it('can bind arguments to a server reference', async () => {
     let actionProxy;
 
diff --git a/packages/react-server-dom-webpack/src/__tests__/utils/WebpackMock.js b/packages/react-server-dom-webpack/src/__tests__/utils/WebpackMock.js
