beta.5 docs tags feature - better validation for malformed tags

[swyxio]

🐛 Bug Report

Description

when i upgraded my docs repo temporalio/documentation#619 to beta.5 i got a new error.

Starting the development server...
Can't process doc metadatas for doc at path "/Users/swyx/Temporal/documentation/docs/go/hello-world-tutorial.md" in version "current"
Can't process doc metadatas for doc at path "/Users/swyx/Temporal/documentation/docs/java/hello-world-tutorial.md" in version "current"
Loading of version failed for version "current"
TypeError: frontMatterTags.map is not a function
    at Object.normalizeFrontMatterTags (/Users/swyx/Temporal/documentation/node_modules/@docusaurus/utils/lib/tags.js:37:106)
    at doProcessDocMetadata (/Users/swyx/Temporal/documentation/node_modules/@docusaurus/plugin-content-docs/lib/docs.js:164:23)
    at Object.processDocMetadata (/Users/swyx/Temporal/documentation/node_modules/@docusaurus/plugin-content-docs/lib/docs.js:177:16)
    at processVersionDoc (/Users/swyx/Temporal/documentation/node_modules/@docusaurus/plugin-content-docs/lib/index.js:88:35)
    at Array.map (<anonymous>)
    at loadVersionDocsBase (/Users/swyx/Temporal/documentation/node_modules/@docusaurus/plugin-content-docs/lib/index.js:95:45)
    at async doLoadVersion (/Users/swyx/Temporal/documentation/node_modules/@docusaurus/plugin-content-docs/lib/index.js:102:34)
    at async loadVersion (/Users/swyx/Temporal/documentation/node_modules/@docusaurus/plugin-content-docs/lib/index.js:166:28)
    at async Promise.all (index 0)
    at async Object.loadContent (/Users/swyx/Temporal/documentation/node_modules/@docusaurus/plugin-content-docs/lib/index.js:174:33)
    at async /Users/swyx/Temporal/documentation/node_modules/@docusaurus/core/lib/server/plugins/index.js:58:46
    at async Promise.all (index 0)
    at async Object.loadPlugins (/Users/swyx/Temporal/documentation/node_modules/@docusaurus/core/lib/server/plugins/index.js:57:27)
    at async Object.load (/Users/swyx/Temporal/documentation/node_modules/@docusaurus/core/lib/server/index.js:186:82)
    at async start (/Users/swyx/Temporal/documentation/node_modules/@docusaurus/core/lib/commands/start.js:44:19)
error Command failed with exit code 1.

this is because the new docs tag feature (#3646) expects this format:

---
tags:
  - foo
  - bar
  - baz
---

and doesnt gracefully handle:

---
tags: foo, bar, baz
---

or:

---
tags: foo
---

we were already using tags in our own way so our errors arent important, but i do think the comma separated tags experience is important that helpful/good errors are important.

Have you read the Contributing Guidelines on issues?

yep

Steps to reproduce

temporalio/documentation#619

Expected behavior

clean upgrade process, elegant handling of tags

[swyxio]

question: is this behavior you want or are you fine with a very strict format of docs tags?
recommendation: at LEAST handle error better here, OR handle comma separated tags

[Josh-Cena]

For some reason this shows up in my inbox...

As I mentioned in #5433 (comment), the Markdown front matter conforms to YAML syntax. Per YAML spec there are two ways to define an array:

---
# Block style
- Item 1
- Item 2
- Item 3
---
# Inline style
[Item 1, Item 2, Item 3]
---

So comma-separated list without the square brackets isn't even valid YAML. It's not something Docusaurus can gracefully handle without significantly tweaking the YAML parser or we taking over the parsing job ourselves (which is probably not worth the effort).

Maybe in the end we should document the latter use case explicitly and assume less knowledge of Markdown front matter syntax from the user? I thought using the inline style elsewhere would be enough to inform readers of this possibility

I noticed that foo: item1, item2 and foo: item1 would both become a property foo with a string value. This should be handled by Joi, so the real issue is they forgot to add validation for tags here:

docusaurus/packages/docusaurus-plugin-content-docs/src/docFrontMatter.ts

Lines 19 to 33 in fd3c0c6

	const DocFrontMatterSchema = Joi.object<DocFrontMatter>({
	id: Joi.string(),
	title: Joi.string().allow(''), // see https://github.com/facebook/docusaurus/issues/4591#issuecomment-822372398
	hide_title: Joi.boolean(),
	hide_table_of_contents: Joi.boolean(),
	keywords: Joi.array().items(Joi.string().required()),
	image: URISchema,
	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(),
	pagination_label: Joi.string(),
	custom_edit_url: URISchema.allow('', null),
	parse_number_prefixes: Joi.boolean(),
	}).unknown();

[swyxio]

hmm i see and hear you. yes lets do the validation. I'm not for extending YAML, i'm just in favor of nice errors so that when people make common mistakes we help them get back on track.

[slorber]

hmm i see and hear you. yes lets do the validation. I'm not for extending YAML, i'm just in favor of nice errors so that when people make common mistakes we help them get back on track.

We should definitively add nice validation messages. That's why I added a strict fail-fast validation step but Joi is a bit painful to use and default messages are not always nice, so I always prefer to add tests about validation messages.

I also plan to add a method like createFrontMatter or processFrontMatter that gives the opportunity to add/modify/normalize frontmatters before the validation step, so eventually you could transform ["tag1, tag2"] to ["tag1", ”tag2"] yourself if that make sense for your use-case and legacy docs