fix(v2): make client-redirect-plugin not baseUrl sensitive (#3010)

* feat(v2): use relative routes path in postBuild hook

* feat(v2): use relativeRoutesPath in other methods and modify tests

* fix(v2): fix D2 client redirects and tests

* feat(v2): add tests for relativeRoutesPaths

* fix(v2): fix some typos in configValidation

* fix(v2): fix an eslint warning and restart github action

* refactor(v2): create a removePrefix method

* refactor(v2): inline unnecessary method
This commit is contained in:
Teik Jun 2020-06-30 00:38:28 +08:00 committed by GitHub
parent b58a53eae8
commit 2e055f4ae2
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 81 additions and 71 deletions

View file

@ -12,12 +12,12 @@ import {removeTrailingSlash} from '@docusaurus/utils';
function createTestPluginContext( function createTestPluginContext(
options?: UserPluginOptions, options?: UserPluginOptions,
routesPaths: string[] = [], relativeRoutesPaths: string[] = [],
): PluginContext { ): PluginContext {
return { return {
outDir: '/tmp', outDir: '/tmp',
baseUrl: 'https://docusaurus.io', baseUrl: 'https://docusaurus.io',
routesPaths, relativeRoutesPaths,
options: normalizePluginOptions(options), options: normalizePluginOptions(options),
}; };
} }

View file

@ -15,33 +15,32 @@ const createExtensionValidationTests = (
extensionRedirectCreatorFn: ( extensionRedirectCreatorFn: (
paths: string[], paths: string[],
extensions: string[], extensions: string[],
baseUrl: string,
) => RedirectMetadata[], ) => RedirectMetadata[],
) => { ) => {
test('should reject empty extensions', () => { test('should reject empty extensions', () => {
expect(() => { expect(() => {
extensionRedirectCreatorFn(['/'], ['.html'], '/'); extensionRedirectCreatorFn(['/'], ['.html']);
}).toThrowErrorMatchingInlineSnapshot( }).toThrowErrorMatchingInlineSnapshot(
`"Extension=['.html'] contains a . (dot) and is not allowed. If the redirect extension system is not good enough for your usecase, you can create redirects yourself with the 'createRedirects' plugin option."`, `"Extension=['.html'] contains a . (dot) and is not allowed. If the redirect extension system is not good enough for your usecase, you can create redirects yourself with the 'createRedirects' plugin option."`,
); );
}); });
test('should reject extensions with .', () => { test('should reject extensions with .', () => {
expect(() => { expect(() => {
extensionRedirectCreatorFn(['/'], ['.html'], '/'); extensionRedirectCreatorFn(['/'], ['.html']);
}).toThrowErrorMatchingInlineSnapshot( }).toThrowErrorMatchingInlineSnapshot(
`"Extension=['.html'] contains a . (dot) and is not allowed. If the redirect extension system is not good enough for your usecase, you can create redirects yourself with the 'createRedirects' plugin option."`, `"Extension=['.html'] contains a . (dot) and is not allowed. If the redirect extension system is not good enough for your usecase, you can create redirects yourself with the 'createRedirects' plugin option."`,
); );
}); });
test('should reject extensions with /', () => { test('should reject extensions with /', () => {
expect(() => { expect(() => {
extensionRedirectCreatorFn(['/'], ['ht/ml'], '/'); extensionRedirectCreatorFn(['/'], ['ht/ml']);
}).toThrowErrorMatchingInlineSnapshot( }).toThrowErrorMatchingInlineSnapshot(
`"Extension=['ht/ml'] contains a / and is not allowed. If the redirect extension system is not good enough for your usecase, you can create redirects yourself with the 'createRedirects' plugin option."`, `"Extension=['ht/ml'] contains a / and is not allowed. If the redirect extension system is not good enough for your usecase, you can create redirects yourself with the 'createRedirects' plugin option."`,
); );
}); });
test('should reject extensions with illegal url char', () => { test('should reject extensions with illegal url char', () => {
expect(() => { expect(() => {
extensionRedirectCreatorFn(['/'], [','], '/'); extensionRedirectCreatorFn(['/'], [',']);
}).toThrowErrorMatchingInlineSnapshot( }).toThrowErrorMatchingInlineSnapshot(
`"Extension=[','] contains invalid uri characters. If the redirect extension system is not good enough for your usecase, you can create redirects yourself with the 'createRedirects' plugin option."`, `"Extension=[','] contains invalid uri characters. If the redirect extension system is not good enough for your usecase, you can create redirects yourself with the 'createRedirects' plugin option."`,
); );
@ -53,28 +52,28 @@ describe('createToExtensionsRedirects', () => {
test('should create redirects from html/htm extensions', () => { test('should create redirects from html/htm extensions', () => {
const ext = ['html', 'htm']; const ext = ['html', 'htm'];
expect(createToExtensionsRedirects([''], ext, '/')).toEqual([]); expect(createToExtensionsRedirects([''], ext)).toEqual([]);
expect(createToExtensionsRedirects(['/'], ext, '/')).toEqual([]); expect(createToExtensionsRedirects(['/'], ext)).toEqual([]);
expect(createToExtensionsRedirects(['/abc.html'], ext, '/')).toEqual([ expect(createToExtensionsRedirects(['/abc.html'], ext)).toEqual([
{from: '/abc', to: '/abc.html'}, {from: '/abc', to: '/abc.html'},
]); ]);
expect(createToExtensionsRedirects(['/abc.htm'], ext, '/')).toEqual([ expect(createToExtensionsRedirects(['/abc.htm'], ext)).toEqual([
{from: '/abc', to: '/abc.htm'}, {from: '/abc', to: '/abc.htm'},
]); ]);
expect(createToExtensionsRedirects(['/abc.xyz'], ext, '/')).toEqual([]); expect(createToExtensionsRedirects(['/abc.xyz'], ext)).toEqual([]);
}); });
test('should create "to" redirects without baseUrl when baseUrl is used', () => { test('should create "to" redirects when relativeRoutesPath contains a prefix', () => {
expect( expect(
createToExtensionsRedirects(['/prefix/file.html'], ['html'], '/prefix/'), createToExtensionsRedirects(['/prefix/file.html'], ['html']),
).toEqual([{from: '/file', to: '/file.html'}]); ).toEqual([{from: '/prefix/file', to: '/prefix/file.html'}]);
}); });
test('should not create redirection for an empty extension array', () => { test('should not create redirection for an empty extension array', () => {
const ext: string[] = []; const ext: string[] = [];
expect(createToExtensionsRedirects([''], ext, '/')).toEqual([]); expect(createToExtensionsRedirects([''], ext)).toEqual([]);
expect(createToExtensionsRedirects(['/'], ext, '/')).toEqual([]); expect(createToExtensionsRedirects(['/'], ext)).toEqual([]);
expect(createToExtensionsRedirects(['/abc.html'], ext, '/')).toEqual([]); expect(createToExtensionsRedirects(['/abc.html'], ext)).toEqual([]);
}); });
}); });
@ -83,28 +82,28 @@ describe('createFromExtensionsRedirects', () => {
test('should create redirects to html/htm extensions', () => { test('should create redirects to html/htm extensions', () => {
const ext = ['html', 'htm']; const ext = ['html', 'htm'];
expect(createFromExtensionsRedirects([''], ext, '/')).toEqual([]); expect(createFromExtensionsRedirects([''], ext)).toEqual([]);
expect(createFromExtensionsRedirects(['/'], ext, '/')).toEqual([]); expect(createFromExtensionsRedirects(['/'], ext)).toEqual([]);
expect(createFromExtensionsRedirects(['/abc'], ext, '/')).toEqual([ expect(createFromExtensionsRedirects(['/abc'], ext)).toEqual([
{from: '/abc.html', to: '/abc'}, {from: '/abc.html', to: '/abc'},
{from: '/abc.htm', to: '/abc'}, {from: '/abc.htm', to: '/abc'},
]); ]);
expect(createFromExtensionsRedirects(['/def.html'], ext, '/')).toEqual([]); expect(createFromExtensionsRedirects(['/def.html'], ext)).toEqual([]);
expect(createFromExtensionsRedirects(['/def/'], ext, '/')).toEqual([]); expect(createFromExtensionsRedirects(['/def/'], ext)).toEqual([]);
}); });
test('should create "from" redirects without baseUrl when baseUrl is used', () => { test('should create "from" redirects when relativeRoutesPath contains a prefix', () => {
expect( expect(createFromExtensionsRedirects(['/prefix/file'], ['html'])).toEqual([
createFromExtensionsRedirects(['/prefix/file'], ['html'], '/prefix/'), {from: '/prefix/file.html', to: '/prefix/file'},
).toEqual([{from: '/file.html', to: '/file'}]); ]);
}); });
test('should not create redirection for an empty extension array', () => { test('should not create redirection for an empty extension array', () => {
const ext: string[] = []; const ext: string[] = [];
expect(createFromExtensionsRedirects([''], ext, '/')).toEqual([]); expect(createFromExtensionsRedirects([''], ext)).toEqual([]);
expect(createFromExtensionsRedirects(['/'], ext, '/')).toEqual([]); expect(createFromExtensionsRedirects(['/'], ext)).toEqual([]);
expect(createFromExtensionsRedirects(['/abc'], ext, '/')).toEqual([]); expect(createFromExtensionsRedirects(['/abc'], ext)).toEqual([]);
expect(createFromExtensionsRedirects(['/def.html'], ext, '/')).toEqual([]); expect(createFromExtensionsRedirects(['/def.html'], ext)).toEqual([]);
expect(createFromExtensionsRedirects(['/def/'], ext, '/')).toEqual([]); expect(createFromExtensionsRedirects(['/def/'], ext)).toEqual([]);
}); });
}); });

View file

@ -50,9 +50,7 @@ function validateCollectedRedirects(
); );
} }
const allowedToPaths = pluginContext.routesPaths.map((path) => const allowedToPaths = pluginContext.relativeRoutesPaths;
path.replace(pluginContext.baseUrl, '/'),
);
const toPaths = redirects.map((redirect) => redirect.to); const toPaths = redirects.map((redirect) => redirect.to);
const illegalToPaths = difference(toPaths, allowedToPaths); const illegalToPaths = difference(toPaths, allowedToPaths);
if (illegalToPaths.length > 0) { if (illegalToPaths.length > 0) {
@ -91,7 +89,7 @@ It is not possible to redirect the same pathname to multiple destinations:
// We don't want to override an already existing route with a redirect file! // We don't want to override an already existing route with a redirect file!
const redirectsOverridingExistingPath = redirects.filter((redirect) => const redirectsOverridingExistingPath = redirects.filter((redirect) =>
pluginContext.routesPaths.includes(redirect.from), pluginContext.relativeRoutesPaths.includes(redirect.from),
); );
if (redirectsOverridingExistingPath.length > 0) { if (redirectsOverridingExistingPath.length > 0) {
console.error( console.error(
@ -103,7 +101,7 @@ It is not possible to redirect the same pathname to multiple destinations:
); );
} }
redirects = redirects.filter( redirects = redirects.filter(
(redirect) => !pluginContext.routesPaths.includes(redirect.from), (redirect) => !pluginContext.relativeRoutesPaths.includes(redirect.from),
); );
return redirects; return redirects;
@ -113,18 +111,16 @@ It is not possible to redirect the same pathname to multiple destinations:
function doCollectRedirects(pluginContext: PluginContext): RedirectMetadata[] { function doCollectRedirects(pluginContext: PluginContext): RedirectMetadata[] {
return [ return [
...createFromExtensionsRedirects( ...createFromExtensionsRedirects(
pluginContext.routesPaths, pluginContext.relativeRoutesPaths,
pluginContext.options.fromExtensions, pluginContext.options.fromExtensions,
pluginContext.baseUrl,
), ),
...createToExtensionsRedirects( ...createToExtensionsRedirects(
pluginContext.routesPaths, pluginContext.relativeRoutesPaths,
pluginContext.options.toExtensions, pluginContext.options.toExtensions,
pluginContext.baseUrl,
), ),
...createRedirectsOptionRedirects(pluginContext.options.redirects), ...createRedirectsOptionRedirects(pluginContext.options.redirects),
...createCreateRedirectsOptionRedirects( ...createCreateRedirectsOptionRedirects(
pluginContext.routesPaths, pluginContext.relativeRoutesPaths,
pluginContext.options.createRedirects, pluginContext.options.createRedirects,
), ),
]; ];

View file

@ -43,7 +43,6 @@ const addLeadingDot = (extension: string) => `.${extension}`;
export function createToExtensionsRedirects( export function createToExtensionsRedirects(
paths: string[], paths: string[],
extensions: string[], extensions: string[],
baseUrl: string,
): RedirectMetadata[] { ): RedirectMetadata[] {
extensions.forEach(validateExtension); extensions.forEach(validateExtension);
@ -54,8 +53,8 @@ export function createToExtensionsRedirects(
if (extensionFound) { if (extensionFound) {
const routePathWithoutExtension = removeSuffix(path, extensionFound); const routePathWithoutExtension = removeSuffix(path, extensionFound);
return [routePathWithoutExtension].map((from) => ({ return [routePathWithoutExtension].map((from) => ({
from: trimBaseUrl(from, baseUrl), from,
to: trimBaseUrl(path, baseUrl), to: path,
})); }));
} }
return []; return [];
@ -68,7 +67,6 @@ export function createToExtensionsRedirects(
export function createFromExtensionsRedirects( export function createFromExtensionsRedirects(
paths: string[], paths: string[],
extensions: string[], extensions: string[],
baseUrl: string,
): RedirectMetadata[] { ): RedirectMetadata[] {
extensions.forEach(validateExtension); extensions.forEach(validateExtension);
@ -82,14 +80,10 @@ export function createFromExtensionsRedirects(
return []; return [];
} }
return extensions.map((ext) => ({ return extensions.map((ext) => ({
from: `${trimBaseUrl(path, baseUrl)}.${ext}`, from: `${path}.${ext}`,
to: trimBaseUrl(path, baseUrl), to: path,
})); }));
}; };
return flatten(paths.map(createPathRedirects)); return flatten(paths.map(createPathRedirects));
} }
function trimBaseUrl(path: string, baseUrl: string) {
return path.startsWith(baseUrl) ? path.replace(baseUrl, '/') : path;
}

View file

@ -14,6 +14,7 @@ import writeRedirectFiles, {
toRedirectFilesMetadata, toRedirectFilesMetadata,
RedirectFileMetadata, RedirectFileMetadata,
} from './writeRedirectFiles'; } from './writeRedirectFiles';
import {removePrefix} from '@docusaurus/utils';
export default function pluginClientRedirectsPages( export default function pluginClientRedirectsPages(
_context: LoadContext, _context: LoadContext,
@ -25,7 +26,9 @@ export default function pluginClientRedirectsPages(
name: 'docusaurus-plugin-client-redirects', name: 'docusaurus-plugin-client-redirects',
async postBuild(props: Props) { async postBuild(props: Props) {
const pluginContext: PluginContext = { const pluginContext: PluginContext = {
routesPaths: props.routesPaths, relativeRoutesPaths: props.routesPaths.map(
(path) => `/${removePrefix(path, props.baseUrl)}`,
),
baseUrl: props.baseUrl, baseUrl: props.baseUrl,
outDir: props.outDir, outDir: props.outDir,
options, options,

View file

@ -28,11 +28,9 @@ export type RedirectOption = {
export type UserPluginOptions = Partial<PluginOptions>; export type UserPluginOptions = Partial<PluginOptions>;
// The minimal infos the plugin needs to work // The minimal infos the plugin needs to work
export type PluginContext = Pick< export type PluginContext = Pick<Props, 'outDir' | 'baseUrl'> & {
Props,
'routesPaths' | 'outDir' | 'baseUrl'
> & {
options: PluginOptions; options: PluginOptions;
relativeRoutesPaths: string[];
}; };
// In-memory representation of redirects we want: easier to test // In-memory representation of redirects we want: easier to test

View file

@ -22,6 +22,7 @@ import {
addTrailingSlash, addTrailingSlash,
removeTrailingSlash, removeTrailingSlash,
removeSuffix, removeSuffix,
removePrefix,
getFilePathForRoutePath, getFilePathForRoutePath,
} from '../index'; } from '../index';
@ -419,6 +420,21 @@ describe('removeSuffix', () => {
}); });
}); });
describe('removePrefix', () => {
test('should no-op 1', () => {
expect(removePrefix('abcdef', 'ijk')).toEqual('abcdef');
});
test('should no-op 2', () => {
expect(removePrefix('abcdef', 'def')).toEqual('abcdef');
});
test('should no-op 3', () => {
expect(removePrefix('abcdef', '')).toEqual('abcdef');
});
test('should remove prefix', () => {
expect(removePrefix('abcdef', 'ab')).toEqual('cdef');
});
});
describe('getFilePathForRoutePath', () => { describe('getFilePathForRoutePath', () => {
test('works for /', () => { test('works for /', () => {
expect(getFilePathForRoutePath('/')).toEqual('/index.html'); expect(getFilePathForRoutePath('/')).toEqual('/index.html');

View file

@ -384,6 +384,10 @@ export function removeSuffix(str: string, suffix: string): string {
return str.endsWith(suffix) ? str.slice(0, -suffix.length) : str; return str.endsWith(suffix) ? str.slice(0, -suffix.length) : str;
} }
export function removePrefix(str: string, prefix: string): string {
return str.startsWith(prefix) ? str.slice(prefix.length) : str;
}
export function getFilePathForRoutePath(routePath: string): string { export function getFilePathForRoutePath(routePath: string): string {
const fileName = path.basename(routePath); const fileName = path.basename(routePath);
const filePath = path.dirname(routePath); const filePath = path.dirname(routePath);

View file

@ -90,23 +90,23 @@ export function validateConfig(
abortEarly: false, abortEarly: false,
}); });
if (error) { if (error) {
const unknownFields = error.details.reduce((formatedError, err) => { const unknownFields = error.details.reduce((formattedError, err) => {
if (err.type === 'object.unknown') { if (err.type === 'object.unknown') {
return `${formatedError}"${err.path}",`; return `${formattedError}"${err.path}",`;
} }
return formatedError; return formattedError;
}, ''); }, '');
let formatedError = error.details.reduce( let formattedError = error.details.reduce(
(accumalatedErr, err) => (accumulatedErr, err) =>
err.type !== 'object.unknown' err.type !== 'object.unknown'
? `${accumalatedErr}${err.message}\n` ? `${accumulatedErr}${err.message}\n`
: accumalatedErr, : accumulatedErr,
'', '',
); );
formatedError = unknownFields formattedError = unknownFields
? `${formatedError}These field(s) [${unknownFields}] are not recognized in ${CONFIG_FILE_NAME}.\nIf you still want these fields to be in your configuration, put them in the 'customFields' attribute.\nSee https://v2.docusaurus.io/docs/docusaurus.config.js/#customfields` ? `${formattedError}These field(s) [${unknownFields}] are not recognized in ${CONFIG_FILE_NAME}.\nIf you still want these fields to be in your configuration, put them in the 'customFields' attribute.\nSee https://v2.docusaurus.io/docs/docusaurus.config.js/#customfields`
: formatedError; : formattedError;
throw new Error(formatedError); throw new Error(formattedError);
} else { } else {
return value; return value;
} }

View file

@ -8,9 +8,9 @@
const versions = require('./versions.json'); const versions = require('./versions.json');
const allDocHomesPaths = [ const allDocHomesPaths = [
'/docs', '/docs/',
'/docs/next', '/docs/next/',
...versions.slice(1).map((version) => `/docs/${version}`), ...versions.slice(1).map((version) => `/docs/${version}/`),
]; ];
module.exports = { module.exports = {