From 809874124543405d72fe4d2e2eb67d2fd62c0eb7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Lorber?= Date: Fri, 29 Nov 2024 15:36:02 +0100 Subject: [PATCH] fix(core): fix codegen routesChunkName possible hash collision (#10727) --- .../codegen/__tests__/codegenRoutes.test.ts | 72 ++++++++++++++++--- .../src/server/codegen/codegenRoutes.ts | 48 +++++++++++-- 2 files changed, 104 insertions(+), 16 deletions(-) diff --git a/packages/docusaurus/src/server/codegen/__tests__/codegenRoutes.test.ts b/packages/docusaurus/src/server/codegen/__tests__/codegenRoutes.test.ts index 4353e2733027..f46069227fe3 100644 --- a/packages/docusaurus/src/server/codegen/__tests__/codegenRoutes.test.ts +++ b/packages/docusaurus/src/server/codegen/__tests__/codegenRoutes.test.ts @@ -5,6 +5,7 @@ * LICENSE file in the root directory of this source tree. */ +import {fromPartial} from '@total-typescript/shoehorn'; import { generateRoutesCode, genChunkName, @@ -12,6 +13,10 @@ import { } from '../codegenRoutes'; import type {RouteConfig} from '@docusaurus/types'; +function route(routeConfig: Partial): RouteConfig { + return fromPartial(routeConfig); +} + describe('generateRoutePropFilename', () => { it('generate filename based on route path', () => { expect( @@ -206,9 +211,7 @@ describe('loadRoutes', () => { }, ], }; - expect( - generateRoutesCode([nestedRouteConfig], '/', 'ignore'), - ).toMatchSnapshot(); + expect(generateRoutesCode([nestedRouteConfig])).toMatchSnapshot(); }); it('loads flat route config', () => { @@ -243,9 +246,7 @@ describe('loadRoutes', () => { ], }, }; - expect( - generateRoutesCode([flatRouteConfig], '/', 'ignore'), - ).toMatchSnapshot(); + expect(generateRoutesCode([flatRouteConfig])).toMatchSnapshot(); }); it('rejects invalid route config', () => { @@ -253,7 +254,7 @@ describe('loadRoutes', () => { component: 'hello/world.js', } as RouteConfig; - expect(() => generateRoutesCode([routeConfigWithoutPath], '/', 'ignore')) + expect(() => generateRoutesCode([routeConfigWithoutPath])) .toThrowErrorMatchingInlineSnapshot(` "Invalid route config: path must be a string and component is required. {"component":"hello/world.js"}" @@ -263,9 +264,8 @@ describe('loadRoutes', () => { path: '/hello/world', } as RouteConfig; - expect(() => - generateRoutesCode([routeConfigWithoutComponent], '/', 'ignore'), - ).toThrowErrorMatchingInlineSnapshot(` + expect(() => generateRoutesCode([routeConfigWithoutComponent])) + .toThrowErrorMatchingInlineSnapshot(` "Invalid route config: path must be a string and component is required. {"path":"/hello/world"}" `); @@ -277,6 +277,56 @@ describe('loadRoutes', () => { component: 'hello/world.js', } as RouteConfig; - expect(generateRoutesCode([routeConfig], '/', 'ignore')).toMatchSnapshot(); + expect(generateRoutesCode([routeConfig])).toMatchSnapshot(); + }); + + it('generates an entry for each route and handle hash collisions', () => { + // See bug https://github.com/facebook/docusaurus/issues/10718#issuecomment-2507635907 + const routeConfigs = [ + route({ + path: '/docs', + component: '@theme/Root', + routes: [ + route({ + path: '/docs', + component: '@theme/Version', + children: [], + }), + ], + }), + route({ + path: '/docs', + component: '@theme/Root', + routes: [ + route({ + path: '/docs', + component: '@theme/Version', + children: [], + }), + ], + }), + ]; + + const result = generateRoutesCode(routeConfigs); + + // We absolutely want to have 2 entries here, even if routes are the same + // One should not override the other + expect(Object.keys(result.routesChunkNames)).toHaveLength(4); + expect(result.routesChunkNames).toMatchInlineSnapshot(` + { + "/docs-611": { + "__comp": "__comp---theme-version-6-f-8-19f", + }, + "/docs-96a": { + "__comp": "__comp---theme-root-1-dd-d3a", + }, + "/docs-d3d": { + "__comp": "__comp---theme-version-6-f-8-19f", + }, + "/docs-e4f": { + "__comp": "__comp---theme-root-1-dd-d3a", + }, + } + `); }); }); diff --git a/packages/docusaurus/src/server/codegen/codegenRoutes.ts b/packages/docusaurus/src/server/codegen/codegenRoutes.ts index b637fd956d72..db494a67224f 100644 --- a/packages/docusaurus/src/server/codegen/codegenRoutes.ts +++ b/packages/docusaurus/src/server/codegen/codegenRoutes.ts @@ -194,7 +194,12 @@ function genChunkNames( * config node, it returns the node's serialized form, and mutates `registry`, * `routesPaths`, and `routesChunkNames` accordingly. */ -function genRouteCode(routeConfig: RouteConfig, res: RoutesCode): string { +function genRouteCode( + routeConfig: RouteConfig, + res: RoutesCode, + index: number, + level: number, +): string { const { path: routePath, component, @@ -216,8 +221,39 @@ ${JSON.stringify(routeConfig)}`, ); } - const routeHash = simpleHash(JSON.stringify(routeConfig), 3); - res.routesChunkNames[`${routePath}-${routeHash}`] = { + // Because 2 routes with the same path could lead to hash collisions + // See https://github.com/facebook/docusaurus/issues/10718#issuecomment-2498516394 + function generateUniqueRouteKey(): { + routeKey: string; + routeHash: string; + } { + const hashes = [ + // // OG algo to keep former snapshots + () => simpleHash(JSON.stringify(routeConfig), 3), + // Other attempts, not ideal but good enough + // Technically we could use Math.random() here but it's annoying for tests + () => simpleHash(`${level}${index}`, 3), + () => simpleHash(JSON.stringify(routeConfig), 4), + () => simpleHash(`${level}${index}`, 4), + ]; + + for (const tryHash of hashes) { + const routeHash = tryHash(); + const routeKey = `${routePath}-${routeHash}`; + if (!res.routesChunkNames[routeKey]) { + return {routeKey, routeHash}; + } + } + throw new Error( + `Docusaurus couldn't generate a unique hash for route ${routeConfig.path} (level=${level} - index=${index}). +This is a bug, please report it here! +https://github.com/facebook/docusaurus/issues/10718`, + ); + } + + const {routeKey, routeHash} = generateUniqueRouteKey(); + + res.routesChunkNames[routeKey] = { // Avoid clash with a prop called "component" ...genChunkNames({__comp: component}, 'component', component, res), ...(context && @@ -228,7 +264,9 @@ ${JSON.stringify(routeConfig)}`, return serializeRouteConfig({ routePath: routePath.replace(/'/g, "\\'"), routeHash, - subroutesCodeStrings: subroutes?.map((r) => genRouteCode(r, res)), + subroutesCodeStrings: subroutes?.map((r, i) => + genRouteCode(r, res, i, level + 1), + ), exact, attributes, }); @@ -253,7 +291,7 @@ export function generateRoutesCode(routeConfigs: RouteConfig[]): RoutesCode { // `genRouteCode` would mutate `res` const routeConfigSerialized = routeConfigs - .map((r) => genRouteCode(r, res)) + .map((r, i) => genRouteCode(r, res, i, 0)) .join(',\n'); res.routesConfig = `import React from 'react';