Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 4 commits into from
Jun 29, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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);
}
Expand All @@ -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),
};
});
}
Expand Down
5 changes: 4 additions & 1 deletion packages/docusaurus-plugin-sitemap/src/createSitemap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
});
}
}

Expand Down
2 changes: 1 addition & 1 deletion packages/docusaurus-types/src/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});
});
23 changes: 20 additions & 3 deletions packages/docusaurus-utils-common/src/applyTrailingSlash.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
}
2 changes: 2 additions & 0 deletions packages/docusaurus-utils-common/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,6 @@
*/

export {default as applyTrailingSlash} from './applyTrailingSlash';
export type {ApplyTrailingSlashParams} from './applyTrailingSlash';

export {default as uniq} from './uniq';
4 changes: 2 additions & 2 deletions packages/docusaurus/src/client/exports/Link.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ function Link({
...props
}: LinkProps): JSX.Element {
const {
siteConfig: {trailingSlash},
siteConfig: {trailingSlash, baseUrl},
} = useDocusaurusContext();
const {withBaseUrl} = useBaseUrlUtils();
const linksCollector = useLinksCollector();
Expand Down Expand Up @@ -80,7 +80,7 @@ function Link({
: undefined;

if (targetLink && isInternal) {
targetLink = applyTrailingSlash(targetLink, trailingSlash);
targetLink = applyTrailingSlash(targetLink, {trailingSlash, baseUrl});
}

const preloaded = useRef(false);
Expand Down
2 changes: 1 addition & 1 deletion packages/docusaurus/src/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading