fix(core): codegen should generate unique route prop filenames (#10131)

Author: slorber

packages/docusaurus-utils/src/__tests__/hashUtils.test.ts

@@ -48,4 +48,30 @@ describe('docuHash', () => {
       expect(docuHash(file)).toBe(asserts[file]);
     });
   });
+
+  it('docuHash works with hashLength option', () => {
+    const asserts: {[key: string]: string} = {
+      '': '-d41d8',
+      '/': 'index',
+      '/foo-bar': 'foo-bar-09652',
+      '/foo/bar': 'foo-bar-1df48',
+    };
+    Object.keys(asserts).forEach((file) => {
+      expect(docuHash(file, {hashLength: 5})).toBe(asserts[file]);
+    });
+  });
+
+  it('docuHash works with hashExtra option', () => {
+    expect(docuHash('')).toBe('-d41');
+    expect(docuHash('', {hashExtra: ''})).toBe('-d41');
+    expect(docuHash('', {hashExtra: 'some-extra'})).toBe('-928');
+
+    expect(docuHash('/')).toBe('index');
+    expect(docuHash('/', {hashExtra: ''})).toBe('index-6a9');
+    expect(docuHash('/', {hashExtra: 'some-extra'})).toBe('index-68e');
+
+    expect(docuHash('/foo/bar')).toBe('foo-bar-1df');
+    expect(docuHash('/foo/bar', {hashExtra: ''})).toBe('foo-bar-1df');
+    expect(docuHash('/foo/bar', {hashExtra: 'some-extra'})).toBe('foo-bar-7d4');
+  });
 });

packages/docusaurus-utils/src/hashUtils.ts

