fix(core): normalize slashes for url/baseUrl instead of throwing (#8066)

facebook · Oct 28, 2022 · c7301f5 · c7301f5
1 parent 00b2d6c
commit c7301f5
Show file tree

Hide file tree

Showing 4 changed files with 73 additions and 16 deletions.
diff --git a/packages/docusaurus-utils-validation/src/validationSchemas.ts b/packages/docusaurus-utils-validation/src/validationSchemas.ts
@@ -65,6 +65,9 @@ export const URISchema = Joi.alternatives(
   // This custom validation logic is required notably because Joi does not
   // accept paths like /a/b/c ...
   Joi.custom((val: unknown, helpers) => {
+    if (typeof val !== 'string') {
+      return helpers.error('any.invalid');
+    }
     try {
       // eslint-disable-next-line no-new
       new URL(String(val));

diff --git a/packages/docusaurus/src/server/__tests__/__snapshots__/configValidation.test.ts.snap b/packages/docusaurus/src/server/__tests__/__snapshots__/configValidation.test.ts.snap
@@ -138,11 +138,6 @@ exports[`normalizeConfig should throw error if themes is not array for the input
 "
 `;
 
-exports[`normalizeConfig throws error for baseUrl without trailing \`/\` 1`] = `
-""baseUrl" must be a string with a trailing slash.
-"
-`;
-
 exports[`normalizeConfig throws error for required fields 1`] = `
 ""baseUrl" is required
 "title" is required

diff --git a/packages/docusaurus/src/server/__tests__/configValidation.test.ts b/packages/docusaurus/src/server/__tests__/configValidation.test.ts
@@ -5,6 +5,7 @@
  * LICENSE file in the root directory of this source tree.
  */
 
+import {jest} from '@jest/globals';
 import {
   ConfigSchema,
   DEFAULT_CONFIG,
@@ -86,12 +87,68 @@ describe('normalizeConfig', () => {
     }).toThrowErrorMatchingSnapshot();
   });
 
-  it('throws error for baseUrl without trailing `/`', () => {
-    expect(() => {
+  it('throws for non-string URLs', () => {
+    expect(() =>
+      normalizeConfig({
+        // @ts-expect-error: test
+        url: 1,
+      }),
+    ).toThrowErrorMatchingInlineSnapshot(`
+      ""url" contains an invalid value
+      "
+    `);
+  });
+
+  it('normalizes various URLs', () => {
+    const consoleMock = jest
+      .spyOn(console, 'warn')
+      .mockImplementation(() => {});
+
+    expect(
+      normalizeConfig({
+        url: 'https://mysite.com/',
+      }).url,
+    ).toBe('https://mysite.com');
+    expect(
+      normalizeConfig({
+        // This shouldn't happen
+        url: 'https://mysite.com/foo/',
+      }).url,
+    ).toBe('https://mysite.com/foo');
+
+    expect(consoleMock.mock.calls[0][0]).toMatchInlineSnapshot(
+      `"[WARNING] Docusaurus config validation warning. Field "url": The url is not supposed to contain a sub-path like '/foo/'. Please use the baseUrl field for sub-paths."`,
+    );
+  });
+
+  it('throws for non-string base URLs', () => {
+    expect(() =>
+      normalizeConfig({
+        // @ts-expect-error: test
+        baseUrl: 1,
+      }),
+    ).toThrowErrorMatchingInlineSnapshot(`
+      ""baseUrl" must be a string
+      "
+    `);
+  });
+
+  it('normalizes various base URLs', () => {
+    expect(
       normalizeConfig({
         baseUrl: 'noSlash',
-      });
-    }).toThrowErrorMatchingSnapshot();
+      }).baseUrl,
+    ).toBe('/noSlash/');
+    expect(
+      normalizeConfig({
+        baseUrl: '/noSlash',
+      }).baseUrl,
+    ).toBe('/noSlash/');
+    expect(
+      normalizeConfig({
+        baseUrl: 'noSlash/foo',
+      }).baseUrl,
+    ).toBe('/noSlash/foo/');
   });
 
   it.each([
@@ -342,7 +399,7 @@ describe('config warnings', () => {
     expect(warning).toBeDefined();
     expect(warning.details).toHaveLength(1);
     expect(warning.details[0]!.message).toMatchInlineSnapshot(
-      `"Docusaurus config validation warning. Field "url": the url is not supposed to contain a sub-path like '/someSubpath', please use the baseUrl field for sub-paths"`,
+      `"Docusaurus config validation warning. Field "url": The url is not supposed to contain a sub-path like '/someSubpath'. Please use the baseUrl field for sub-paths."`,
     );
   });
 });
diff --git a/packages/docusaurus/src/server/configValidation.ts b/packages/docusaurus/src/server/configValidation.ts
@@ -8,6 +8,9 @@
 import {
   DEFAULT_STATIC_DIR_NAME,
   DEFAULT_I18N_DIR_NAME,
+  addLeadingSlash,
+  addTrailingSlash,
+  removeTrailingSlash,
 } from '@docusaurus/utils';
 import {Joi, URISchema, printWarning} from '@docusaurus/utils-validation';
 import type {DocusaurusConfig, I18nConfig} from '@docusaurus/types';
@@ -149,24 +152,23 @@ const I18N_CONFIG_SCHEMA = Joi.object<I18nConfig>({
   .optional()
   .default(DEFAULT_I18N_CONFIG);
 
-const SiteUrlSchema = URISchema.required().custom((value: unknown, helpers) => {
+const SiteUrlSchema = URISchema.required().custom((value: string, helpers) => {
   try {
     const {pathname} = new URL(String(value));
     if (pathname !== '/') {
       helpers.warn('docusaurus.configValidationWarning', {
-        warningMessage: `the url is not supposed to contain a sub-path like '${pathname}', please use the baseUrl field for sub-paths`,
+        warningMessage: `The url is not supposed to contain a sub-path like '${pathname}'. Please use the baseUrl field for sub-paths.`,
       });
     }
   } catch {}
-  return value;
-}, 'siteUrlCustomValidation');
+  return removeTrailingSlash(value);
+});
 
 // TODO move to @docusaurus/utils-validation
 export const ConfigSchema = Joi.object<DocusaurusConfig>({
   baseUrl: Joi.string()
     .required()
-    .regex(/\/$/m)
-    .message('{{#label}} must be a string with a trailing slash.'),
+    .custom((value: string) => addLeadingSlash(addTrailingSlash(value))),
   baseUrlIssueBanner: Joi.boolean().default(DEFAULT_CONFIG.baseUrlIssueBanner),
   favicon: Joi.string().optional(),
   title: Joi.string().required(),