refactor(core): add comments for react-loadable logic (#7075)

* refactor(core): add comments for react-loadable logic

* fix test

packages/docusaurus/src/client/exports/ComponentCreator.tsx

@@ -21,77 +21,97 @@ export default function ComponentCreator(
   if (path === '*') {
     return Loadable({
       loading: Loading,
-      loader: async () => {
-        const NotFound = (await import('@theme/NotFound')).default;
-        return (props) => (
-          // Is there a better API for this?
+      loader: () =>
+        import('@theme/NotFound').then(({default: NotFound}) => (props) => (
           <RouteContextProvider
+            // Do we want a better name than native-default?
             value={{plugin: {name: 'native', id: 'default'}}}>
             <NotFound {...(props as never)} />
           </RouteContextProvider>
-        );
-      },
+        )),
     });
   }
 
   const chunkNames = routesChunkNames[`${path}-${hash}`]!;
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
-  const optsLoader: {[key: string]: () => Promise<any>} = {};
-  const optsModules: string[] = [];
+  const loader: {[key: string]: () => Promise<any>} = {};
+  const modules: string[] = [];
   const optsWebpack: string[] = [];
 
+  // A map from prop names to chunk names.
+  // e.g. Suppose the plugin added this as route:
+  //   { __comp: "...", prop: { foo: "..." }, items: ["...", "..."] }
+  // It will become:
+  //   { __comp: "...", "prop.foo": "...", "items.0": "...", "items.1": ... }
+  // Loadable.Map will _map_ over `loader` and load each key.
   const flatChunkNames = flat(chunkNames);
-  Object.entries(flatChunkNames).forEach(([key, chunkName]) => {
+  Object.entries(flatChunkNames).forEach(([keyPath, chunkName]) => {
     const chunkRegistry = registry[chunkName];
     if (chunkRegistry) {
       // eslint-disable-next-line prefer-destructuring
-      optsLoader[key] = chunkRegistry[0];
-      optsModules.push(chunkRegistry[1]);
+      loader[keyPath] = chunkRegistry[0];
+      modules.push(chunkRegistry[1]);
       optsWebpack.push(chunkRegistry[2]);
     }
   });
 
   return Loadable.Map({
     loading: Loading,
-    loader: optsLoader,
-    modules: optsModules,
+    loader,
+    modules,
     webpack: () => optsWebpack,
     render: (loaded, props) => {
-      // Clone the original object since we don't want to alter the original.
+      // `loaded` will be a map from key path (as returned from the flattened
+      // chunk names) to the modules loaded from the loaders. We now have to
+      // restore the chunk names' previous shape from this flat record.
+      // We do so by taking advantage of the existing `chunkNames` and replacing
+      // each chunk name with its loaded module, so we don't create another
+      // object from scratch.
       const loadedModules = JSON.parse(JSON.stringify(chunkNames));
-      Object.keys(loaded).forEach((key) => {
-        const newComp = loaded[key].default;
-        if (!newComp) {
+      Object.entries(loaded).forEach(([keyPath, loadedModule]) => {
+        // JSON modules are also loaded as `{ default: ... }` (`import()`
+        // semantics) but we just want to pass the actual value to props.
+        const chunk = loadedModule.default;
+        // One loaded chunk can only be one of two things: a module (props) or a
+        // component. Modules are always JSON, so `default` always exists. This
+        // could only happen with a user-defined component.
+        if (!chunk) {
           throw new Error(
             `The page component at ${path} doesn't have a default export. This makes it impossible to render anything. Consider default-exporting a React component.`,
           );
         }
-        if (typeof newComp === 'object' || typeof newComp === 'function') {
-          Object.keys(loaded[key])
+        // A module can be a primitive, for example, if the user stored a string
+        // as a prop. However, there seems to be a bug with swc-loader's CJS
+        // logic, in that it would load a JSON module with content "foo" as
+        // `{ default: "foo", 0: "f", 1: "o", 2: "o" }`. Just to be safe, we
+        // first make sure that the chunk is non-primitive.
+        if (typeof chunk === 'object' || typeof chunk === 'function') {
+          Object.keys(loadedModule)
             .filter((k) => k !== 'default')
             .forEach((nonDefaultKey) => {
-              newComp[nonDefaultKey] = loaded[key][nonDefaultKey];
+              chunk[nonDefaultKey] = loadedModule[nonDefaultKey];
             });
         }
+        // We now have this chunk prepared. Go down the key path and replace the
+        // chunk name with the actual chunk.
         let val = loadedModules;
-        const keyPath = key.split('.');
-        keyPath.slice(0, -1).forEach((k) => {
+        const keyPaths = keyPath.split('.');
+        keyPaths.slice(0, -1).forEach((k) => {
           val = val[k];
         });
-        val[keyPath[keyPath.length - 1]!] = newComp;
+        val[keyPaths[keyPaths.length - 1]!] = chunk;
       });
 
-      const Component = loadedModules.component;
-      delete loadedModules.component;
-
       /* eslint-disable no-underscore-dangle */
-      const routeContextModule = loadedModules.__routeContextModule;
-      delete loadedModules.__routeContextModule;
+      const Component = loadedModules.__comp;
+      delete loadedModules.__comp;
+      const routeContext = loadedModules.__context;
+      delete loadedModules.__context;
       /* eslint-enable no-underscore-dangle */
 
       // Is there any way to put this RouteContextProvider upper in the tree?
       return (
-        <RouteContextProvider value={routeContextModule}>
+        <RouteContextProvider value={routeContext}>
           <Component {...loadedModules} {...props} />
         </RouteContextProvider>
       );

packages/docusaurus/src/client/prefetch.ts

@@ -5,21 +5,13 @@
  * LICENSE file in the root directory of this source tree.
  */
 
-function support(feature: string) {
-  if (typeof document === 'undefined') {
-    return false;
-  }
-
-  const fakeLink = document.createElement('link');
+function supports(feature: string) {
   try {
-    if (fakeLink.relList && typeof fakeLink.relList.supports === 'function') {
-      return fakeLink.relList.supports(feature);
-    }
+    const fakeLink = document.createElement('link');
+    return fakeLink.relList?.supports?.(feature);
   } catch (err) {
     return false;
   }
-
-  return false;
 }
 
 function linkPrefetchStrategy(url: string) {
@@ -61,7 +53,7 @@ function xhrPrefetchStrategy(url: string): Promise<void> {
   });
 }
 
-const supportedPrefetchStrategy = support('prefetch')
+const supportedPrefetchStrategy = supports('prefetch')
   ? linkPrefetchStrategy
   : xhrPrefetchStrategy;
 

packages/docusaurus/src/client/serverEntry.tsx

@@ -78,14 +78,14 @@ async function doRender(locals: Locals & {path: string}) {
   const location = routesLocation[locals.path]!;
   await preload(routes, location);
   const modules = new Set<string>();
-  const context = {};
+  const routerContext = {};
   const helmetContext = {};
 
   const linksCollector = createStatefulLinksCollector();
   const appHtml = ReactDOMServer.renderToString(
     <Loadable.Capture report={(moduleName) => modules.add(moduleName)}>
       <HelmetProvider context={helmetContext}>
-        <StaticRouter location={location} context={context}>
+        <StaticRouter location={location} context={routerContext}>
           <LinksCollectorProvider linksCollector={linksCollector}>
             <App />
           </LinksCollectorProvider>

packages/docusaurus/src/server/__tests__/__snapshots__/routes.test.ts.snap

@@ -12,8 +12,8 @@ This could lead to non-deterministic routing behavior."
 exports[`loadRoutes loads flat route config 1`] = `
 {
   "registry": {
-    "component---theme-blog-list-pagea-6-a-7ba": {
-      "loader": "() => import(/* webpackChunkName: 'component---theme-blog-list-pagea-6-a-7ba' */ '@theme/BlogListPage')",
+    "__comp---theme-blog-list-pagea-6-a-7ba": {
+      "loader": "() => import(/* webpackChunkName: '__comp---theme-blog-list-pagea-6-a-7ba' */ '@theme/BlogListPage')",
       "modulePath": "@theme/BlogListPage",
     },
     "content---blog-0-b-4-09e": {
@@ -31,7 +31,7 @@ exports[`loadRoutes loads flat route config 1`] = `
   },
   "routesChunkNames": {
     "/blog-599": {
-      "component": "component---theme-blog-list-pagea-6-a-7ba",
+      "__comp": "__comp---theme-blog-list-pagea-6-a-7ba",
       "items": [
         {
           "content": "content---blog-0-b-4-09e",
@@ -71,12 +71,12 @@ export default [
 exports[`loadRoutes loads nested route config 1`] = `
 {
   "registry": {
-    "component---theme-doc-item-178-a40": {
-      "loader": "() => import(/* webpackChunkName: 'component---theme-doc-item-178-a40' */ '@theme/DocItem')",
+    "__comp---theme-doc-item-178-a40": {
+      "loader": "() => import(/* webpackChunkName: '__comp---theme-doc-item-178-a40' */ '@theme/DocItem')",
       "modulePath": "@theme/DocItem",
     },
-    "component---theme-doc-page-1-be-9be": {
-      "loader": "() => import(/* webpackChunkName: 'component---theme-doc-page-1-be-9be' */ '@theme/DocPage')",
+    "__comp---theme-doc-page-1-be-9be": {
+      "loader": "() => import(/* webpackChunkName: '__comp---theme-doc-page-1-be-9be' */ '@theme/DocPage')",
       "modulePath": "@theme/DocPage",
     },
     "content---docs-foo-baz-8-ce-61e": {
@@ -102,16 +102,16 @@ exports[`loadRoutes loads nested route config 1`] = `
   },
   "routesChunkNames": {
     "/docs/hello-44b": {
-      "component": "component---theme-doc-item-178-a40",
+      "__comp": "__comp---theme-doc-item-178-a40",
       "content": "content---docs-helloaff-811",
       "metadata": "metadata---docs-hello-956-741",
     },
     "/docs:route-52d": {
-      "component": "component---theme-doc-page-1-be-9be",
+      "__comp": "__comp---theme-doc-page-1-be-9be",
       "docsMetadata": "docsMetadata---docs-routef-34-881",
     },
     "docs/foo/baz-070": {
-      "component": "component---theme-doc-item-178-a40",
+      "__comp": "__comp---theme-doc-item-178-a40",
       "content": "content---docs-foo-baz-8-ce-61e",
       "metadata": "metadata---docs-foo-baz-2-cf-fa7",
     },
@@ -159,14 +159,14 @@ export default [
 exports[`loadRoutes loads route config with empty (but valid) path string 1`] = `
 {
   "registry": {
-    "component---hello-world-jse-0-f-b6c": {
-      "loader": "() => import(/* webpackChunkName: 'component---hello-world-jse-0-f-b6c' */ 'hello/world.js')",
+    "__comp---hello-world-jse-0-f-b6c": {
+      "loader": "() => import(/* webpackChunkName: '__comp---hello-world-jse-0-f-b6c' */ 'hello/world.js')",
       "modulePath": "hello/world.js",
     },
   },
   "routesChunkNames": {
     "-b2a": {
-      "component": "component---hello-world-jse-0-f-b6c",
+      "__comp": "__comp---hello-world-jse-0-f-b6c",
     },
   },
   "routesConfig": "import React from 'react';

packages/docusaurus/src/server/plugins/index.ts

@@ -131,7 +131,7 @@ export async function loadPlugins(context: LoadContext): Promise<{
             ...finalRouteConfig,
             modules: {
               ...finalRouteConfig.modules,
-              __routeContextModule: pluginRouteContextModulePath,
+              __context: pluginRouteContextModulePath,
             },
           });
         },

packages/docusaurus/src/server/routes.ts

@@ -151,7 +151,10 @@ const isModule = (value: unknown): value is Module =>
     // eslint-disable-next-line no-underscore-dangle
     !!(value as {[key: string]: unknown})?.__import);
 
-/** Takes a {@link Module} and returns the string path it represents. */
+/**
+ * Takes a {@link Module} (which is nothing more than a path plus some metadata
+ * like query) and returns the string path it represents.
+ */
 function getModulePath(target: Module): string {
   if (typeof target === 'string') {
     return target;
@@ -241,7 +244,7 @@ This could lead to non-deterministic routing behavior.`;
 
 /**
  * This is the higher level overview of route code generation. For each route
- * config node, it return the node's serialized form, and mutate `registry`,
+ * config node, it returns the node's serialized form, and mutates `registry`,
  * `routesPaths`, and `routesChunkNames` accordingly.
  */
 function genRouteCode(routeConfig: RouteConfig, res: LoadedRoutes): string {
@@ -268,7 +271,8 @@ ${JSON.stringify(routeConfig)}`,
 
   const routeHash = simpleHash(JSON.stringify(routeConfig), 3);
   res.routesChunkNames[`${routePath}-${routeHash}`] = {
-    ...genChunkNames({component}, 'component', component, res),
+    // Avoid clash with a prop called "component"
+    ...genChunkNames({__comp: component}, 'component', component, res),
     ...genChunkNames(modules, 'module', routePath, res),
   };
 
@@ -297,7 +301,7 @@ export async function loadRoutes(
 ): Promise<LoadedRoutes> {
   handleDuplicateRoutes(routeConfigs, onDuplicateRoutes);
   const res: LoadedRoutes = {
-    // To be written
+    // To be written by `genRouteCode`
     routesConfig: '',
     routesChunkNames: {},
     registry: {},