feat: add metatags support for seo / blogposts #5373 #5375

johnnyreilly · 2021-08-18T09:42:52Z

Motivation

Docusaurus does not provide og:type at present. It would be good if it did; perhaps with a value of website or article. eg:

<meta property="og:type" content="article" />

This PR provides that support through a generic meta tags mechanism which is used only by the blog mechanism at present. Blogs get the og:type of article as well as some article data, powered by the metadata available:

 {
        'og:type': 'article',
        'article:published_time': date,
        ...(authorURL ? {'article:author': authorURL} : {}),
        ...(frontMatter.tags
          ? {'article:tag': frontMatter.tags.join(',')}
          : {}),
 }

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

Building and running locally you can see the new tags being generated:

<meta property="og:type" content="article" data-react-helmet="true">
<meta property="article:published_time" content="2018-09-11T00:00:00.000Z" data-react-helmet="true">
<meta property="article:author" content="https://github.com/endiliey" data-react-helmet="true">
<meta property="article:tag" content="new,adoption" data-react-helmet="true">

Also visible in the preview:

Related PRs

N/A

netlify · 2021-08-18T09:48:50Z

✔️ [V2]

🔨 Explore the source changes: e59c59a

🔍 Inspect the deploy log: https://app.netlify.com/sites/docusaurus-2/deploys/611fb356d20fcf0008a7a8e2

😎 Browse the preview: https://deploy-preview-5375--docusaurus-2.netlify.app

github-actions · 2021-08-18T09:49:19Z

⚡️ Lighthouse report for the changes in this PR:

Category	Score
🟢 Performance	97
🟢 Accessibility	98
🟢 Best practices	100
🟢 SEO	100
🟢 PWA	95

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

packages/docusaurus-theme-classic/src/theme/BlogPostItem/index.tsx

slorber

This works but I'd prefer another implementation

slorber · 2021-08-19T15:14:00Z

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

@@ -132,6 +132,7 @@ declare module '@theme/Seo' {
    readonly description?: string;
    readonly keywords?: readonly string[] | string;
    readonly image?: string;
+    readonly metaTags?: Record<string, string>;


Honestly, I'm not a fan of using the SEO component for all use-cases but I'd rather keep it small and not increase this API.

Head/helmet is here to handle other use-cases

Also, metas can be property/name (difference matter afaik):

Using Record<string,string> is confusing, less explicit and can only handle one of those 2 cases so it's an error-prone abstraction.

I'd rather use <Head> in BlogPostItem even if this feels duplicate with <Seo>.

Eventually:

add a generic children prop to pass additional head elements inside <Seo>?

let Seo accept any Head prop and spread them? (it has a meta prop btw)

Okay cool - just pushed a different implementation based upon your feedback. Does that look kind of like what you had in mind?

Yes, that looks better to me :)

Another thing to consider: this comp is used in the paginated list view.

This means that on this page: https://deploy-preview-5375--docusaurus-2.netlify.app/blog/
We have 3 times those metas being declared in <Head>, and the last article of the list "wins".

I'm not sure it's what you want in this case. Unlike structured data, page metas override each others.

