Skip to content

fix(docusaurus-utils-validation): baseUrl + routeBasePath: allow empty string, normalized as "/" #8258

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.

Sign up for GitHub

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

Merged
slorber merged 6 commits into from
Dec 8, 2022
Merged

fix(docusaurus-utils-validation): baseUrl + routeBasePath: allow empty string, normalized as "/" #8258

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a4263d3
fix(docusaurus-utils-validation): empty routeBasePath replace / inste…
Djunnni Oct 27, 2022
0c56a0c
test: validationSchemas add test case
Djunnni Oct 27, 2022
978170f
test: update routeBasePathSchema case
Djunnni Oct 28, 2022
8c065d3
Merge branch 'main' into Djunnni/main
slorber Dec 8, 2022
1eb9827
normalize routeBasePath
slorber Dec 8, 2022
21a04ed
fix tests
slorber Dec 8, 2022
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
6 changes: 2 additions & 4 deletions packages/docusaurus-plugin-content-blog/src/options.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
RemarkPluginsSchema,
RehypePluginsSchema,
AdmonitionsSchema,
RouteBasePathSchema,
URISchema,
} from '@docusaurus/utils-validation';
import {GlobExcludeDefault} from '@docusaurus/utils';
Expand Down Expand Up @@ -56,10 +57,7 @@ const PluginOptionSchema = Joi.object<PluginOptions>({
archiveBasePath: Joi.string()
.default(DEFAULT_OPTIONS.archiveBasePath)
.allow(null),
routeBasePath: Joi.string()
// '' not allowed, see https://github.com/facebook/docusaurus/issues/3374
// .allow('')
.default(DEFAULT_OPTIONS.routeBasePath),
routeBasePath: RouteBasePathSchema.default(DEFAULT_OPTIONS.routeBasePath),
tagsBasePath: Joi.string().default(DEFAULT_OPTIONS.tagsBasePath),
include: Joi.array().items(Joi.string()).default(DEFAULT_OPTIONS.include),
exclude: Joi.array().items(Joi.string()).default(DEFAULT_OPTIONS.exclude),
Expand Down
6 changes: 2 additions & 4 deletions packages/docusaurus-plugin-content-docs/src/options.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
RemarkPluginsSchema,
RehypePluginsSchema,
AdmonitionsSchema,
RouteBasePathSchema,
URISchema,
} from '@docusaurus/utils-validation';
import {GlobExcludeDefault} from '@docusaurus/utils';
Expand Down Expand Up @@ -73,10 +74,7 @@ const OptionsSchema = Joi.object<PluginOptions>({
editUrl: Joi.alternatives().try(URISchema, Joi.function()),
editCurrentVersion: Joi.boolean().default(DEFAULT_OPTIONS.editCurrentVersion),
editLocalizedFiles: Joi.boolean().default(DEFAULT_OPTIONS.editLocalizedFiles),
routeBasePath: Joi.string()
// '' not allowed, see https://github.com/facebook/docusaurus/issues/3374
// .allow('') ""
.default(DEFAULT_OPTIONS.routeBasePath),
routeBasePath: RouteBasePathSchema.default(DEFAULT_OPTIONS.routeBasePath),
tagsBasePath: Joi.string().default(DEFAULT_OPTIONS.tagsBasePath),
// @ts-expect-error: deprecated
homePageId: Joi.any().forbidden().messages({
Expand Down
11 changes: 11 additions & 0 deletions packages/docusaurus-plugin-content-pages/src/__tests__/options.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,4 +51,15 @@ describe('normalizePagesPluginOptions', () => {
});
}).toThrowErrorMatchingInlineSnapshot(`""path" must be a string"`);
});

it('empty routeBasePath replace default path("/")', () => {
expect(
testValidate({
routeBasePath: '',
}),
).toEqual({
...defaultOptions,
routeBasePath: '/',
});
});
});
3 changes: 2 additions & 1 deletion packages/docusaurus-plugin-content-pages/src/options.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
RemarkPluginsSchema,
RehypePluginsSchema,
AdmonitionsSchema,
RouteBasePathSchema,
} from '@docusaurus/utils-validation';
import {GlobExcludeDefault} from '@docusaurus/utils';
import type {OptionValidationContext} from '@docusaurus/types';
Expand All @@ -30,7 +31,7 @@ export const DEFAULT_OPTIONS: PluginOptions = {

const PluginOptionSchema = Joi.object<PluginOptions>({
path: Joi.string().default(DEFAULT_OPTIONS.path),
routeBasePath: Joi.string().default(DEFAULT_OPTIONS.routeBasePath),
routeBasePath: RouteBasePathSchema.default(DEFAULT_OPTIONS.routeBasePath),
include: Joi.array().items(Joi.string()).default(DEFAULT_OPTIONS.include),
exclude: Joi.array().items(Joi.string()).default(DEFAULT_OPTIONS.exclude),
mdxPageComponent: Joi.string().default(DEFAULT_OPTIONS.mdxPageComponent),
Expand Down
8 changes: 8 additions & 0 deletions ...es/docusaurus-utils-validation/src/__tests__/__snapshots__/validationSchemas.test.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`validation schemas RouteBasePathSchema: for value=[] 1`] = `""value" must be a string"`;

exports[`validation schemas RouteBasePathSchema: for value={} 1`] = `""value" must be a string"`;

exports[`validation schemas RouteBasePathSchema: for value=3 1`] = `""value" must be a string"`;

exports[`validation schemas RouteBasePathSchema: for value=null 1`] = `""value" must be a string"`;

exports[`validation schemas admonitionsSchema: for value=[] 1`] = `""value" does not look like a valid admonitions config"`;

exports[`validation schemas admonitionsSchema: for value={"customTypes":{"myKeyword":{"keyword":"myKeyword","infima":true,"svg":"<svg width=\\"512px\\" height=\\"512px\\" viewBox=\\"0 0 512 512\\" xmlns=\\"http://www.w3.org/2000/svg\\"></svg>"}}} 1`] = `
Expand Down
18 changes: 18 additions & 0 deletions packages/docusaurus-utils-validation/src/__tests__/validationSchemas.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
PluginIdSchema,
URISchema,
PathnameSchema,
RouteBasePathSchema,
} from '../validationSchemas';

function createTestHelpers({
Expand Down Expand Up @@ -166,4 +167,21 @@ describe('validation schemas', () => {
testFail('foo');
testFail('https://github.com/foo');
});

it('routeBasePathSchema', () => {
const {testFail, testOK} = createTestHelpers({
schema: RouteBasePathSchema,
defaultValue: '',
});

testOK('');
testOK('/foo');
testOK('blog');
testOK(undefined);

testFail(3);
testFail([]);
testFail(null);
testFail({});
});
});
1 change: 1 addition & 0 deletions packages/docusaurus-utils-validation/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export {
RemarkPluginsSchema,
RehypePluginsSchema,
AdmonitionsSchema,
RouteBasePathSchema,
URISchema,
PathnameSchema,
FrontMatterTagsSchema,
Expand Down
3 changes: 3 additions & 0 deletions packages/docusaurus-utils-validation/src/validationSchemas.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,9 @@ export const PathnameSchema = Joi.string()
'{{#label}} is not a valid pathname. Pathname should start with slash and not contain any domain or query string.',
);

// joi empty string not allowed by default. so using empty('') for custom use
export const RouteBasePathSchema = Joi.string().empty('').default('');
Copy link
Collaborator

@slorber slorber Oct 28, 2022

Choose a reason for hiding this comment

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

Let's wait for a decision to be made in #8254 (comment)

I'd prefer to have allow('') instead of empty('') here, and not have a default value of '' => default should applied on a case by case basis.

Eventually we could use this scema for baseUrl too (rename to PathSegmentSchema?), and normalize both baseUrl + routeBasePath to have leading/trailing slashes

Copy link
Contributor Author

@Djunnni Djunnni Oct 28, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

When i did some of test, if use allow(''), it return ''

So i use empty('') then default('')

It gauarantee this case

joi.string().allow('').default('/') => ''
joi.string().empty('').default('').default('/') => '/'

joi.string().empty('').default('/') => '/' => this case can't gaurantee because it return undefined when i tested docusaurus-utils-validation

I found rootBasePath used '/', '', 'blogs' in this repo and then customized by default value individually

Copy link
Collaborator

@slorber slorber Oct 28, 2022

Choose a reason for hiding this comment

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

You forgot I initially suggested to add the same code as baseurl: .custom((value: string) => addLeadingSlash(addTrailingSlash(value))),

Copy link
Contributor Author

@Djunnni Djunnni Oct 28, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

i also test this case you announced,

when i input empty string ""
joi.string().custom((value: string) => addLeadingSlash(addTrailingSlash(value))) => throw error
joi.string().allow('').custom((value: string) => addLeadingSlash(addTrailingSlash(value)) => return ''

Copy link
Contributor Author

@Djunnni Djunnni Oct 28, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

example


const FrontMatterTagSchema = JoiFrontMatter.alternatives()
.try(
JoiFrontMatter.string().required(),
Expand Down