From 128738786b5f7c99c07922cede1c640804cdbed9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Lorber?= Date: Thu, 18 Apr 2024 15:08:30 +0200 Subject: [PATCH] fix(core): sortRoutes shouldn't have a default baseUrl value, this led to a bug (#10054) --- .../src/__tests__/index.test.ts | 4 +-- .../plugins/__tests__/routeConfig.test.ts | 31 ++++++++++++++----- .../docusaurus/src/server/plugins/plugins.ts | 13 +++++--- .../src/server/plugins/routeConfig.ts | 30 +++++++++++++++--- 4 files changed, 58 insertions(+), 20 deletions(-) diff --git a/packages/docusaurus-plugin-content-docs/src/__tests__/index.test.ts b/packages/docusaurus-plugin-content-docs/src/__tests__/index.test.ts index 150ca3776628..65c1b3217919 100644 --- a/packages/docusaurus-plugin-content-docs/src/__tests__/index.test.ts +++ b/packages/docusaurus-plugin-content-docs/src/__tests__/index.test.ts @@ -58,7 +58,7 @@ Available ids are:\n- ${version.docs.map((d) => d.id).join('\n- ')}`, } const createFakeActions = (contentDir: string) => { - const routeConfigs: RouteConfig[] = []; + let routeConfigs: RouteConfig[] = []; const dataContainer: {[key: string]: unknown} = {}; const globalDataContainer: {pluginName?: {pluginId: unknown}} = {}; @@ -83,7 +83,7 @@ const createFakeActions = (contentDir: string) => { expectSnapshot: () => { // Sort the route config like in src/server/plugins/index.ts for // consistent snapshot ordering - sortRoutes(routeConfigs); + routeConfigs = sortRoutes(routeConfigs, '/'); expect(routeConfigs).not.toEqual([]); expect(routeConfigs).toMatchSnapshot('route config'); expect(dataContainer).toMatchSnapshot('data'); diff --git a/packages/docusaurus/src/server/plugins/__tests__/routeConfig.test.ts b/packages/docusaurus/src/server/plugins/__tests__/routeConfig.test.ts index da4345add7bb..f35dc7637fe5 100644 --- a/packages/docusaurus/src/server/plugins/__tests__/routeConfig.test.ts +++ b/packages/docusaurus/src/server/plugins/__tests__/routeConfig.test.ts @@ -202,9 +202,7 @@ describe('sortRoutes', () => { }, ]; - sortRoutes(routes); - - expect(routes).toMatchSnapshot(); + expect(sortRoutes(routes, '/')).toMatchSnapshot(); }); it('sorts route config recursively', () => { @@ -248,9 +246,7 @@ describe('sortRoutes', () => { }, ]; - sortRoutes(routes); - - expect(routes).toMatchSnapshot(); + expect(sortRoutes(routes, '/')).toMatchSnapshot(); }); it('sorts route config given a baseURL', () => { @@ -290,8 +286,27 @@ describe('sortRoutes', () => { }, ]; - sortRoutes(routes, baseURL); + expect(sortRoutes(routes, baseURL)).toMatchSnapshot(); + }); + + it('sorts parent route configs when one included in another', () => { + const r1: RouteConfig = { + path: '/one', + component: '', + routes: [{path: `/one/myDoc`, component: ''}], + }; + const r2: RouteConfig = { + path: '/', + component: '', + routes: [{path: `/someDoc`, component: ''}], + }; + const r3: RouteConfig = { + path: '/one/another', + component: '', + routes: [{path: `/one/another/myDoc`, component: ''}], + }; - expect(routes).toMatchSnapshot(); + expect(sortRoutes([r1, r2, r3], '/')).toEqual([r3, r1, r2]); + expect(sortRoutes([r3, r1, r2], '/')).toEqual([r3, r1, r2]); }); }); diff --git a/packages/docusaurus/src/server/plugins/plugins.ts b/packages/docusaurus/src/server/plugins/plugins.ts index 17e1b2d6c751..631e58664446 100644 --- a/packages/docusaurus/src/server/plugins/plugins.ts +++ b/packages/docusaurus/src/server/plugins/plugins.ts @@ -224,17 +224,18 @@ async function executeAllPluginsAllContentLoaded({ // - contentLoaded() // - allContentLoaded() function mergeResults({ + baseUrl, plugins, allContentLoadedResult, }: { + baseUrl: string; plugins: LoadedPlugin[]; allContentLoadedResult: AllContentLoadedResult; }) { - const routes: PluginRouteConfig[] = [ - ...aggregateRoutes(plugins), - ...allContentLoadedResult.routes, - ]; - sortRoutes(routes); + const routes: PluginRouteConfig[] = sortRoutes( + [...aggregateRoutes(plugins), ...allContentLoadedResult.routes], + baseUrl, + ); const globalData: GlobalData = mergeGlobalData( aggregateGlobalData(plugins), @@ -279,6 +280,7 @@ export async function loadPlugins( }); const {routes, globalData} = mergeResults({ + baseUrl: context.baseUrl, plugins, allContentLoadedResult, }); @@ -324,6 +326,7 @@ export async function reloadPlugin({ }); const {routes, globalData} = mergeResults({ + baseUrl: context.baseUrl, plugins, allContentLoadedResult, }); diff --git a/packages/docusaurus/src/server/plugins/routeConfig.ts b/packages/docusaurus/src/server/plugins/routeConfig.ts index 6e0891bfd2de..7832499ecc5d 100644 --- a/packages/docusaurus/src/server/plugins/routeConfig.ts +++ b/packages/docusaurus/src/server/plugins/routeConfig.ts @@ -27,10 +27,11 @@ export function applyRouteTrailingSlash( }; } -export function sortRoutes( - routeConfigs: RouteConfig[], - baseUrl: string = '/', -): void { +export function sortRoutes( + routesToSort: Route[], + baseUrl: string, +): Route[] { + const routeConfigs = [...routesToSort]; // Sort the route config. This ensures that route with nested // routes is always placed last. routeConfigs.sort((a, b) => { @@ -48,6 +49,23 @@ export function sortRoutes( if (!a.routes && b.routes) { return -1; } + + // If both are parent routes (for example routeBasePath: "/" and "/docs/" + // We must order them carefully in case of overlapping paths + if (a.routes && b.routes) { + if (a.path === b.path) { + // We don't really support that kind of routing ATM + // React-Router by default will only "enter" a single parent route + } else { + if (a.path.includes(b.path)) { + return -1; + } + if (b.path.includes(a.path)) { + return 1; + } + } + } + // Higher priority get placed first. if (a.priority || b.priority) { const priorityA = a.priority ?? 0; @@ -64,7 +82,9 @@ export function sortRoutes( routeConfigs.forEach((routeConfig) => { if (routeConfig.routes) { - sortRoutes(routeConfig.routes, baseUrl); + routeConfig.routes = sortRoutes(routeConfig.routes, baseUrl); } }); + + return routeConfigs; }