fix(v2): never remove trailing slash from site root like '/baseUrl/' (#5082)

* never apply trailingSlash to site root ('/baseUrl/') => only subroutes

* add deprecation comment for loadContext.baseUrl in favor of loadContext.siteConfig.baseUrl

* commit typo

* useless code
This commit is contained in:
Sébastien Lorber 2021-06-29 15:17:23 +02:00 committed by GitHub
parent 41b78466da
commit 7592982960
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 254 additions and 102 deletions

View file

@ -17,7 +17,10 @@ import {
createToExtensionsRedirects,
} from './extensionRedirects';
import {validateRedirect} from './redirectValidation';
import {applyTrailingSlash} from '@docusaurus/utils-common';
import {
applyTrailingSlash,
ApplyTrailingSlashParams,
} from '@docusaurus/utils-common';
import chalk from 'chalk';
@ -26,7 +29,12 @@ export default function collectRedirects(
trailingSlash: boolean | undefined,
): RedirectMetadata[] {
let redirects = doCollectRedirects(pluginContext);
redirects = applyRedirectsTrailingSlash(redirects, trailingSlash);
redirects = applyRedirectsTrailingSlash(redirects, {
trailingSlash,
baseUrl: pluginContext.baseUrl,
});
validateCollectedRedirects(redirects, pluginContext);
return filterUnwantedRedirects(redirects, pluginContext);
}
@ -37,12 +45,12 @@ export default function collectRedirects(
// It should be easy to toggle siteConfig.trailingSlash option without having to change other configs
function applyRedirectsTrailingSlash(
redirects: RedirectMetadata[],
trailingSlash: boolean | undefined,
params: ApplyTrailingSlashParams,
) {
return redirects.map((redirect) => {
return {
...redirect,
to: applyTrailingSlash(redirect.to, trailingSlash),
to: applyTrailingSlash(redirect.to, params),
};
});
}

View file

@ -32,7 +32,10 @@ export default async function createSitemap(
if (options.trailingSlash) {
return addTrailingSlash(routePath);
} else {
return applyTrailingSlash(routePath, siteConfig.trailingSlash);
return applyTrailingSlash(routePath, {
trailingSlash: siteConfig.trailingSlash,
baseUrl: siteConfig.baseUrl,
});
}
}

View file

@ -180,7 +180,7 @@ export interface LoadContext {
siteConfig: DocusaurusConfig;
siteConfigPath: string;
outDir: string;
baseUrl: string;
baseUrl: string; // TODO to remove: useless, there's already siteConfig.baseUrl!
i18n: I18n;
ssrTemplate?: string;
codeTranslations: Record<string, string>;

View file

@ -5,116 +5,174 @@
* LICENSE file in the root directory of this source tree.
*/
import applyTrailingSlash from '../applyTrailingSlash';
import applyTrailingSlash, {
ApplyTrailingSlashParams,
} from '../applyTrailingSlash';
function params(
trailingSlash: boolean | undefined,
baseUrl: string = '/',
): ApplyTrailingSlashParams {
return {trailingSlash, baseUrl};
}
describe('applyTrailingSlash', () => {
test('should apply to empty', () => {
expect(applyTrailingSlash('', true)).toEqual('/');
expect(applyTrailingSlash('', false)).toEqual('');
expect(applyTrailingSlash('', undefined)).toEqual('');
expect(applyTrailingSlash('', params(true))).toEqual('/');
expect(applyTrailingSlash('', params(false))).toEqual('');
expect(applyTrailingSlash('', params(undefined))).toEqual('');
});
test('should not apply to /', () => {
expect(applyTrailingSlash('/', true)).toEqual('/');
expect(applyTrailingSlash('/', false)).toEqual('/');
expect(applyTrailingSlash('/', undefined)).toEqual('/');
expect(applyTrailingSlash('/', params(true))).toEqual('/');
expect(applyTrailingSlash('/', params(false))).toEqual('/');
expect(applyTrailingSlash('/', params(undefined))).toEqual('/');
expect(applyTrailingSlash('/?query#anchor', true)).toEqual(
expect(applyTrailingSlash('/?query#anchor', params(true))).toEqual(
'/?query#anchor',
);
expect(applyTrailingSlash('/?query#anchor', false)).toEqual(
expect(applyTrailingSlash('/?query#anchor', params(false))).toEqual(
'/?query#anchor',
);
expect(applyTrailingSlash('/?query#anchor', undefined)).toEqual(
expect(applyTrailingSlash('/?query#anchor', params(undefined))).toEqual(
'/?query#anchor',
);
});
test('should not apply to /baseUrl/', () => {
const baseUrl = '/baseUrl/';
expect(applyTrailingSlash('/baseUrl/', params(true, baseUrl))).toEqual(
'/baseUrl/',
);
expect(applyTrailingSlash('/baseUrl/', params(false, baseUrl))).toEqual(
'/baseUrl/',
);
expect(applyTrailingSlash('/baseUrl/', params(undefined, baseUrl))).toEqual(
'/baseUrl/',
);
expect(
applyTrailingSlash('/baseUrl/?query#anchor', params(true, baseUrl)),
).toEqual('/baseUrl/?query#anchor');
expect(
applyTrailingSlash('/baseUrl/?query#anchor', params(false, baseUrl)),
).toEqual('/baseUrl/?query#anchor');
expect(
applyTrailingSlash('/baseUrl/?query#anchor', params(undefined, baseUrl)),
).toEqual('/baseUrl/?query#anchor');
});
test('should not apply to #anchor links ', () => {
expect(applyTrailingSlash('#', true)).toEqual('#');
expect(applyTrailingSlash('#', false)).toEqual('#');
expect(applyTrailingSlash('#', undefined)).toEqual('#');
expect(applyTrailingSlash('#anchor', true)).toEqual('#anchor');
expect(applyTrailingSlash('#anchor', false)).toEqual('#anchor');
expect(applyTrailingSlash('#anchor', undefined)).toEqual('#anchor');
expect(applyTrailingSlash('#', params(true))).toEqual('#');
expect(applyTrailingSlash('#', params(false))).toEqual('#');
expect(applyTrailingSlash('#', params(undefined))).toEqual('#');
expect(applyTrailingSlash('#anchor', params(true))).toEqual('#anchor');
expect(applyTrailingSlash('#anchor', params(false))).toEqual('#anchor');
expect(applyTrailingSlash('#anchor', params(undefined))).toEqual('#anchor');
});
test('should apply to simple paths', () => {
expect(applyTrailingSlash('abc', true)).toEqual('abc/');
expect(applyTrailingSlash('abc', false)).toEqual('abc');
expect(applyTrailingSlash('abc', undefined)).toEqual('abc');
expect(applyTrailingSlash('abc/', true)).toEqual('abc/');
expect(applyTrailingSlash('abc/', false)).toEqual('abc');
expect(applyTrailingSlash('abc/', undefined)).toEqual('abc/');
expect(applyTrailingSlash('/abc', true)).toEqual('/abc/');
expect(applyTrailingSlash('/abc', false)).toEqual('/abc');
expect(applyTrailingSlash('/abc', undefined)).toEqual('/abc');
expect(applyTrailingSlash('/abc/', true)).toEqual('/abc/');
expect(applyTrailingSlash('/abc/', false)).toEqual('/abc');
expect(applyTrailingSlash('/abc/', undefined)).toEqual('/abc/');
expect(applyTrailingSlash('abc', params(true))).toEqual('abc/');
expect(applyTrailingSlash('abc', params(false))).toEqual('abc');
expect(applyTrailingSlash('abc', params(undefined))).toEqual('abc');
expect(applyTrailingSlash('abc/', params(true))).toEqual('abc/');
expect(applyTrailingSlash('abc/', params(false))).toEqual('abc');
expect(applyTrailingSlash('abc/', params(undefined))).toEqual('abc/');
expect(applyTrailingSlash('/abc', params(true))).toEqual('/abc/');
expect(applyTrailingSlash('/abc', params(false))).toEqual('/abc');
expect(applyTrailingSlash('/abc', params(undefined))).toEqual('/abc');
expect(applyTrailingSlash('/abc/', params(true))).toEqual('/abc/');
expect(applyTrailingSlash('/abc/', params(false))).toEqual('/abc');
expect(applyTrailingSlash('/abc/', params(undefined))).toEqual('/abc/');
});
test('should apply to path with #anchor', () => {
expect(applyTrailingSlash('/abc#anchor', true)).toEqual('/abc/#anchor');
expect(applyTrailingSlash('/abc#anchor', false)).toEqual('/abc#anchor');
expect(applyTrailingSlash('/abc#anchor', undefined)).toEqual('/abc#anchor');
expect(applyTrailingSlash('/abc/#anchor', true)).toEqual('/abc/#anchor');
expect(applyTrailingSlash('/abc/#anchor', false)).toEqual('/abc#anchor');
expect(applyTrailingSlash('/abc/#anchor', undefined)).toEqual(
expect(applyTrailingSlash('/abc#anchor', params(true))).toEqual(
'/abc/#anchor',
);
expect(applyTrailingSlash('/abc#anchor', params(false))).toEqual(
'/abc#anchor',
);
expect(applyTrailingSlash('/abc#anchor', params(undefined))).toEqual(
'/abc#anchor',
);
expect(applyTrailingSlash('/abc/#anchor', params(true))).toEqual(
'/abc/#anchor',
);
expect(applyTrailingSlash('/abc/#anchor', params(false))).toEqual(
'/abc#anchor',
);
expect(applyTrailingSlash('/abc/#anchor', params(undefined))).toEqual(
'/abc/#anchor',
);
});
test('should apply to path with ?search', () => {
expect(applyTrailingSlash('/abc?search', true)).toEqual('/abc/?search');
expect(applyTrailingSlash('/abc?search', false)).toEqual('/abc?search');
expect(applyTrailingSlash('/abc?search', undefined)).toEqual('/abc?search');
expect(applyTrailingSlash('/abc/?search', true)).toEqual('/abc/?search');
expect(applyTrailingSlash('/abc/?search', false)).toEqual('/abc?search');
expect(applyTrailingSlash('/abc/?search', undefined)).toEqual(
expect(applyTrailingSlash('/abc?search', params(true))).toEqual(
'/abc/?search',
);
expect(applyTrailingSlash('/abc?search', params(false))).toEqual(
'/abc?search',
);
expect(applyTrailingSlash('/abc?search', params(undefined))).toEqual(
'/abc?search',
);
expect(applyTrailingSlash('/abc/?search', params(true))).toEqual(
'/abc/?search',
);
expect(applyTrailingSlash('/abc/?search', params(false))).toEqual(
'/abc?search',
);
expect(applyTrailingSlash('/abc/?search', params(undefined))).toEqual(
'/abc/?search',
);
});
test('should apply to path with ?search#anchor', () => {
expect(applyTrailingSlash('/abc?search#anchor', true)).toEqual(
expect(applyTrailingSlash('/abc?search#anchor', params(true))).toEqual(
'/abc/?search#anchor',
);
expect(applyTrailingSlash('/abc?search#anchor', false)).toEqual(
expect(applyTrailingSlash('/abc?search#anchor', params(false))).toEqual(
'/abc?search#anchor',
);
expect(applyTrailingSlash('/abc?search#anchor', undefined)).toEqual(
expect(applyTrailingSlash('/abc?search#anchor', params(undefined))).toEqual(
'/abc?search#anchor',
);
expect(applyTrailingSlash('/abc/?search#anchor', true)).toEqual(
expect(applyTrailingSlash('/abc/?search#anchor', params(true))).toEqual(
'/abc/?search#anchor',
);
expect(applyTrailingSlash('/abc/?search#anchor', false)).toEqual(
expect(applyTrailingSlash('/abc/?search#anchor', params(false))).toEqual(
'/abc?search#anchor',
);
expect(applyTrailingSlash('/abc/?search#anchor', undefined)).toEqual(
'/abc/?search#anchor',
);
expect(
applyTrailingSlash('/abc/?search#anchor', params(undefined)),
).toEqual('/abc/?search#anchor');
});
test('should apply to fully qualified urls', () => {
expect(
applyTrailingSlash('https://xyz.com/abc?search#anchor', true),
applyTrailingSlash('https://xyz.com/abc?search#anchor', params(true)),
).toEqual('https://xyz.com/abc/?search#anchor');
expect(
applyTrailingSlash('https://xyz.com/abc?search#anchor', false),
applyTrailingSlash('https://xyz.com/abc?search#anchor', params(false)),
).toEqual('https://xyz.com/abc?search#anchor');
expect(
applyTrailingSlash('https://xyz.com/abc?search#anchor', undefined),
applyTrailingSlash(
'https://xyz.com/abc?search#anchor',
params(undefined),
),
).toEqual('https://xyz.com/abc?search#anchor');
expect(
applyTrailingSlash('https://xyz.com/abc/?search#anchor', true),
applyTrailingSlash('https://xyz.com/abc/?search#anchor', params(true)),
).toEqual('https://xyz.com/abc/?search#anchor');
expect(
applyTrailingSlash('https://xyz.com/abc/?search#anchor', false),
applyTrailingSlash('https://xyz.com/abc/?search#anchor', params(false)),
).toEqual('https://xyz.com/abc?search#anchor');
expect(
applyTrailingSlash('https://xyz.com/abc/?search#anchor', undefined),
applyTrailingSlash(
'https://xyz.com/abc/?search#anchor',
params(undefined),
),
).toEqual('https://xyz.com/abc/?search#anchor');
});
});

View file

@ -5,10 +5,20 @@
* LICENSE file in the root directory of this source tree.
*/
import type {DocusaurusConfig} from '@docusaurus/types';
export type ApplyTrailingSlashParams = Pick<
DocusaurusConfig,
'trailingSlash' | 'baseUrl'
>;
// Trailing slash handling depends in some site configuration options
export default function applyTrailingSlash(
path: string,
trailingSlash: boolean | undefined,
options: ApplyTrailingSlashParams,
): string {
const {trailingSlash, baseUrl} = options;
if (path.startsWith('#')) {
// Never apply trailing slash to an anchor link
return path;
@ -34,7 +44,14 @@ export default function applyTrailingSlash(
const [pathname] = path.split(/[#?]/);
// Never transform '/' to ''
const newPathname =
pathname === '/' ? '/' : handleTrailingSlash(pathname, trailingSlash);
// Never remove the baseUrl trailing slash!
// If baseUrl = /myBase/, we want to emit /myBase/index.html and not /myBase.html !
// See https://github.com/facebook/docusaurus/issues/5077
const shouldNotApply = pathname === '/' || pathname === baseUrl;
const newPathname = shouldNotApply
? pathname
: handleTrailingSlash(pathname, trailingSlash);
return path.replace(pathname, newPathname);
}

View file

@ -6,4 +6,6 @@
*/
export {default as applyTrailingSlash} from './applyTrailingSlash';
export type {ApplyTrailingSlashParams} from './applyTrailingSlash';
export {default as uniq} from './uniq';

View file

@ -42,7 +42,7 @@ function Link({
...props
}: LinkProps): JSX.Element {
const {
siteConfig: {trailingSlash},
siteConfig: {trailingSlash, baseUrl},
} = useDocusaurusContext();
const {withBaseUrl} = useBaseUrlUtils();
const linksCollector = useLinksCollector();
@ -80,7 +80,7 @@ function Link({
: undefined;
if (targetLink && isInternal) {
targetLink = applyTrailingSlash(targetLink, trailingSlash);
targetLink = applyTrailingSlash(targetLink, {trailingSlash, baseUrl});
}
const preloaded = useRef(false);

View file

@ -116,7 +116,7 @@ export async function loadContext(
siteConfig,
siteConfigPath,
outDir,
baseUrl,
baseUrl, // TODO to remove: useless, there's already siteConfig.baseUrl! (and yes, it's the same value, cf code above)
i18n,
ssrTemplate,
codeTranslations,

View file

@ -7,6 +7,7 @@
import applyRouteTrailingSlash from '../applyRouteTrailingSlash';
import {RouteConfig} from '@docusaurus/types';
import {ApplyTrailingSlashParams} from '@docusaurus/utils-common';
function route(path: string, subRoutes?: string[]): RouteConfig {
const result: RouteConfig = {path, component: 'any'};
@ -18,76 +19,126 @@ function route(path: string, subRoutes?: string[]): RouteConfig {
return result;
}
function params(
trailingSlash: boolean | undefined,
baseUrl: string = '/',
): ApplyTrailingSlashParams {
return {trailingSlash, baseUrl};
}
describe('applyRouteTrailingSlash', () => {
test('apply to empty', () => {
expect(applyRouteTrailingSlash(route(''), true)).toEqual(route('/'));
expect(applyRouteTrailingSlash(route(''), false)).toEqual(route(''));
expect(applyRouteTrailingSlash(route(''), undefined)).toEqual(route(''));
expect(applyRouteTrailingSlash(route(''), params(true))).toEqual(
route('/'),
);
expect(applyRouteTrailingSlash(route(''), params(false))).toEqual(
route(''),
);
expect(applyRouteTrailingSlash(route(''), params(undefined))).toEqual(
route(''),
);
});
test('apply to /', () => {
expect(applyRouteTrailingSlash(route('/'), true)).toEqual(route('/'));
expect(applyRouteTrailingSlash(route('/'), false)).toEqual(route('/'));
expect(applyRouteTrailingSlash(route('/'), undefined)).toEqual(route('/'));
expect(applyRouteTrailingSlash(route('/'), params(true))).toEqual(
route('/'),
);
expect(applyRouteTrailingSlash(route('/'), params(false))).toEqual(
route('/'),
);
expect(applyRouteTrailingSlash(route('/'), params(undefined))).toEqual(
route('/'),
);
});
test('apply to /abc', () => {
expect(applyRouteTrailingSlash(route('/abc'), true)).toEqual(
expect(applyRouteTrailingSlash(route('/abc'), params(true))).toEqual(
route('/abc/'),
);
expect(applyRouteTrailingSlash(route('/abc'), false)).toEqual(
expect(applyRouteTrailingSlash(route('/abc'), params(false))).toEqual(
route('/abc'),
);
expect(applyRouteTrailingSlash(route('/abc'), undefined)).toEqual(
expect(applyRouteTrailingSlash(route('/abc'), params(undefined))).toEqual(
route('/abc'),
);
});
test('apply to /abc/', () => {
expect(applyRouteTrailingSlash(route('/abc/'), true)).toEqual(
expect(applyRouteTrailingSlash(route('/abc/'), params(true))).toEqual(
route('/abc/'),
);
expect(applyRouteTrailingSlash(route('/abc/'), false)).toEqual(
expect(applyRouteTrailingSlash(route('/abc/'), params(false))).toEqual(
route('/abc'),
);
expect(applyRouteTrailingSlash(route('/abc/'), undefined)).toEqual(
expect(applyRouteTrailingSlash(route('/abc/'), params(undefined))).toEqual(
route('/abc/'),
);
});
test('apply to /abc?search#anchor', () => {
expect(applyRouteTrailingSlash(route('/abc?search#anchor'), true)).toEqual(
route('/abc/?search#anchor'),
);
expect(applyRouteTrailingSlash(route('/abc?search#anchor'), false)).toEqual(
route('/abc?search#anchor'),
);
expect(
applyRouteTrailingSlash(route('/abc?search#anchor'), undefined),
applyRouteTrailingSlash(route('/abc?search#anchor'), params(true)),
).toEqual(route('/abc/?search#anchor'));
expect(
applyRouteTrailingSlash(route('/abc?search#anchor'), params(false)),
).toEqual(route('/abc?search#anchor'));
expect(
applyRouteTrailingSlash(route('/abc?search#anchor'), params(undefined)),
).toEqual(route('/abc?search#anchor'));
});
test('apply to /abc/?search#anchor', () => {
expect(applyRouteTrailingSlash(route('/abc/?search#anchor'), true)).toEqual(
route('/abc/?search#anchor'),
);
expect(
applyRouteTrailingSlash(route('/abc/?search#anchor'), false),
applyRouteTrailingSlash(route('/abc/?search#anchor'), params(true)),
).toEqual(route('/abc/?search#anchor'));
expect(
applyRouteTrailingSlash(route('/abc/?search#anchor'), params(false)),
).toEqual(route('/abc?search#anchor'));
expect(
applyRouteTrailingSlash(route('/abc/?search#anchor'), undefined),
applyRouteTrailingSlash(route('/abc/?search#anchor'), params(undefined)),
).toEqual(route('/abc/?search#anchor'));
});
test('not apply to /abc/?search#anchor when baseUrl=/abc/', () => {
const baseUrl = '/abc/';
expect(
applyRouteTrailingSlash(
route('/abc/?search#anchor'),
params(true, baseUrl),
),
).toEqual(route('/abc/?search#anchor'));
expect(
applyRouteTrailingSlash(
route('/abc/?search#anchor'),
params(false, baseUrl),
),
).toEqual(route('/abc/?search#anchor'));
expect(
applyRouteTrailingSlash(
route('/abc/?search#anchor'),
params(undefined, baseUrl),
),
).toEqual(route('/abc/?search#anchor'));
});
test('apply to subroutes', () => {
expect(
applyRouteTrailingSlash(route('/abc', ['/abc/1', '/abc/2']), true),
applyRouteTrailingSlash(
route('/abc', ['/abc/1', '/abc/2']),
params(true),
),
).toEqual(route('/abc/', ['/abc/1/', '/abc/2/']));
expect(
applyRouteTrailingSlash(route('/abc', ['/abc/1', '/abc/2']), false),
applyRouteTrailingSlash(
route('/abc', ['/abc/1', '/abc/2']),
params(false),
),
).toEqual(route('/abc', ['/abc/1', '/abc/2']));
expect(
applyRouteTrailingSlash(route('/abc', ['/abc/1', '/abc/2']), undefined),
applyRouteTrailingSlash(
route('/abc', ['/abc/1', '/abc/2']),
params(undefined),
),
).toEqual(route('/abc', ['/abc/1', '/abc/2']));
});
@ -95,10 +146,20 @@ describe('applyRouteTrailingSlash', () => {
expect(
applyRouteTrailingSlash(
route('/abc?search#anchor', ['/abc/1?search', '/abc/2#anchor']),
true,
params(true),
),
).toEqual(
route('/abc/?search#anchor', ['/abc/1/?search', '/abc/2/#anchor']),
);
});
test('apply for complex case with baseUrl', () => {
const baseUrl = '/abc/';
expect(
applyRouteTrailingSlash(
route('/abc/?search#anchor', ['/abc/1?search', '/abc/2#anchor']),
params(false, baseUrl),
),
).toEqual(route('/abc/?search#anchor', ['/abc/1?search', '/abc/2#anchor']));
});
});

View file

@ -6,18 +6,21 @@
*/
import {RouteConfig} from '@docusaurus/types';
import {applyTrailingSlash} from '@docusaurus/utils-common';
import {
applyTrailingSlash,
ApplyTrailingSlashParams,
} from '@docusaurus/utils-common';
export default function applyRouteTrailingSlash(
route: RouteConfig,
trailingSlash: boolean | undefined,
params: ApplyTrailingSlashParams,
): RouteConfig {
return {
...route,
path: applyTrailingSlash(route.path, trailingSlash),
path: applyTrailingSlash(route.path, params),
...(route.routes && {
routes: route.routes.map((subroute) =>
applyRouteTrailingSlash(subroute, trailingSlash),
applyRouteTrailingSlash(subroute, params),
),
}),
};

View file

@ -142,10 +142,10 @@ export async function loadPlugins({
initialRouteConfig,
) => {
// Trailing slash behavior is handled in a generic way for all plugins
const finalRouteConfig = applyRouteTrailingSlash(
initialRouteConfig,
context.siteConfig.trailingSlash,
);
const finalRouteConfig = applyRouteTrailingSlash(initialRouteConfig, {
trailingSlash: context.siteConfig.trailingSlash,
baseUrl: context.siteConfig.baseUrl,
});
pluginsRouteConfigs.push(finalRouteConfig);
};