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

fix(v2): modify validation schema and tests for rehype/remark + remove duplicate dependency #3247

Merged
merged 3 commits into from
Aug 11, 2020

Conversation

teikjun
Copy link
Contributor

@teikjun teikjun commented Aug 9, 2020

Motivation

This PR makes some changes related to validation:

  • modify validation schema of rehype/remark fields to accept object type. This fixes Error when including rehype plugins #3125. Previously, I assumed that remark/rehype plugins could only be function type (AKA attachers in unified's lingo), but this is not exhaustive. It is also possible that rehype/remark plugins are object type. Based on unified's documentation and typescript types, I believe that we have included all the possible types for rehype/remark plugins after adding object type.
  • validate babel style user input
  • add tests for the above
  • remove a duplicate dependency for utils-validation

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

Automated tests

@teikjun teikjun requested a review from yangshun as a code owner August 9, 2020 08:16
@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Aug 9, 2020
@docusaurus-bot
Copy link
Contributor

docusaurus-bot commented Aug 9, 2020

Deploy preview for docusaurus-2 ready!

Built with commit c664d34

https://deploy-preview-3247--docusaurus-2.netlify.app

Comment on lines 14 to 21
const MarkdownPluginsSchema = Joi.array()
.items(
Joi.array()
// TODO, this allows [config,fn] too?
.items(Joi.function().required(), Joi.object().required())
.length(2),
.ordered(Joi.function().required(), Joi.object().required()),
Joi.function(),
Joi.object(),
)
.default([]);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This completes the TODO for validating 'babel style' input and fixes the validation issue

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ordered is really cool

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, looked for that but couldn't find it. Joi doc is a pain :D

@@ -107,8 +107,7 @@
"webpack-bundle-analyzer": "^3.6.1",
"webpack-dev-server": "^3.11.0",
"webpack-merge": "^4.2.2",
"webpackbar": "^4.0.0",
"@docusaurus/utils-validation": "^2.0.0-alpha.61"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a duplicate dependency, the other one is on line 50

expect(value).toEqual(DEFAULT_OPTIONS);
expect(error).toBe(undefined);
Copy link
Contributor Author

@teikjun teikjun Aug 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is possible for value to match when there is an error, so it is safer to include this

@slorber
Copy link
Collaborator

slorber commented Aug 11, 2020

LGTM thanks

@slorber slorber merged commit fda1590 into facebook:master Aug 11, 2020
@slorber slorber added the pr: bug fix This PR fixes a bug in a past release. label Aug 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Signed Facebook CLA pr: bug fix This PR fixes a bug in a past release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error when including rehype plugins
5 participants