@@ -25,11 +25,28 @@ export function simpleHash(str: string, length: number): string {
  * collision. Also removes part of the string if its larger than the allowed
  * filename per OS, avoiding `ERRNAMETOOLONG` error.
  */
-export function docuHash(str: string): string {
-  if (str === '/') {
+export function docuHash(
+  strInput: string,
+  options?: {
+    // String that contributes to the hash value
+    // but does not contribute to the returned string
+    hashExtra?: string;
+    // Length of the hash to append
+    hashLength?: number;
+  },
+): string {
+  // TODO check this historical behavior
+  //  I'm not sure it makes sense to keep it...
+  if (strInput === '/' && typeof options?.hashExtra === 'undefined') {
     return 'index';
   }
-  const shortHash = simpleHash(str, 3);
+  const str = strInput === '/' ? 'index' : strInput;
+
+  const hashExtra = options?.hashExtra ?? '';
+  const hashLength = options?.hashLength ?? 3;
+
+  const stringToHash = str + hashExtra;
+  const shortHash = simpleHash(stringToHash, hashLength);
   const parsedPath = `${_.kebabCase(str)}-${shortHash}`;
   if (isNameTooLong(parsedPath)) {
     return `${shortName(_.kebabCase(str))}-${shortHash}`;

packages/docusaurus/src/server/codegen/__tests__/codegenRoutes.test.ts

@@ -5,9 +5,75 @@
  * LICENSE file in the root directory of this source tree.
  */
 
-import {generateRoutesCode, genChunkName} from '../codegenRoutes';
+import {
+  generateRoutesCode,
+  genChunkName,
+  generateRoutePropFilename,
+} from '../codegenRoutes';
 import type {RouteConfig} from '@docusaurus/types';
 
+describe('generateRoutePropFilename', () => {
+  it('generate filename based on route path', () => {
+    expect(
+      generateRoutePropFilename({
+        path: '/some/route-path/',
+        component: '@theme/Home',
+      }),
+    ).toEqual(expect.stringMatching(/^some-route-path-[a-z\d]{3}.json$/));
+  });
+
+  it('generate filename for /', () => {
+    expect(
+      generateRoutePropFilename({
+        path: '/',
+        component: '@theme/Home',
+      }),
+    ).toEqual(expect.stringMatching(/^index-[a-z\d]{3}.json$/));
+  });
+
+  it('generate filename for /category/', () => {
+    expect(
+      generateRoutePropFilename({
+        path: '/category/',
+        component: '@theme/Home',
+      }),
+    ).toEqual(expect.stringMatching(/^category-[a-z\d]{3}.json$/));
+  });
+
+  it('generate unique filenames for /', () => {
+    expect(
+      generateRoutePropFilename({path: '/', component: '@theme/Home'}),
+    ).toEqual(generateRoutePropFilename({path: '/', component: '@theme/Home'}));
+    expect(
+      generateRoutePropFilename({path: '/', component: '@theme/Home'}),
+    ).not.toEqual(
+      generateRoutePropFilename({
+        path: '/',
+        component: '@theme/AnotherComponent',
+      }),
+    );
+  });
+
+  it('generate unique filenames for /some/path', () => {
+    expect(
+      generateRoutePropFilename({path: '/some/path', component: '@theme/Home'}),
+    ).toEqual(
+      generateRoutePropFilename({path: '/some/path', component: '@theme/Home'}),
+    );
+    expect(
+      generateRoutePropFilename({
+        path: '/some/path',
+        component: '@theme/Home',
+      }),
+    ).not.toEqual(
+      generateRoutePropFilename({
+        path: '/some/path',
+        component: '@theme/AnotherComponent',
+      }),
+    );
+  });
+});
+
 describe('genChunkName', () => {
   it('works', () => {
     const firstAssert: {[key: string]: string} = {

packages/docusaurus/src/server/codegen/codegenRoutes.ts

@@ -320,6 +320,22 @@ type GenerateRouteFilesParams = {
   baseUrl: string;
 };
 
+// The generated filename per route must be unique to avoid conflicts
+// See also https://github.com/facebook/docusaurus/issues/10125
+export function generateRoutePropFilename(route: RouteConfig): string {
+  // TODO if possible, we could try to shorten the filename by removing
+  //  the plugin routeBasePath prefix from the name
+  return `${docuHash(
+    route.path,
+    // Note: using hash(route.path + route.component) is not technically
+    // as robust as hashing the entire prop content object.
+    // But it's faster and should be good enough considering it's very unlikely
+    // anyone would have 2 routes on the same path also rendering the exact
+    // same component.
+    {hashExtra: route.component},
+  )}.json`;
+}
+
 async function generateRoutePropModule({
   generatedFilesDir,
   route,
@@ -339,7 +355,7 @@ async function generateRoutePropModule({
     plugin.name,
     plugin.id,
     'p',
-    `${docuHash(route.path)}.json`,
+    generateRoutePropFilename(route),
   );
   const modulePath = path.posix.join(generatedFilesDir, relativePath);
   const aliasedPath = path.posix.join('@generated', relativePath);
@@ -376,29 +392,31 @@ async function preprocessRouteProps({
   route: RouteConfig;
   plugin: PluginIdentifier;
 }): Promise<RouteConfig> {
-  const propsModulePathPromise = route.props
-    ? generateRoutePropModule({
-        generatedFilesDir,
-        route,
-        plugin,
-      })
-    : undefined;
-
-  const subRoutesPromise = route.routes
-    ? Promise.all(
-        route.routes.map((subRoute: RouteConfig) => {
-          return preprocessRouteProps({
-            generatedFilesDir,
-            route: subRoute,
-            plugin,
-          });
-        }),
-      )
-    : undefined;
+  const getPropsModulePathPromise = () =>
+    route.props
+      ? generateRoutePropModule({
+          generatedFilesDir,
+          route,
+          plugin,
+        })
+      : undefined;
+
+  const getSubRoutesPromise = () =>
+    route.routes
+      ? Promise.all(
+          route.routes.map((subRoute: RouteConfig) => {
+            return preprocessRouteProps({
+              generatedFilesDir,
+              route: subRoute,
+              plugin,
+            });
+          }),
+        )
+      : undefined;
 
   const [propsModulePath, subRoutes] = await Promise.all([
-    propsModulePathPromise,
-    subRoutesPromise,
+    getPropsModulePathPromise(),
+    getSubRoutesPromise(),
   ]);
 
   const newRoute: RouteConfig = {