From 09fcb9b173b8546fc2107e29a728b89a46e529a7 Mon Sep 17 00:00:00 2001 From: slorber Date: Wed, 30 Jun 2021 17:00:50 +0200 Subject: [PATCH 1/2] another redirect plugin fix when trailingSlash=false --- .../src/__tests__/writeRedirectFiles.test.ts | 3 +- .../src/writeRedirectFiles.ts | 41 +++++++++++-------- 2 files changed, 25 insertions(+), 19 deletions(-) diff --git a/packages/docusaurus-plugin-client-redirects/src/__tests__/writeRedirectFiles.test.ts b/packages/docusaurus-plugin-client-redirects/src/__tests__/writeRedirectFiles.test.ts index 6aa9c0501e49..b715250baffc 100644 --- a/packages/docusaurus-plugin-client-redirects/src/__tests__/writeRedirectFiles.test.ts +++ b/packages/docusaurus-plugin-client-redirects/src/__tests__/writeRedirectFiles.test.ts @@ -113,7 +113,8 @@ describe('toRedirectFilesMetadata', () => { ); expect(redirectFiles.map((f) => f.fileAbsolutePath)).toEqual([ - path.join(pluginContext.outDir, '/abc.html/index.html'), + // path.join(pluginContext.outDir, '/abc.html/index.html'), // Can't be used because /abc.html already exists, and file/folder can't share same name on Unix! + path.join(pluginContext.outDir, '/abc.html.html'), // Weird but on purpose! path.join(pluginContext.outDir, '/def/index.html'), path.join(pluginContext.outDir, '/xyz/index.html'), ]); diff --git a/packages/docusaurus-plugin-client-redirects/src/writeRedirectFiles.ts b/packages/docusaurus-plugin-client-redirects/src/writeRedirectFiles.ts index f81c66f4a35b..389a3432babd 100644 --- a/packages/docusaurus-plugin-client-redirects/src/writeRedirectFiles.ts +++ b/packages/docusaurus-plugin-client-redirects/src/writeRedirectFiles.ts @@ -24,22 +24,30 @@ export function createToUrl(baseUrl: string, to: string): string { return normalizeUrl([baseUrl, to]); } -// if the target path is /xyz, with file /xyz/index.html -// we don't want the redirect file to be /xyz.html -// otherwise it could be picked in priority and the redirect file would redirect to itself -// This can potentially create an infinite loop (depends on host, like Netlify) +// Create redirect file path +// Make sure this path has lower precedence over the original file path when served by host providers! +// Otherwise it can produce infinite redirect loops! // -// We prefer the redirect file to be /xyz.html/index.html, as it has lower serving priority for most static hosting tools -// It is also the historical behavior of this plugin and nobody complained -// See also https://github.com/facebook/docusaurus/issues/5055#issuecomment-870731207 -// See also PR reverting to historical behavior: https://github.com/facebook/docusaurus/pull/5085 -function getRedirectFilePathForRoutePath( - routePath: string, - _trailingSlash: boolean | undefined, // Now unused, on purpose +// See https://github.com/facebook/docusaurus/issues/5055 +// See https://github.com/facebook/docusaurus/pull/5085 +function getRedirectFilePath( + fromPath: string, + trailingSlash: boolean | undefined, // Now unused, on purpose ): string { - const fileName = path.basename(routePath); - const filePath = path.dirname(routePath); - return path.join(filePath, `${fileName}/index.html`); + const fileName = path.basename(fromPath); + const filePath = path.dirname(fromPath); + // If the redirect source path is /xyz, with file /xyz.html + // We can't write the redirect file at /xyz.html/index.html because for Unix FS, a file/folder can't have the same name "xyz.html" + // The only possible solution for a redirect file is thus /xyz.html.html (I know, looks suspicious) + if (trailingSlash === false && fileName.endsWith('.html')) { + return path.join(filePath, `${fileName}.html`); + } + // If the target path is /xyz, with file /xyz/index.html, we don't want the redirect file to be /xyz.html + // otherwise it would be picked in priority and the redirect file would redirect to itself + // We prefer the redirect file to be /xyz.html/index.html, served with lower priority for most static hosting tools + else { + return path.join(filePath, `${fileName}/index.html`); + } } export function toRedirectFilesMetadata( @@ -55,10 +63,7 @@ export function toRedirectFilesMetadata( }); const createFileMetadata = (redirect: RedirectMetadata) => { - const fileRelativePath = getRedirectFilePathForRoutePath( - redirect.from, - trailingSlash, - ); + const fileRelativePath = getRedirectFilePath(redirect.from, trailingSlash); const fileAbsolutePath = path.join(pluginContext.outDir, fileRelativePath); const toUrl = createToUrl(pluginContext.baseUrl, redirect.to); const fileContent = createPageContentMemoized(toUrl); From 88d9d11dd0e8a3ead8b7d2459c4710e4f9b84818 Mon Sep 17 00:00:00 2001 From: slorber Date: Wed, 30 Jun 2021 17:07:18 +0200 Subject: [PATCH 2/2] add comment --- .../src/writeRedirectFiles.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/docusaurus-plugin-client-redirects/src/writeRedirectFiles.ts b/packages/docusaurus-plugin-client-redirects/src/writeRedirectFiles.ts index 389a3432babd..44d7c851709f 100644 --- a/packages/docusaurus-plugin-client-redirects/src/writeRedirectFiles.ts +++ b/packages/docusaurus-plugin-client-redirects/src/writeRedirectFiles.ts @@ -30,12 +30,14 @@ export function createToUrl(baseUrl: string, to: string): string { // // See https://github.com/facebook/docusaurus/issues/5055 // See https://github.com/facebook/docusaurus/pull/5085 +// See https://github.com/facebook/docusaurus/pull/5102 function getRedirectFilePath( fromPath: string, trailingSlash: boolean | undefined, // Now unused, on purpose ): string { const fileName = path.basename(fromPath); const filePath = path.dirname(fromPath); + // Edge case for https://github.com/facebook/docusaurus/pull/5102 // If the redirect source path is /xyz, with file /xyz.html // We can't write the redirect file at /xyz.html/index.html because for Unix FS, a file/folder can't have the same name "xyz.html" // The only possible solution for a redirect file is thus /xyz.html.html (I know, looks suspicious)