We have a isBlogPostPage prop on this comp that you could use.
(I don't like it much, it's error-prone as we can see here, and I'd rather use composition and have a dedicated comp for the list blog post item)

Note: I'm not sure Head supports React fragments well 🤪

We have a isBlogPostPage prop on this comp that you could use.

Done!

slorber · 2021-08-19T17:02:33Z

packages/docusaurus-theme-classic/src/theme/BlogPostItem/index.tsx

+          )}
+        </Seo>
+      ) : (
+        <Seo {...{keywords, image}} />


I think we shouldn't need that either and is probably a mistake?

It doesn't make sense to me to render the meta keywords of the last blog post of the list 🤪

What about just moving this Seo comp to BlogPostPage?

The list should have totally different metadata than a single blog post imho

So I wanted SEO to continue doing what it did before... Are you saying you'd like to remove the SEO component entirely from this page? As I didn't add it there; it was already present.

The change as implemented right now renders the SEO component as it already is in the case that this is not a isBlogPostPage, and in the case where it is, it renders the SEO component with the child meta tags.

So the current implementation is this:

<Seo {...{keywords, image}} />

With the changes in this PR that will still be rendered in the case where isBlogPostPage === false

When it is true it will render:

<Seo {...{keywords, image}}> <meta property="og:type" content="article" /> <meta property="article:published_time" content={date} /> {authorURL && <meta property="article:author" content={authorURL} />} {frontMatter.tags && ( <meta property="article:tag" content={frontMatter.tags.join(',')} /> )} </Seo>

If we pull Seo entirely from this component then presumably that breaks other use cases?

I guess we could just put Seo back to how it was before and do this in BlogPostPage:

<Head> <meta property="og:type" content="article" /> <meta property="article:published_time" content={date} /> {authorURL && <meta property="article:author" content={authorURL} />} {frontMatter.tags && ( <meta property="article:tag" content={frontMatter.tags.join(',')} /> )} </Head>

Would that work?

Implemented this approach. That what you had in mind?

🤪

So I wanted SEO to continue doing what it did before... Are you saying you'd like to remove the SEO component entirely from this page?

Yes that's exactly what I mean: what we already do is incorrect and should be fixed, so this PRR is a good opportunity to do so :)

https://socialsharepreview.com/?url=https://docusaurus.io/blog

=> we should probably not display the social card + keywords of the last blog post of the list here.

Just move Seo to BlogPostPage instead (even if it's not as before)

It does not make much sense to me anyway to declare SEO metadata on reusable comps that can be useful in both list+details pages.

Metadatas should preferably be handled at the page entry-point level: BlogListPage + BlogPostPage, not BlogPostItem

Okay cool - I'm away from my keyboard the next couple of weeks. If you'd like to work on this branch in the meantime please feel free. If not I can take a look when I'm back!

This reverts commit 1cba459

feat: add metatags support for seo / blogposts

29afb64

johnnyreilly requested review from lex111 and slorber as code owners August 18, 2021 09:42

facebook-github-bot added the CLA Signed Signed Facebook CLA label Aug 18, 2021

johnnyreilly mentioned this pull request Aug 18, 2021

Open Graph tags missing required property of og:type #5373

Closed

5 tasks

lex111 reviewed Aug 18, 2021

View reviewed changes

packages/docusaurus-theme-classic/src/theme/BlogPostItem/index.tsx Outdated Show resolved Hide resolved

lex111 approved these changes Aug 18, 2021

View reviewed changes

slorber requested changes Aug 19, 2021

View reviewed changes

feat: different implementation

84bf24c

slorber added the pr: new feature This PR adds a new API or behavior. label Aug 19, 2021

feat: use isBlogPostPage

88e6d74

slorber reviewed Aug 19, 2021

View reviewed changes

johnnyreilly and others added 7 commits August 19, 2021 18:28

feat: implement in BlogPostPage-remove Seo changes

1cba459

Revert "feat: implement in BlogPostPage-remove Seo changes"

89f302f

This reverts commit 1cba459

Merge branch 'main' into johnnyreilly/main

0af2ba9

Move Seo to BlogPostPage + some fixes

d453539

fix social preview asset

1aa0cda

Fix blog social image + improve a bit Seo setup

bac2b33

fix bootstrap theme

e59c59a

slorber merged commit 0859704 into facebook:main Aug 20, 2021

swyxio mentioned this pull request Sep 2, 2021

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

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add metatags support for seo / blogposts #5373 #5375

feat: add metatags support for seo / blogposts #5373 #5375

johnnyreilly commented Aug 18, 2021 •

edited

Loading

netlify bot commented Aug 18, 2021 •

edited

Loading

github-actions bot commented Aug 18, 2021 •

edited

Loading

slorber left a comment

slorber Aug 19, 2021

johnnyreilly Aug 19, 2021

slorber Aug 19, 2021

johnnyreilly Aug 19, 2021

slorber Aug 19, 2021

slorber Aug 19, 2021

johnnyreilly Aug 19, 2021

johnnyreilly Aug 19, 2021

johnnyreilly Aug 19, 2021 •

edited

Loading

johnnyreilly Aug 19, 2021

johnnyreilly Aug 19, 2021

slorber Aug 20, 2021 •

edited

Loading

johnnyreilly Aug 20, 2021

slorber Aug 20, 2021

feat: add metatags support for seo / blogposts #5373 #5375

feat: add metatags support for seo / blogposts #5373 #5375

Conversation

johnnyreilly commented Aug 18, 2021 • edited Loading

Motivation

Have you read the Contributing Guidelines on pull requests?

Test Plan

Related PRs

netlify bot commented Aug 18, 2021 • edited Loading

github-actions bot commented Aug 18, 2021 • edited Loading

slorber left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johnnyreilly Aug 19, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

slorber Aug 20, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johnnyreilly commented Aug 18, 2021 •

edited

Loading

netlify bot commented Aug 18, 2021 •

edited

Loading

github-actions bot commented Aug 18, 2021 •

edited

Loading

johnnyreilly Aug 19, 2021 •

edited

Loading

slorber Aug 20, 2021 •

edited

Loading