fix(core): fix codegen routesChunkName possible hash collision (#10727)

This commit is contained in:
Sébastien Lorber 2024-11-29 15:36:02 +01:00 committed by GitHub
parent 1777b14566
commit 8098741245
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 104 additions and 16 deletions

View file

@ -5,6 +5,7 @@
* LICENSE file in the root directory of this source tree. * LICENSE file in the root directory of this source tree.
*/ */
import {fromPartial} from '@total-typescript/shoehorn';
import { import {
generateRoutesCode, generateRoutesCode,
genChunkName, genChunkName,
@ -12,6 +13,10 @@ import {
} from '../codegenRoutes'; } from '../codegenRoutes';
import type {RouteConfig} from '@docusaurus/types'; import type {RouteConfig} from '@docusaurus/types';
function route(routeConfig: Partial<RouteConfig>): RouteConfig {
return fromPartial(routeConfig);
}
describe('generateRoutePropFilename', () => { describe('generateRoutePropFilename', () => {
it('generate filename based on route path', () => { it('generate filename based on route path', () => {
expect( expect(
@ -206,9 +211,7 @@ describe('loadRoutes', () => {
}, },
], ],
}; };
expect( expect(generateRoutesCode([nestedRouteConfig])).toMatchSnapshot();
generateRoutesCode([nestedRouteConfig], '/', 'ignore'),
).toMatchSnapshot();
}); });
it('loads flat route config', () => { it('loads flat route config', () => {
@ -243,9 +246,7 @@ describe('loadRoutes', () => {
], ],
}, },
}; };
expect( expect(generateRoutesCode([flatRouteConfig])).toMatchSnapshot();
generateRoutesCode([flatRouteConfig], '/', 'ignore'),
).toMatchSnapshot();
}); });
it('rejects invalid route config', () => { it('rejects invalid route config', () => {
@ -253,7 +254,7 @@ describe('loadRoutes', () => {
component: 'hello/world.js', component: 'hello/world.js',
} as RouteConfig; } as RouteConfig;
expect(() => generateRoutesCode([routeConfigWithoutPath], '/', 'ignore')) expect(() => generateRoutesCode([routeConfigWithoutPath]))
.toThrowErrorMatchingInlineSnapshot(` .toThrowErrorMatchingInlineSnapshot(`
"Invalid route config: path must be a string and component is required. "Invalid route config: path must be a string and component is required.
{"component":"hello/world.js"}" {"component":"hello/world.js"}"
@ -263,9 +264,8 @@ describe('loadRoutes', () => {
path: '/hello/world', path: '/hello/world',
} as RouteConfig; } as RouteConfig;
expect(() => expect(() => generateRoutesCode([routeConfigWithoutComponent]))
generateRoutesCode([routeConfigWithoutComponent], '/', 'ignore'), .toThrowErrorMatchingInlineSnapshot(`
).toThrowErrorMatchingInlineSnapshot(`
"Invalid route config: path must be a string and component is required. "Invalid route config: path must be a string and component is required.
{"path":"/hello/world"}" {"path":"/hello/world"}"
`); `);
@ -277,6 +277,56 @@ describe('loadRoutes', () => {
component: 'hello/world.js', component: 'hello/world.js',
} as RouteConfig; } 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",
},
}
`);
}); });
}); });

View file

@ -194,7 +194,12 @@ function genChunkNames(
* config node, it returns the node's serialized form, and mutates `registry`, * config node, it returns the node's serialized form, and mutates `registry`,
* `routesPaths`, and `routesChunkNames` accordingly. * `routesPaths`, and `routesChunkNames` accordingly.
*/ */
function genRouteCode(routeConfig: RouteConfig, res: RoutesCode): string { function genRouteCode(
routeConfig: RouteConfig,
res: RoutesCode,
index: number,
level: number,
): string {
const { const {
path: routePath, path: routePath,
component, component,
@ -216,8 +221,39 @@ ${JSON.stringify(routeConfig)}`,
); );
} }
const routeHash = simpleHash(JSON.stringify(routeConfig), 3); // Because 2 routes with the same path could lead to hash collisions
res.routesChunkNames[`${routePath}-${routeHash}`] = { // 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" // Avoid clash with a prop called "component"
...genChunkNames({__comp: component}, 'component', component, res), ...genChunkNames({__comp: component}, 'component', component, res),
...(context && ...(context &&
@ -228,7 +264,9 @@ ${JSON.stringify(routeConfig)}`,
return serializeRouteConfig({ return serializeRouteConfig({
routePath: routePath.replace(/'/g, "\\'"), routePath: routePath.replace(/'/g, "\\'"),
routeHash, routeHash,
subroutesCodeStrings: subroutes?.map((r) => genRouteCode(r, res)), subroutesCodeStrings: subroutes?.map((r, i) =>
genRouteCode(r, res, i, level + 1),
),
exact, exact,
attributes, attributes,
}); });
@ -253,7 +291,7 @@ export function generateRoutesCode(routeConfigs: RouteConfig[]): RoutesCode {
// `genRouteCode` would mutate `res` // `genRouteCode` would mutate `res`
const routeConfigSerialized = routeConfigs const routeConfigSerialized = routeConfigs
.map((r) => genRouteCode(r, res)) .map((r, i) => genRouteCode(r, res, i, 0))
.join(',\n'); .join(',\n');
res.routesConfig = `import React from 'react'; res.routesConfig = `import React from 'react';