fix: navbar item validation done correctly (#5202)

* Initial work

Signed-off-by: Josh-Cena <sidachen2003@gmail.com>

* Fix

Signed-off-by: Josh-Cena <sidachen2003@gmail.com>

* Fix again

Signed-off-by: Josh-Cena <sidachen2003@gmail.com>

* More fix (Joi is so hard!)

Signed-off-by: Josh-Cena <sidachen2003@gmail.com>

* This should pass now

Signed-off-by: Josh-Cena <sidachen2003@gmail.com>

* Such pain

Signed-off-by: Josh-Cena <sidachen2003@gmail.com>

* Minor tweaks

Signed-off-by: Josh-Cena <sidachen2003@gmail.com>

* More test cases

Signed-off-by: Josh-Cena <sidachen2003@gmail.com>

* Minor tweaks

Signed-off-by: Josh-Cena <sidachen2003@gmail.com>

* Errr... this should be better

Signed-off-by: Josh-Cena <sidachen2003@gmail.com>

* Redo isOfType

Signed-off-by: Josh-Cena <sidachen2003@gmail.com>

* Make things more concise

Signed-off-by: Josh-Cena <sidachen2003@gmail.com>

* Remove TODO

Signed-off-by: Josh-Cena <sidachen2003@gmail.com>

* Rename isOfType

Signed-off-by: Josh-Cena <sidachen2003@gmail.com>

* Slight refactor

Signed-off-by: Josh-Cena <sidachen2003@gmail.com>

* More error messages

Signed-off-by: Josh-Cena <sidachen2003@gmail.com>

* More test cases

Signed-off-by: Josh-Cena <sidachen2003@gmail.com>

Co-authored-by: slorber <lorber.sebastien@gmail.com>

packages/docusaurus-theme-classic/src/__tests__/validateThemeConfig.test.js

@@ -134,6 +134,24 @@ describe('themeConfig', () => {
               },
             ],
           },
+          // Dropdown with name
+          {
+            type: 'dropdown',
+            label: 'Tools',
+            position: 'left',
+            items: [
+              {
+                type: 'doc',
+                activeSidebarClassName: 'custom-class',
+                docId: 'npm',
+                label: 'NPM',
+              },
+              {
+                to: '/yarn',
+                label: 'Yarn',
+              },
+            ],
+          },
           // Doc version dropdown
           {
             type: 'docsVersionDropdown',
@@ -181,6 +199,112 @@ describe('themeConfig', () => {
     });
   });
 
