fix(v2): redirect plugin should emit redirect files with lower precedence than redirect target (#5085)

* revert old behavior of the redirect plugin

* revert old behavior of the redirect plugin

packages/docusaurus-plugin-client-redirects/src/__tests__/writeRedirectFiles.test.ts

@@ -59,7 +59,7 @@ describe('toRedirectFilesMetadata', () => {
     );
 
     expect(redirectFiles.map((f) => f.fileAbsolutePath)).toEqual([
-      path.join(pluginContext.outDir, '/abc.html'),
+      path.join(pluginContext.outDir, '/abc.html/index.html'),
       path.join(pluginContext.outDir, '/def/index.html'),
       path.join(pluginContext.outDir, '/xyz/index.html'),
     ]);
@@ -86,7 +86,7 @@ describe('toRedirectFilesMetadata', () => {
     );
 
     expect(redirectFiles.map((f) => f.fileAbsolutePath)).toEqual([
-      path.join(pluginContext.outDir, '/abc.html'),
+      path.join(pluginContext.outDir, '/abc.html/index.html'),
       path.join(pluginContext.outDir, '/def/index.html'),
       path.join(pluginContext.outDir, '/xyz/index.html'),
     ]);
@@ -113,9 +113,9 @@ describe('toRedirectFilesMetadata', () => {
     );
 
     expect(redirectFiles.map((f) => f.fileAbsolutePath)).toEqual([
-      path.join(pluginContext.outDir, '/abc.html'),
+      path.join(pluginContext.outDir, '/abc.html/index.html'),
-      path.join(pluginContext.outDir, '/def.html'),
+      path.join(pluginContext.outDir, '/def/index.html'),
-      path.join(pluginContext.outDir, '/xyz.html'),
+      path.join(pluginContext.outDir, '/xyz/index.html'),
     ]);
 
     expect(redirectFiles.map((f) => f.fileContent)).toMatchSnapshot(

packages/docusaurus-plugin-client-redirects/src/writeRedirectFiles.ts

@@ -11,7 +11,7 @@ import {memoize} from 'lodash';
 
 import {PluginContext, RedirectMetadata} from './types';
 import createRedirectPageContent from './createRedirectPageContent';
-import {getFilePathForRoutePath, normalizeUrl} from '@docusaurus/utils';
+import {normalizeUrl} from '@docusaurus/utils';
 
 export type WriteFilesPluginContext = Pick<PluginContext, 'baseUrl' | 'outDir'>;
 
@@ -24,6 +24,24 @@ 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)
+//
+// 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
+): string {
+  const fileName = path.basename(routePath);
+  const filePath = path.dirname(routePath);
+  return path.join(filePath, `${fileName}/index.html`);
+}
+
 export function toRedirectFilesMetadata(
   redirects: RedirectMetadata[],
   pluginContext: WriteFilesPluginContext,
@@ -37,8 +55,11 @@ export function toRedirectFilesMetadata(
   });
 
   const createFileMetadata = (redirect: RedirectMetadata) => {
-    const filePath = getFilePathForRoutePath(redirect.from, trailingSlash);
+    const fileRelativePath = getRedirectFilePathForRoutePath(
-    const fileAbsolutePath = path.join(pluginContext.outDir, filePath);
+      redirect.from,
+      trailingSlash,
+    );
+    const fileAbsolutePath = path.join(pluginContext.outDir, fileRelativePath);
     const toUrl = createToUrl(pluginContext.baseUrl, redirect.to);
     const fileContent = createPageContentMemoized(toUrl);
     return {

packages/docusaurus-utils/src/__tests__/getFilePathForRoutePath.test.ts

@@ -1,87 +0,0 @@
-/**
- * Copyright (c) Facebook, Inc. and its affiliates.
- *
- * This source code is licensed under the MIT license found in the
- * LICENSE file in the root directory of this source tree.
- */
-
-import {getFilePathForRoutePath} from '../getFilePathForRoutePath';
-import {posixPath} from '../posixPath';
-
-describe('getFilePathForRoutePath trailingSlash=undefined', () => {
-  test('works for /', () => {
-    expect(posixPath(getFilePathForRoutePath('/', undefined))).toEqual(
-      '/index.html',
-    );
-  });
-
-  test('works for /somePath', () => {
-    expect(posixPath(getFilePathForRoutePath('/somePath', undefined))).toEqual(
-      '/somePath/index.html',
-    );
-  });
-
-  test('works for /somePath/', () => {
-    expect(posixPath(getFilePathForRoutePath('/somePath/', undefined))).toEqual(
-      '/somePath/index.html',
-    );
-  });
-
-  test('works for /somePath/xyz.html', () => {
-    expect(
-      posixPath(getFilePathForRoutePath('/somePath/xyz.html', undefined)),
-    ).toEqual('/somePath/xyz.html');
-  });
-});
-
-describe('getFilePathForRoutePath trailingSlash=true', () => {
-  test('works for /', () => {
-    expect(posixPath(getFilePathForRoutePath('/', true))).toEqual(
-      '/index.html',
-    );
-  });
-
-  test('works for /somePath', () => {
-    expect(posixPath(getFilePathForRoutePath('/somePath', true))).toEqual(
-      '/somePath/index.html',
-    );
-  });
-
-  test('works for /somePath/', () => {
-    expect(posixPath(getFilePathForRoutePath('/somePath/', true))).toEqual(
-      '/somePath/index.html',
-    );
-  });
-
-  test('works for /somePath/xyz.html', () => {
-    expect(
-      posixPath(getFilePathForRoutePath('/somePath/xyz.html', true)),
-    ).toEqual('/somePath/xyz.html');
-  });
-});
-
-describe('getFilePathForRoutePath trailingSlash=false', () => {
-  test('works for /', () => {
-    expect(posixPath(getFilePathForRoutePath('/', false))).toEqual(
-      '/index.html',
-    );
-  });
-
-  test('works for /somePath', () => {
-    expect(posixPath(getFilePathForRoutePath('/somePath', false))).toEqual(
-      '/somePath.html',
-    );
-  });
-
-  test('works for /somePath/', () => {
-    expect(posixPath(getFilePathForRoutePath('/somePath/', false))).toEqual(
-      '/somePath/index.html',
-    );
-  });
-
-  test('works for /somePath/xyz.html', () => {
-    expect(
-      posixPath(getFilePathForRoutePath('/somePath/xyz.html', false)),
-    ).toEqual('/somePath/xyz.html');
-  });
-});

packages/docusaurus-utils/src/getFilePathForRoutePath.ts

@@ -1,43 +0,0 @@
-/**
- * Copyright (c) Facebook, Inc. and its affiliates.
- *
- * This source code is licensed under the MIT license found in the
- * LICENSE file in the root directory of this source tree.
- */
-
-import path from 'path';
-
-/*
-export function getFilePathForRoutePath(routePath: string): string {
-  const fileName = path.basename(routePath);
-  const filePath = path.dirname(routePath);
-  return path.join(filePath, `${fileName}/index.html`);
-}
- */
-
-// Almost exact copy of the behavior we implemented in our Docusaurus fork of the webpack static gen plugin
-// See https://github.com/slorber/static-site-generator-webpack-plugin/blob/master/index.js#L167
-export function getFilePathForRoutePath(
-  routePath: string,
-  trailingSlash: boolean | undefined,
-): string {
-  // const outputFileName = routePath.replace(/^(\/|\\)/, ''); // Remove leading slashes for webpack-dev-server
-
-  // Paths ending with .html are left untouched
-  if (/\.(html?)$/i.test(routePath)) {
-    return routePath;
-  }
-
-  // Legacy retro-compatible behavior
-  if (typeof trailingSlash === 'undefined') {
-    return path.join(routePath, 'index.html');
-  }
-
-  // New behavior: we can say if we prefer file/folder output
-  // Useful resource: https://github.com/slorber/trailing-slash-guide
-  if (routePath === '' || routePath.endsWith('/') || trailingSlash) {
-    return path.join(routePath, 'index.html');
-  } else {
-    return `${routePath}.html`;
-  }
-}

packages/docusaurus-utils/src/index.ts

@@ -26,7 +26,6 @@ import {simpleHash, docuHash} from './hashUtils';
 
 export const posixPath = posixPathImport;
 
-export * from './getFilePathForRoutePath';
 export * from './codeTranslationsUtils';
 export * from './markdownParser';
 export * from './markdownLinks';