From 29b7a4ddbbd057372d10370fc062075d79c98b46 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Lorber?= Date: Fri, 10 May 2024 18:17:21 +0200 Subject: [PATCH] fix(core): codegen should generate unique route prop filenames (#10131) --- .../src/__tests__/hashUtils.test.ts | 26 +++++++ packages/docusaurus-utils/src/hashUtils.ts | 23 ++++++- .../codegen/__tests__/codegenRoutes.test.ts | 68 ++++++++++++++++++- .../src/server/codegen/codegenRoutes.ts | 62 +++++++++++------ 4 files changed, 153 insertions(+), 26 deletions(-) diff --git a/packages/docusaurus-utils/src/__tests__/hashUtils.test.ts b/packages/docusaurus-utils/src/__tests__/hashUtils.test.ts index 6cc32c0e4640..c51acd087eb4 100644 --- a/packages/docusaurus-utils/src/__tests__/hashUtils.test.ts +++ b/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'); + }); }); diff --git a/packages/docusaurus-utils/src/hashUtils.ts b/packages/docusaurus-utils/src/hashUtils.ts index 8d6e344b4732..8e216cba9e0f 100644 --- a/packages/docusaurus-utils/src/hashUtils.ts +++ b/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}`; diff --git a/packages/docusaurus/src/server/codegen/__tests__/codegenRoutes.test.ts b/packages/docusaurus/src/server/codegen/__tests__/codegenRoutes.test.ts index c1e55c917879..4353e2733027 100644 --- a/packages/docusaurus/src/server/codegen/__tests__/codegenRoutes.test.ts +++ b/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} = { diff --git a/packages/docusaurus/src/server/codegen/codegenRoutes.ts b/packages/docusaurus/src/server/codegen/codegenRoutes.ts index cb40afc585dc..b637fd956d72 100644 --- a/packages/docusaurus/src/server/codegen/codegenRoutes.ts +++ b/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 { - 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 = {