+  test('should reject unknown navbar item type', () => {
+    const config = {
+      navbar: {
+        items: [
+          {
+            type: 'joke',
+            position: 'left',
+            label: 'haha',
+          },
+        ],
+      },
+    };
+    expect(() =>
+      testValidateThemeConfig(config),
+    ).toThrowErrorMatchingInlineSnapshot(`"Bad navbar item type joke"`);
+  });
+
+  test('should reject nested dropdowns', () => {
+    const config = {
+      navbar: {
+        items: [
+          {
+            position: 'left',
+            label: 'Nested',
+            items: [
+              {
+                label: 'Still a dropdown',
+                items: [
+                  {
+                    label: 'Should reject this',
+                    to: '/rejected',
+                  },
+                ],
+              },
+            ],
+          },
+        ],
+      },
+    };
+    expect(() =>
+      testValidateThemeConfig(config),
+    ).toThrowErrorMatchingInlineSnapshot(`"Nested dropdowns are not allowed"`);
+  });
+
+  test('should reject nested dropdowns', () => {
+    const config = {
+      navbar: {
+        items: [
+          {
+            position: 'left',
+            label: 'Nested',
+            items: [{type: 'docsVersionDropdown'}],
+          },
+        ],
+      },
+    };
+    expect(() =>
+      testValidateThemeConfig(config),
+    ).toThrowErrorMatchingInlineSnapshot(`"Nested dropdowns are not allowed"`);
+  });
+
+  test('should reject position attribute within dropdown', () => {
+    const config = {
+      navbar: {
+        items: [
+          {
+            position: 'left',
+            label: 'Dropdown',
+            items: [
+              {
+                label: 'Hi',
+                position: 'left',
+                to: '/link',
+              },
+            ],
+          },
+        ],
+      },
+    };
+    expect(() =>
+      testValidateThemeConfig(config),
+    ).toThrowErrorMatchingInlineSnapshot(
+      `"\\"navbar.items[0].items[0].position\\" is not allowed"`,
+    );
+  });
+
+  test('should give friendly error when href and to coexist', () => {
+    const config = {
+      navbar: {
+        items: [
+          {
+            position: 'left',
+            label: 'Nested',
+            to: '/link',
+            href: 'http://example.com/link',
+          },
+        ],
+      },
+    };
+    expect(() =>
+      testValidateThemeConfig(config),
+    ).toThrowErrorMatchingInlineSnapshot(
+      `"One and only one between \\"to\\" and \\"href\\" should be provided"`,
+    );
+  });
+
   test('should allow empty alt tags for the logo image in the header', () => {
     const altTagConfig = {
       navbar: {

packages/docusaurus-theme-classic/src/validateThemeConfig.js

@@ -45,79 +45,48 @@ exports.DEFAULT_CONFIG = DEFAULT_CONFIG;
 
 const NavbarItemPosition = Joi.string().equal('left', 'right').default('left');
 
-const BaseNavbarItemSchema = Joi.object({
+const NavbarItemBaseSchema = Joi.object({
-  to: Joi.string(),
-  href: URISchema,
   label: Joi.string(),
   className: Joi.string(),
-  prependBaseUrlToHref: Joi.bool(),
 })
   // We allow any unknown attributes on the links
   // (users may need additional attributes like target, aria-role, data-customAttribute...)
   .unknown();
 
-// TODO we should probably create a custom navbar item type "dropdown"
+const DefaultNavbarItemSchema = NavbarItemBaseSchema.append({
-// having this recursive structure is bad because we only support 2 levels
+  to: Joi.string(),
-// + parent/child don't have exactly the same props
+  href: URISchema,
-const DefaultNavbarItemSchema = BaseNavbarItemSchema.append({
-  items: Joi.array().optional().items(BaseNavbarItemSchema),
-  position: NavbarItemPosition,
   activeBasePath: Joi.string(),
   activeBaseRegex: Joi.string(),
+  prependBaseUrlToHref: Joi.bool(),
+  // This is only triggered in case of a nested dropdown
+  items: Joi.forbidden().messages({
+    'any.unknown': 'Nested dropdowns are not allowed',
+  }),
+})
+  .xor('href', 'to')
+  .messages({
+    'object.xor': 'One and only one between "to" and "href" should be provided',
   });
-// TODO the dropdown parent item can have no href/to
-// should check should not apply to dropdown parent item
-// .xor('href', 'to');
 
-const DocsVersionNavbarItemSchema = Joi.object({
+const DocsVersionNavbarItemSchema = NavbarItemBaseSchema.append({
   type: Joi.string().equal('docsVersion').required(),
-  position: NavbarItemPosition,
-  label: Joi.string(),
   to: Joi.string(),
   docsPluginId: Joi.string(),
-  className: Joi.string(),
 });
 
-const DocsVersionDropdownNavbarItemSchema = Joi.object({
+const DocItemSchema = NavbarItemBaseSchema.append({
-  type: Joi.string().equal('docsVersionDropdown').required(),
-  position: NavbarItemPosition,
-  docsPluginId: Joi.string(),
-  dropdownActiveClassDisabled: Joi.boolean(),
-  dropdownItemsBefore: Joi.array().items(BaseNavbarItemSchema).default([]),
-  dropdownItemsAfter: Joi.array().items(BaseNavbarItemSchema).default([]),
-  className: Joi.string(),
-});
-
-const DocItemSchema = Joi.object({
   type: Joi.string().equal('doc').required(),
-  position: NavbarItemPosition,
   docId: Joi.string().required(),
-  label: Joi.string(),
   docsPluginId: Joi.string(),
   activeSidebarClassName: Joi.string().default('navbar__link--active'),
-  className: Joi.string(),
 });
 
-const LocaleDropdownNavbarItemSchema = Joi.object({
+const itemWithType = (type) => {
-  type: Joi.string().equal('localeDropdown').required(),
-  position: NavbarItemPosition,
-  dropdownItemsBefore: Joi.array().items(BaseNavbarItemSchema).default([]),
-  dropdownItemsAfter: Joi.array().items(BaseNavbarItemSchema).default([]),
-  className: Joi.string(),
-});
-
-const SearchItemSchema = Joi.object({
-  type: Joi.string().equal('search').required(),
-  position: NavbarItemPosition,
-});
-
-// Can this be made easier? :/
-const isOfType = (type) => {
-  let typeSchema = Joi.string().required();
   // because equal(undefined) is not supported :/
-  if (type) {
+  const typeSchema = type
-    typeSchema = typeSchema.equal(type);
+    ? Joi.string().required().equal(type)
-  }
+    : Joi.string().forbidden();
   return Joi.object({
     type: typeSchema,
   })
@@ -125,58 +94,105 @@ const isOfType = (type) => {
     .required();
 };
 
-const NavbarItemSchema = Joi.object().when({
+const DropdownSubitemSchema = Joi.object({
+  position: Joi.forbidden(),
+}).when({
   switch: [
     {
-      is: isOfType('docsVersion'),
+      is: itemWithType('docsVersion'),
       then: DocsVersionNavbarItemSchema,
     },
     {
-      is: isOfType('docsVersionDropdown'),
+      is: itemWithType('doc'),
-      then: DocsVersionDropdownNavbarItemSchema,
-    },
-    {
-      is: isOfType('doc'),
       then: DocItemSchema,
     },
     {
-      is: isOfType('localeDropdown'),
+      is: itemWithType(undefined),
-      then: LocaleDropdownNavbarItemSchema,
+      then: DefaultNavbarItemSchema,
     },
     {
-      is: isOfType('search'),
+      is: Joi.alternatives().try(
-      then: SearchItemSchema,
+        itemWithType('dropdown'),
-    },
+        itemWithType('docsVersionDropdown'),
-    {
+        itemWithType('localeDropdown'),
-      is: isOfType(undefined),
+        itemWithType('search'),
+      ),
       then: Joi.forbidden().messages({
-        'any.unknown': 'Bad navbar item type {.type}',
+        'any.unknown': 'Nested dropdowns are not allowed',
       }),
     },
   ],
-  otherwise: DefaultNavbarItemSchema,
+  otherwise: Joi.forbidden().messages({
+    'any.unknown': 'Bad navbar item type {.type}',
+  }),
+});
+
+const DropdownNavbarItemSchema = NavbarItemBaseSchema.append({
+  items: Joi.array().items(DropdownSubitemSchema).required(),
+});
+
+const DocsVersionDropdownNavbarItemSchema = NavbarItemBaseSchema.append({
+  type: Joi.string().equal('docsVersionDropdown').required(),
+  docsPluginId: Joi.string(),
+  dropdownActiveClassDisabled: Joi.boolean(),
+  dropdownItemsBefore: Joi.array().items(DropdownSubitemSchema).default([]),
+  dropdownItemsAfter: Joi.array().items(DropdownSubitemSchema).default([]),
+});
+
+const LocaleDropdownNavbarItemSchema = NavbarItemBaseSchema.append({
+  type: Joi.string().equal('localeDropdown').required(),
+  dropdownItemsBefore: Joi.array().items(DropdownSubitemSchema).default([]),
+  dropdownItemsAfter: Joi.array().items(DropdownSubitemSchema).default([]),
+});
+
+const SearchItemSchema = Joi.object({
+  type: Joi.string().equal('search').required(),
 });
 
-/*
 const NavbarItemSchema = Joi.object({
-  type: Joi.string().only(['docsVersion'])
+  position: NavbarItemPosition,
-})
+}).when({
-  .when(Joi.object({ type: 'docsVersion' }).unknown(), {
+  switch: [
-    then: Joi.object({ pepperoni: Joi.boolean() })
+    {
-  })
+      is: itemWithType('docsVersion'),
-  .when(Joi.object().unknown(), {
-    then: Joi.object({ croutons: Joi.boolean() })
-  })
-
- */
-
-/*
-const NavbarItemSchema = Joi.object().when('type', {
-  is: Joi.valid('docsVersion'),
       then: DocsVersionNavbarItemSchema,
+    },
+    {
+      is: itemWithType('dropdown'),
+      then: DropdownNavbarItemSchema,
+    },
+    {
+      is: itemWithType('docsVersionDropdown'),
+      then: DocsVersionDropdownNavbarItemSchema,
+    },
+    {
+      is: itemWithType('doc'),
+      then: DocItemSchema,
+    },
+    {
+      is: itemWithType('localeDropdown'),
+      then: LocaleDropdownNavbarItemSchema,
+    },
+    {
+      is: itemWithType('search'),
+      then: SearchItemSchema,
+    },
+    {
+      is: itemWithType(undefined),
+      then: Joi.object().when({
+        // Dropdown item can be specified without type field
+        is: Joi.object({
+          items: Joi.array().required(),
+        }).unknown(),
+        then: DropdownNavbarItemSchema,
         otherwise: DefaultNavbarItemSchema,
+      }),
+    },
+  ],
+  otherwise: Joi.forbidden().messages({
+    'any.unknown': 'Bad navbar item type {.type}',
+  }),
 });
- */
 
 const ColorModeSchema = Joi.object({
   defaultMode: Joi.string()