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): make client-redirect-plugin not baseUrl sensitive #3010

Merged
merged 8 commits into from
Jun 29, 2020
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@ import {removeTrailingSlash} from '@docusaurus/utils';

function createTestPluginContext(
options?: UserPluginOptions,
routesPaths: string[] = [],
relativeRoutesPaths: string[] = [],
): PluginContext {
return {
outDir: '/tmp',
baseUrl: 'https://docusaurus.io',
routesPaths,
relativeRoutesPaths,
options: normalizePluginOptions(options),
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,33 +15,32 @@ const createExtensionValidationTests = (
extensionRedirectCreatorFn: (
paths: string[],
extensions: string[],
baseUrl: string,
) => RedirectMetadata[],
) => {
test('should reject empty extensions', () => {
expect(() => {
extensionRedirectCreatorFn(['/'], ['.html'], '/');
extensionRedirectCreatorFn(['/'], ['.html']);
}).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."`,
);
});
test('should reject extensions with .', () => {
expect(() => {
extensionRedirectCreatorFn(['/'], ['.html'], '/');
extensionRedirectCreatorFn(['/'], ['.html']);
}).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."`,
);
});
test('should reject extensions with /', () => {
expect(() => {
extensionRedirectCreatorFn(['/'], ['ht/ml'], '/');
extensionRedirectCreatorFn(['/'], ['ht/ml']);
}).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."`,
);
});
test('should reject extensions with illegal url char', () => {
expect(() => {
extensionRedirectCreatorFn(['/'], [','], '/');
extensionRedirectCreatorFn(['/'], [',']);
}).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."`,
);
Expand All @@ -53,28 +52,28 @@ describe('createToExtensionsRedirects', () => {

test('should create redirects from html/htm extensions', () => {
const ext = ['html', 'htm'];
expect(createToExtensionsRedirects([''], ext, '/')).toEqual([]);
expect(createToExtensionsRedirects(['/'], ext, '/')).toEqual([]);
expect(createToExtensionsRedirects(['/abc.html'], ext, '/')).toEqual([
expect(createToExtensionsRedirects([''], ext)).toEqual([]);
expect(createToExtensionsRedirects(['/'], ext)).toEqual([]);
expect(createToExtensionsRedirects(['/abc.html'], ext)).toEqual([
{from: '/abc', to: '/abc.html'},
]);
expect(createToExtensionsRedirects(['/abc.htm'], ext, '/')).toEqual([
expect(createToExtensionsRedirects(['/abc.htm'], ext)).toEqual([
{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(
createToExtensionsRedirects(['/prefix/file.html'], ['html'], '/prefix/'),
).toEqual([{from: '/file', to: '/file.html'}]);
createToExtensionsRedirects(['/prefix/file.html'], ['html']),
).toEqual([{from: '/prefix/file', to: '/prefix/file.html'}]);
});

test('should not create redirection for an empty extension array', () => {
const ext: string[] = [];
expect(createToExtensionsRedirects([''], ext, '/')).toEqual([]);
expect(createToExtensionsRedirects(['/'], ext, '/')).toEqual([]);
expect(createToExtensionsRedirects(['/abc.html'], ext, '/')).toEqual([]);
expect(createToExtensionsRedirects([''], ext)).toEqual([]);
expect(createToExtensionsRedirects(['/'], ext)).toEqual([]);
expect(createToExtensionsRedirects(['/abc.html'], ext)).toEqual([]);
});
});

Expand All @@ -83,28 +82,28 @@ describe('createFromExtensionsRedirects', () => {

test('should create redirects to html/htm extensions', () => {
const ext = ['html', 'htm'];
expect(createFromExtensionsRedirects([''], ext, '/')).toEqual([]);
expect(createFromExtensionsRedirects(['/'], ext, '/')).toEqual([]);
expect(createFromExtensionsRedirects(['/abc'], ext, '/')).toEqual([
expect(createFromExtensionsRedirects([''], ext)).toEqual([]);
expect(createFromExtensionsRedirects(['/'], ext)).toEqual([]);
expect(createFromExtensionsRedirects(['/abc'], ext)).toEqual([
{from: '/abc.html', to: '/abc'},
{from: '/abc.htm', to: '/abc'},
]);
expect(createFromExtensionsRedirects(['/def.html'], ext, '/')).toEqual([]);
expect(createFromExtensionsRedirects(['/def/'], ext, '/')).toEqual([]);
expect(createFromExtensionsRedirects(['/def.html'], ext)).toEqual([]);
expect(createFromExtensionsRedirects(['/def/'], ext)).toEqual([]);
});

test('should create "from" redirects without baseUrl when baseUrl is used', () => {
expect(
createFromExtensionsRedirects(['/prefix/file'], ['html'], '/prefix/'),
).toEqual([{from: '/file.html', to: '/file'}]);
test('should create "from" redirects when relativeRoutesPath contains a prefix', () => {
expect(createFromExtensionsRedirects(['/prefix/file'], ['html'])).toEqual([
{from: '/prefix/file.html', to: '/prefix/file'},
]);
});

test('should not create redirection for an empty extension array', () => {
const ext: string[] = [];
expect(createFromExtensionsRedirects([''], ext, '/')).toEqual([]);
expect(createFromExtensionsRedirects(['/'], ext, '/')).toEqual([]);
expect(createFromExtensionsRedirects(['/abc'], ext, '/')).toEqual([]);
expect(createFromExtensionsRedirects(['/def.html'], ext, '/')).toEqual([]);
expect(createFromExtensionsRedirects(['/def/'], ext, '/')).toEqual([]);
expect(createFromExtensionsRedirects([''], ext)).toEqual([]);
expect(createFromExtensionsRedirects(['/'], ext)).toEqual([]);
expect(createFromExtensionsRedirects(['/abc'], ext)).toEqual([]);
expect(createFromExtensionsRedirects(['/def.html'], ext)).toEqual([]);
expect(createFromExtensionsRedirects(['/def/'], ext)).toEqual([]);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,7 @@ function validateCollectedRedirects(
);
}

const allowedToPaths = pluginContext.routesPaths.map((path) =>
path.replace(pluginContext.baseUrl, '/'),
);
const allowedToPaths = pluginContext.relativeRoutesPaths;
const toPaths = redirects.map((redirect) => redirect.to);
const illegalToPaths = difference(toPaths, allowedToPaths);
if (illegalToPaths.length > 0) {
Expand Down Expand Up @@ -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!
const redirectsOverridingExistingPath = redirects.filter((redirect) =>
pluginContext.routesPaths.includes(redirect.from),
pluginContext.relativeRoutesPaths.includes(redirect.from),
);
if (redirectsOverridingExistingPath.length > 0) {
console.error(
Expand All @@ -103,7 +101,7 @@ It is not possible to redirect the same pathname to multiple destinations:
);
}
redirects = redirects.filter(
(redirect) => !pluginContext.routesPaths.includes(redirect.from),
(redirect) => !pluginContext.relativeRoutesPaths.includes(redirect.from),
);

return redirects;
Expand All @@ -113,18 +111,16 @@ It is not possible to redirect the same pathname to multiple destinations:
function doCollectRedirects(pluginContext: PluginContext): RedirectMetadata[] {
return [
...createFromExtensionsRedirects(
pluginContext.routesPaths,
pluginContext.relativeRoutesPaths,
pluginContext.options.fromExtensions,
pluginContext.baseUrl,
),
...createToExtensionsRedirects(
pluginContext.routesPaths,
pluginContext.relativeRoutesPaths,
pluginContext.options.toExtensions,
pluginContext.baseUrl,
),
...createRedirectsOptionRedirects(pluginContext.options.redirects),
...createCreateRedirectsOptionRedirects(
pluginContext.routesPaths,
pluginContext.relativeRoutesPaths,
pluginContext.options.createRedirects,
),
];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ const addLeadingDot = (extension: string) => `.${extension}`;
export function createToExtensionsRedirects(
paths: string[],
extensions: string[],
baseUrl: string,
): RedirectMetadata[] {
extensions.forEach(validateExtension);

Expand All @@ -54,8 +53,8 @@ export function createToExtensionsRedirects(
if (extensionFound) {
const routePathWithoutExtension = removeSuffix(path, extensionFound);
return [routePathWithoutExtension].map((from) => ({
from: trimBaseUrl(from, baseUrl),
to: trimBaseUrl(path, baseUrl),
from,
to: path,
}));
}
return [];
Expand All @@ -68,7 +67,6 @@ export function createToExtensionsRedirects(
export function createFromExtensionsRedirects(
paths: string[],
extensions: string[],
baseUrl: string,
): RedirectMetadata[] {
extensions.forEach(validateExtension);

Expand All @@ -82,14 +80,10 @@ export function createFromExtensionsRedirects(
return [];
}
return extensions.map((ext) => ({
from: `${trimBaseUrl(path, baseUrl)}.${ext}`,
to: trimBaseUrl(path, baseUrl),
from: `${path}.${ext}`,
to: path,
}));
};

return flatten(paths.map(createPathRedirects));
}

function trimBaseUrl(path: string, baseUrl: string) {
return path.startsWith(baseUrl) ? path.replace(baseUrl, '/') : path;
}
5 changes: 4 additions & 1 deletion packages/docusaurus-plugin-client-redirects/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import writeRedirectFiles, {
toRedirectFilesMetadata,
RedirectFileMetadata,
} from './writeRedirectFiles';
import {removePrefix} from '@docusaurus/utils';

export default function pluginClientRedirectsPages(
_context: LoadContext,
Expand All @@ -25,7 +26,9 @@ export default function pluginClientRedirectsPages(
name: 'docusaurus-plugin-client-redirects',
async postBuild(props: Props) {
const pluginContext: PluginContext = {
routesPaths: props.routesPaths,
relativeRoutesPaths: props.routesPaths.map(
(path) => `/${removePrefix(path, props.baseUrl)}`,
),
baseUrl: props.baseUrl,
outDir: props.outDir,
options,
Expand Down
6 changes: 2 additions & 4 deletions packages/docusaurus-plugin-client-redirects/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,9 @@ export type RedirectOption = {
export type UserPluginOptions = Partial<PluginOptions>;

// The minimal infos the plugin needs to work
export type PluginContext = Pick<
Props,
'routesPaths' | 'outDir' | 'baseUrl'
> & {
export type PluginContext = Pick<Props, 'outDir' | 'baseUrl'> & {
options: PluginOptions;
relativeRoutesPaths: string[];
};

// In-memory representation of redirects we want: easier to test
Expand Down
16 changes: 16 additions & 0 deletions packages/docusaurus-utils/src/__tests__/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
addTrailingSlash,
removeTrailingSlash,
removeSuffix,
removePrefix,
getFilePathForRoutePath,
} from '../index';

Expand Down Expand Up @@ -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', () => {
test('works for /', () => {
expect(getFilePathForRoutePath('/')).toEqual('/index.html');
Expand Down
4 changes: 4 additions & 0 deletions packages/docusaurus-utils/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,10 @@ export function removeSuffix(str: string, suffix: string): string {
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 {
const fileName = path.basename(routePath);
const filePath = path.dirname(routePath);
Expand Down
22 changes: 11 additions & 11 deletions packages/docusaurus/src/server/configValidation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,23 +90,23 @@ export function validateConfig(
abortEarly: false,
});
if (error) {
const unknownFields = error.details.reduce((formatedError, err) => {
const unknownFields = error.details.reduce((formattedError, err) => {
if (err.type === 'object.unknown') {
return `${formatedError}"${err.path}",`;
return `${formattedError}"${err.path}",`;
}
return formatedError;
return formattedError;
}, '');
let formatedError = error.details.reduce(
(accumalatedErr, err) =>
let formattedError = error.details.reduce(
(accumulatedErr, err) =>
err.type !== 'object.unknown'
? `${accumalatedErr}${err.message}\n`
: accumalatedErr,
? `${accumulatedErr}${err.message}\n`
: accumulatedErr,
'',
);
formatedError = 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`
: formatedError;
throw new Error(formatedError);
formattedError = unknownFields
? `${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`
: formattedError;
throw new Error(formattedError);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not really related, next time make a other PR for these changes

} else {
return value;
}
Expand Down
6 changes: 3 additions & 3 deletions website/docusaurus.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@
const versions = require('./versions.json');

const allDocHomesPaths = [
'/docs',
'/docs/next',
...versions.slice(1).map((version) => `/docs/${version}`),
'/docs/',
'/docs/next/',
...versions.slice(1).map((version) => `/docs/${version}/`),
];

module.exports = {
Expand Down