fix(v2): fix too strict markdown frontmatter validation (#4654)

* start work * use orta.vscode-jest * node 14 * add some better infra to validate markdown frontmatter * better docs frontmatter validation * fix Yaml / Joi validation issues * fix Yaml / Joi validation issues Co-authored-by: slorber <lorber.sebastien@gmail.com>
facebook · Apr 21, 2021 · e11597a · e11597a
1 parent c04e613
commit e11597a
Show file tree

Hide file tree

Showing 8 changed files with 238 additions and 21 deletions.
diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json
@@ -1,10 +1,10 @@
 {
   "name": "Docusaurus Dev Container",
-  "image": "mcr.microsoft.com/vscode/devcontainers/typescript-node:0-10-buster",
+  "image": "mcr.microsoft.com/vscode/devcontainers/typescript-node:14-buster",
   "settings": {
     "terminal.integrated.shell.linux": "/bin/bash"
   },
-  "extensions": ["dbaeumer.vscode-eslint"],
+  "extensions": ["dbaeumer.vscode-eslint", "orta.vscode-jest"],
   "forwardPorts": [3000],
   "postCreateCommand": "yarn install"
 }
diff --git a/packages/docusaurus-plugin-content-blog/src/__tests__/blogFrontMatter.test.ts b/packages/docusaurus-plugin-content-blog/src/__tests__/blogFrontMatter.test.ts
@@ -0,0 +1,67 @@
+/**
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+import {
+  BlogPostFrontMatter,
+  validateBlogPostFrontMatter,
+} from '../blogFrontMatter';
+
+describe('validateBlogPostFrontMatter', () => {
+  test('accept empty object', () => {
+    const frontMatter = {};
+    expect(validateBlogPostFrontMatter(frontMatter)).toEqual(frontMatter);
+  });
+
+  test('accept valid values', () => {
+    const frontMatter: BlogPostFrontMatter = {
+      id: 'blog',
+      title: 'title',
+      description: 'description',
+      date: 'date',
+      slug: 'slug',
+      draft: true,
+      tags: ['hello', {label: 'tagLabel', permalink: '/tagPermalink'}],
+    };
+    expect(validateBlogPostFrontMatter(frontMatter)).toEqual(frontMatter);
+  });
+
+  // See https://github.com/facebook/docusaurus/issues/4591#issuecomment-822372398
+  test('accept empty title', () => {
+    const frontMatter: BlogPostFrontMatter = {title: ''};
+    expect(validateBlogPostFrontMatter(frontMatter)).toEqual(frontMatter);
+  });
+
+  // See https://github.com/facebook/docusaurus/issues/4591#issuecomment-822372398
+  test('accept empty description', () => {
+    const frontMatter: BlogPostFrontMatter = {description: ''};
+    expect(validateBlogPostFrontMatter(frontMatter)).toEqual(frontMatter);
+  });
+
+  // See https://github.com/facebook/docusaurus/issues/4642
+  test('convert tags as numbers', () => {
+    const frontMatter: BlogPostFrontMatter = {
+      tags: [
+        // @ts-expect-error: number for test
+        42,
+        {
+          // @ts-expect-error: number for test
+          label: 84,
+          permalink: '/tagPermalink',
+        },
+      ],
+    };
+    expect(validateBlogPostFrontMatter(frontMatter)).toEqual({
+      tags: [
+        '42',
+        {
+          label: '84',
+          permalink: '/tagPermalink',
+        },
+      ],
+    });
+  });
+});
diff --git a/packages/docusaurus-plugin-content-blog/src/blogFrontMatter.ts b/packages/docusaurus-plugin-content-blog/src/blogFrontMatter.ts
@@ -5,11 +5,14 @@
  * LICENSE file in the root directory of this source tree.
  */
 
-import {Joi} from '@docusaurus/utils-validation';
+import {
+  JoiFrontMatter as Joi, // Custom instance for frontmatter
+  validateFrontMatter,
+} from '@docusaurus/utils-validation';
 import {Tag} from './types';
 
 // TODO complete this frontmatter + add unit tests
-type BlogPostFrontMatter = {
+export type BlogPostFrontMatter = {
   id?: string;
   title?: string;
   description?: string;
@@ -19,6 +22,10 @@ type BlogPostFrontMatter = {
   date?: string;
 };
 
+// NOTE: we don't add any default value on purpose here
+// We don't want default values to magically appear in doc metadatas and props
+// While the user did not provide those values explicitly
+// We use default values in code instead
 const BlogTagSchema = Joi.alternatives().try(
   Joi.string().required(),
   Joi.object<Tag>({
@@ -29,15 +36,16 @@ const BlogTagSchema = Joi.alternatives().try(
 
 const BlogFrontMatterSchema = Joi.object<BlogPostFrontMatter>({
   id: Joi.string(),
-  title: Joi.string(),
-  description: Joi.string(),
+  title: Joi.string().allow(''),
+  description: Joi.string().allow(''),
   tags: Joi.array().items(BlogTagSchema),
   slug: Joi.string(),
   draft: Joi.boolean(),
+  date: Joi.string().allow(''), // TODO validate the date better!
 }).unknown();
 
-export function assertBlogPostFrontMatter(
+export function validateBlogPostFrontMatter(
   frontMatter: Record<string, unknown>,
-): asserts frontMatter is BlogPostFrontMatter {
-  Joi.attempt(frontMatter, BlogFrontMatterSchema);
+): BlogPostFrontMatter {
+  return validateFrontMatter(frontMatter, BlogFrontMatterSchema);
 }
diff --git a/packages/docusaurus-plugin-content-blog/src/blogUtils.ts b/packages/docusaurus-plugin-content-blog/src/blogUtils.ts
@@ -29,7 +29,7 @@ import {
   replaceMarkdownLinks,
 } from '@docusaurus/utils';
 import {LoadContext} from '@docusaurus/types';
-import {assertBlogPostFrontMatter} from './blogFrontMatter';
+import {validateBlogPostFrontMatter} from './blogFrontMatter';
 
 export function truncate(fileString: string, truncateMarker: RegExp): string {
   return fileString.split(truncateMarker, 1).shift()!;
@@ -142,12 +142,12 @@ export async function generateBlogPosts(
       const source = path.join(blogDirPath, blogSourceFile);
 
       const {
-        frontMatter,
+        frontMatter: unsafeFrontMatter,
         content,
         contentTitle,
         excerpt,
       } = await parseMarkdownFile(source);
-      assertBlogPostFrontMatter(frontMatter);
+      const frontMatter = validateBlogPostFrontMatter(unsafeFrontMatter);
 
       const aliasedSource = aliasedSitePath(source, siteDir);
 

diff --git a/packages/docusaurus-plugin-content-docs/src/__tests__/docFrontMatter.test.ts b/packages/docusaurus-plugin-content-docs/src/__tests__/docFrontMatter.test.ts
@@ -0,0 +1,37 @@
+/**
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+import {DocFrontMatter, validateDocFrontMatter} from '../docFrontMatter';
+
+describe('validateDocFrontMatter', () => {
+  test('accept empty object', () => {
+    const frontMatter: DocFrontMatter = {};
+    expect(validateDocFrontMatter(frontMatter)).toEqual(frontMatter);
+  });
+
+  test('accept valid values', () => {
+    const frontMatter: DocFrontMatter = {
+      id: 'blog',
+      title: 'title',
+      description: 'description',
+      slug: 'slug',
+    };
+    expect(validateDocFrontMatter(frontMatter)).toEqual(frontMatter);
+  });
+
+  // See https://github.com/facebook/docusaurus/issues/4591#issuecomment-822372398
+  test('accept empty title', () => {
+    const frontMatter: DocFrontMatter = {title: ''};
+    expect(validateDocFrontMatter(frontMatter)).toEqual(frontMatter);
+  });
+
+  // See https://github.com/facebook/docusaurus/issues/4591#issuecomment-822372398
+  test('accept empty description', () => {
+    const frontMatter: DocFrontMatter = {description: ''};
+    expect(validateDocFrontMatter(frontMatter)).toEqual(frontMatter);
+  });
+});
diff --git a/packages/docusaurus-plugin-content-docs/src/docFrontMatter.ts b/packages/docusaurus-plugin-content-docs/src/docFrontMatter.ts
@@ -5,17 +5,20 @@
  * LICENSE file in the root directory of this source tree.
  */
 
-import {Joi} from '@docusaurus/utils-validation';
+import {
+  JoiFrontMatter as Joi, // Custom instance for frontmatter
+  validateFrontMatter,
+} from '@docusaurus/utils-validation';
 
 // TODO complete this frontmatter + add unit tests
-type DocFrontMatter = {
+export type DocFrontMatter = {
   id?: string;
   title?: string;
   description?: string;
   slug?: string;
   sidebar_label?: string;
   sidebar_position?: number;
-  custom_edit_url?: string;
+  custom_edit_url?: string | null;
   parse_number_prefixes?: boolean;
 };
 
@@ -25,8 +28,8 @@ type DocFrontMatter = {
 // We use default values in code instead
 const DocFrontMatterSchema = Joi.object<DocFrontMatter>({
   id: Joi.string(),
-  title: Joi.string(),
-  description: Joi.string(),
+  title: Joi.string().allow(''), // see https://github.com/facebook/docusaurus/issues/4591#issuecomment-822372398
+  description: Joi.string().allow(''), // see  https://github.com/facebook/docusaurus/issues/4591#issuecomment-822372398
   slug: Joi.string(),
   sidebar_label: Joi.string(),
   sidebar_position: Joi.number(),
@@ -37,8 +40,5 @@ const DocFrontMatterSchema = Joi.object<DocFrontMatter>({
 export function validateDocFrontMatter(
   frontMatter: Record<string, unknown>,
 ): DocFrontMatter {
-  return Joi.attempt(frontMatter, DocFrontMatterSchema, {
-    convert: true,
-    allowUnknown: true,
-  });
+  return validateFrontMatter(frontMatter, DocFrontMatterSchema);
 }
diff --git a/packages/docusaurus-utils-validation/src/__tests__/validationUtils.test.ts b/packages/docusaurus-utils-validation/src/__tests__/validationUtils.test.ts
@@ -0,0 +1,64 @@
+/**
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+import Joi from '../Joi';
+import {JoiFrontMatter, validateFrontMatter} from '../validationUtils';
+
+describe('validateFrontMatter', () => {
+  test('should accept good values', () => {
+    const schema = Joi.object<{test: string}>({
+      test: Joi.string(),
+    });
+    const frontMatter = {
+      test: 'hello',
+    };
+    expect(validateFrontMatter(frontMatter, schema)).toEqual(frontMatter);
+  });
+
+  test('should reject bad values', () => {
+    const consoleError = jest.spyOn(console, 'error').mockImplementation();
+    const schema = Joi.object<{test: string}>({
+      test: Joi.string(),
+    });
+    const frontMatter = {
+      test: true,
+    };
+    expect(() =>
+      validateFrontMatter(frontMatter, schema),
+    ).toThrowErrorMatchingInlineSnapshot(`"\\"test\\" must be a string"`);
+    expect(consoleError).toHaveBeenCalledWith(
+      expect.stringContaining('FrontMatter contains invalid values: '),
+    );
+  });
+
+  // Fix Yaml trying to convert strings to numbers automatically
+  // We only want to deal with a single type in the final frontmatter (not string | number)
+  test('should convert number values to string when string schema', () => {
+    const schema = Joi.object<{test: string}>({
+      test: JoiFrontMatter.string(),
+    });
+    const frontMatter = {
+      test: 42,
+    };
+    expect(validateFrontMatter(frontMatter, schema)).toEqual({test: '42'});
+  });
+
+  // Helps to fix Yaml trying to convert strings to dates automatically
+  // We only want to deal with a single type in the final frontmatter (not string | Date)
+  test('should convert date values when string schema', () => {
+    const schema = Joi.object<{test: string}>({
+      test: JoiFrontMatter.string(),
+    });
+    const date = new Date();
+    const frontMatter = {
+      test: date,
+    };
+    expect(validateFrontMatter(frontMatter, schema)).toEqual({
+      test: date.toString(),
+    });
+  });
+});
diff --git a/packages/docusaurus-utils-validation/src/validationUtils.ts b/packages/docusaurus-utils-validation/src/validationUtils.ts
@@ -83,3 +83,44 @@ export function normalizeThemeConfig<T>(
   }
   return value;
 }
+
+// Enhance the default Joi.string() type so that it can convert number to strings
+// If user use frontmatter "tag: 2021", we shouldn't need to ask the user to write "tag: '2021'"
+// Also yaml tries to convert patterns like "2019-01-01" to dates automatically
+// see https://github.com/facebook/docusaurus/issues/4642
+// see https://github.com/sideway/joi/issues/1442#issuecomment-823997884
+const JoiFrontMatterString: Joi.Extension = {
+  type: 'string',
+  base: Joi.string(),
+  // Fix Yaml that tries to auto-convert many things to string out of the box
+  prepare: (value) => {
+    if (typeof value === 'number' || value instanceof Date) {
+      return {value: value.toString()};
+    }
+    return {value};
+  },
+};
+export const JoiFrontMatter: typeof Joi = Joi.extend(JoiFrontMatterString);
+
+export function validateFrontMatter<T>(
+  frontMatter: Record<string, unknown>,
+  schema: Joi.ObjectSchema<T>,
+): T {
+  try {
+    return JoiFrontMatter.attempt(frontMatter, schema, {
+      convert: true,
+      allowUnknown: true,
+    });
+  } catch (e) {
+    console.error(
+      chalk.red(
+        `FrontMatter contains invalid values: ${JSON.stringify(
+          frontMatter,
+          null,
+          2,
+        )}`,
+      ),
+    );
+    throw e;
+  }
+}