Skip to content
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.

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

2.0.0-alpha.73 can't handle tags which could be parsed as numbers #4642

Closed
johnnyreilly opened this issue Apr 19, 2021 · 5 comments · Fixed by #4654
Closed

2.0.0-alpha.73 can't handle tags which could be parsed as numbers #4642

johnnyreilly opened this issue Apr 19, 2021 · 5 comments · Fixed by #4654
Labels
bug An error in the Docusaurus core causing instability or issues with its execution

Comments

@johnnyreilly
Copy link
Contributor

johnnyreilly commented Apr 19, 2021

🐛 Bug Report

As of 2.0.0-alpha.73, docusaurus will choke on tags that can be parsed as numbers.

Consider the following:

---
title: "Azure DevOps Node API: The missing episodes"
author: John Reilly
author_url: https://github.com/johnnyreilly
author_image_url: https://blog.johnnyreilly.com/img/profile.jpg
tags: [azure devops api, 203, node.js]
hide_table_of_contents: false
---

The 203 tag above is bad news and will lead to an error like this:

[en] Creating an optimized production build...
error building locale=en
ValidationError: "tags[1]" must be one of [string, object]

See: https://github.com/johnnyreilly/blog.johnnyreilly.com/pull/35/checks?check_run_id=2377642622

You can workaround it by changing 203 to be '203', but ideally this wouldn't be necessary.

Have you read the [Contributing Guidelines on issues]

Yes

To Reproduce

Follow the example above and then yarn build

Expected behavior

Compilation works.

Actual Behavior

An error of the nature:

[en] Creating an optimized production build...
error building locale=en
ValidationError: "tags[1]" must be one of [string, object]
    at Object.exports.process (/home/runner/work/blog.johnnyreilly.com/blog.johnnyreilly.com/blog-website/node_modules/joi/lib/errors.js:184:16)
    at Object.internals.entry (/home/runner/work/blog.johnnyreilly.com/blog.johnnyreilly.com/blog-website/node_modules/joi/lib/validator.js:150:26)
    at Object.exports.entry (/home/runner/work/blog.johnnyreilly.com/blog.johnnyreilly.com/blog-website/node_modules/joi/lib/validator.js:27:30)
    at internals.Base.validate (/home/runner/work/blog.johnnyreilly.com/blog.johnnyreilly.com/blog-website/node_modules/joi/lib/base.js:548:26)
    at Object.internals.assert (/home/runner/work/blog.johnnyreilly.com/blog.johnnyreilly.com/blog-website/node_modules/joi/lib/index.js:225:27)
    at Object.attempt (/home/runner/work/blog.johnnyreilly.com/blog.johnnyreilly.com/blog-website/node_modules/joi/lib/index.js:107:26)
    at Object.assertBlogPostFrontMatter (/home/runner/work/blog.johnnyreilly.com/blog.johnnyreilly.com/blog-website/node_modules/@docusaurus/plugin-content-blog/lib/blogFrontMatter.js:24:28)
    at /home/runner/work/blog.johnnyreilly.com/blog.johnnyreilly.com/blog-website/node_modules/@docusaurus/plugin-content-blog/lib/blogUtils.js:90:27
    at async Promise.all (index 194)
    at async Object.generateBlogPosts (/home/runner/work/blog.johnnyreilly.com/blog.johnnyreilly.com/blog-website/node_modules/@docusaurus/plugin-content-blog/lib/blogUtils.js:84:5)
    at async Object.loadContent (/home/runner/work/blog.johnnyreilly.com/blog.johnnyreilly.com/blog-website/node_modules/@docusaurus/plugin-content-blog/lib/index.js:59:25)
    at async /home/runner/work/blog.johnnyreilly.com/blog.johnnyreilly.com/blog-website/node_modules/@docusaurus/core/lib/server/plugins/index.js:53:46
    at async Promise.all (index 0)
    at async Object.loadPlugins (/home/runner/work/blog.johnnyreilly.com/blog.johnnyreilly.com/blog-website/node_modules/@docusaurus/core/lib/server/plugins/index.js:52:34)
    at async Object.load (/home/runner/work/blog.johnnyreilly.com/blog.johnnyreilly.com/blog-website/node_modules/@docusaurus/core/lib/server/index.js:94:82)

Your Environment

Locally in Linux and in GitHub Actions

Reproducible Demo

See linked PR: johnnyreilly/blog.johnnyreilly.com#35

I've opted to go with the workaround for now; but I thought I should share the experience. Certainly the error message isn't the most helpful in the world.

@johnnyreilly johnnyreilly added bug An error in the Docusaurus core causing instability or issues with its execution status: needs triage This issue has not been triaged by maintainers labels Apr 19, 2021
@slorber
Copy link
Collaborator

slorber commented Apr 19, 2021

Make sense, I'll add the Joi {convert: true} option, that should fix it.

Also worth logging the doc frontmatter in the error message 😅 was it easy to find the problematic blog post?

@johnnyreilly
Copy link
Contributor Author

johnnyreilly commented Apr 19, 2021

So I saw the failure when the GitHub Action ran - I then ran yarn build locally and looked at the stack trace.

This told me where the error was thrown, so I added a bunch of console.log statements and ran yarn build again, and that allowed me to identify the problematic post. So not super hard, but required hacking away in node_modules

If you point me in the direction of the relevant bit of code I'd be happy to submit a fix PR myself if that helps?

@johnnyreilly
Copy link
Contributor Author

johnnyreilly commented Apr 19, 2021

Is it here?

Joi.attempt(frontMatter, BlogFrontMatterSchema);

So it could be:

Joi.attempt(frontMatter, BlogFrontMatterSchema, undefined, { convert: true }); 

https://joi.dev/api/?v=17.4.0#attemptvalue-schema-message-options

Or alternatively,

const BlogTagSchema = Joi.alternatives().try(
  Joi.string().required(),
  Joi.object<Tag>({
    label: Joi.string().required(),
    permalink: Joi.string().required(),
  }),
);

could become

const BlogTagSchema = Joi.alternatives().try(
  Joi.string().required(),
  Joi.number().required(),
  Joi.object<Tag>({
    label: Joi.string().required(),
    permalink: Joi.string().required(),
  }),
);

@slorber
Copy link
Collaborator

slorber commented Apr 19, 2021

Yes that's here, I'd prefer to use "convert" as in the end we want the tag to be a string. And we should also convert tag labels from numbers to string when they are using the object version.

Can you also add a try/catch around the attempt to log the frontmatter in case of error? That would be helpful for users to debug. Something like:

try {
  return attempt(frontMatter);
} catch (e) {
  console.error(chalk.red("bad frontmatter: " + JSON.stringify(frontmatter)));
  throw e;
}

This way users will be able to identify the problematic md file more easily

Also can you add a test for this specific usecase of title (ensuring we don't remove the convert: true option)

Also related to #4591

@johnnyreilly
Copy link
Contributor Author

See questions on the related WIP PR.

@slorber slorber closed this as completed Apr 21, 2021
@slorber slorber linked a pull request Apr 21, 2021 that will close this issue
@Josh-Cena Josh-Cena removed the status: needs triage This issue has not been triaged by maintainers label Feb 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An error in the Docusaurus core causing instability or issues with its execution
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants