Skip to content

Commit

Permalink
fix(core): codegen should generate unique route prop filenames (#10131)
Browse files Browse the repository at this point in the history
  • Loading branch information
slorber committed May 10, 2024
1 parent 394ce84 commit 29b7a4d
Show file tree
Hide file tree
Showing 4 changed files with 153 additions and 26 deletions.
26 changes: 26 additions & 0 deletions packages/docusaurus-utils/src/__tests__/hashUtils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});
});
23 changes: 20 additions & 3 deletions packages/docusaurus-utils/src/hashUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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}`;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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} = {
Expand Down
62 changes: 40 additions & 22 deletions packages/docusaurus/src/server/codegen/codegenRoutes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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);
Expand Down Expand Up @@ -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 = {
Expand Down

0 comments on commit 29b7a4d

Please sign in to comment.