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

feat(v2): exhaustive BlogPostFrontMatter schema validation #4759

Merged
merged 5 commits into from
May 14, 2021

Conversation

nam-hle
Copy link
Contributor

@nam-hle nam-hle commented May 8, 2021

Motivation

From #4591.

What the PR does

  • Complete the BlogPostFrontMatter typing and the corresponding Joi schema validator in packages/docusaurus-plugin-content-blog/src/blogFrontMatter.ts
  • Add unit tests
  • Display friendly messages for wrong schemas:

image

  • @slorber: Can you explain the sentence: "try to use the frontmatter TS types in the theme?" ?

I found the after parsing the frontmatter use gray-matter package, the date field will be converted to a Date object, not a raw string. But we are treating it as a string. Currently there is no problem about it, but maybe furture.

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

Unit test.

@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label May 8, 2021
@netlify
Copy link

netlify bot commented May 8, 2021

[V2]

Built with commit 3a65621

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

@github-actions
Copy link

github-actions bot commented May 8, 2021

⚡️ Lighthouse report for the changes in this PR:

Category Score
🟠 Performance 76
🟢 Accessibility 96
🟢 Best practices 100
🟢 SEO 100
🟢 PWA 95

Lighthouse ran on https://deploy-preview-4759--docusaurus-2.netlify.app/

@nam-hle nam-hle changed the title Validate BlogPostFrontMatter and add tests Strict Validate BlogPostFrontMatter schema May 8, 2021
@nam-hle nam-hle changed the title Strict Validate BlogPostFrontMatter schema fix(v2): improve BlogPostFrontMatter schema validation May 9, 2021
@nam-hle nam-hle marked this pull request as ready for review May 9, 2021 03:07
Copy link
Collaborator

@slorber slorber left a comment

Choose a reason for hiding this comment

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

Thanks!

Overall that looks good. Added a few useful changes to do

website/docs/blog.md Outdated Show resolved Hide resolved
Copy link
Contributor Author

@nam-hle nam-hle left a comment

Choose a reason for hiding this comment

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

Thanks for reviewing. I add an annotation for deprecated fields and provide warning messages for them.

website/docs/blog.md Outdated Show resolved Hide resolved
@netlify
Copy link

netlify bot commented May 14, 2021

[V1]

Built with commit 3a65621

https://deploy-preview-4759--docusaurus-1.netlify.app

@slorber
Copy link
Collaborator

slorber commented May 14, 2021

Thanks, that looks great!

Disabling the warnings for now as our own blog posts are still using those frontmatters (content is shared between v1/v2 so we can't update the frontmatter)

@slorber slorber merged commit e092910 into facebook:master May 14, 2021
@slorber slorber added the pr: new feature This PR adds a new API or behavior. label May 14, 2021
@slorber slorber changed the title fix(v2): improve BlogPostFrontMatter schema validation feat(v2): improve BlogPostFrontMatter schema validation May 14, 2021
@slorber slorber changed the title feat(v2): improve BlogPostFrontMatter schema validation feat(v2): exhaustive BlogPostFrontMatter schema validation May 14, 2021
@nam-hle nam-hle deleted the strict-frontmatter branch May 15, 2021 00:25
@nam-hle
Copy link
Contributor Author

nam-hle commented May 15, 2021

Thanks. Now I can work on the docs frontmatter smoothly :D

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: new feature This PR adds a new API or behavior.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants