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(seo): head content is configurable per pages #588

Closed
wants to merge 9 commits into from

Conversation

georges-gomes
Copy link
Collaborator

@georges-gomes georges-gomes commented Mar 25, 2022

Background

This changes has been created to improve quality of meta tags in head for social links and SEO.

A number of tags are required in <head>.

  • Meta for OG
  • Meta for Twitter
  • Canonical URL
    ...

Some of this content is page specific like title and description but also potentially images.

Currently, some of this information is filled by VitePress, some can be added to head via frontmatters
https://vitepress.vuejs.org/guide/frontmatter.html#head
But the first one has precedence to the other one and the notation is quickly very verbose.

Solution here

The solution here is to support a function in the head configuration
HeadConfig[] | (page: DataPage) => HeadConfig[]

So every page is rendered with this function.

It let you have:

  • og:title or twitter:title with the title of the page without the extra Title decoration
  • og:description or twitter:description
  • create a canonical for each page
  • generate or link to meta images appropriatly
  • add tags in frontmatters of the page and transform them into the right head tags
  • dynamically change anything in the meta tags based on the page and page frontmatters

Not supported

Page routing doesn't update head tags.

@georges-gomes georges-gomes marked this pull request as ready for review March 25, 2022 20:24
@georges-gomes
Copy link
Collaborator Author

@georges-gomes
Copy link
Collaborator Author

fixes #551

@georges-gomes
Copy link
Collaborator Author

Hi @kiaking
Let me know if you like that.
I think this feature is better than adding individual tags support in frontmatters.
We use it in production already.
At your service

@kiaking
Copy link
Member

kiaking commented May 23, 2022

@georges-gomes Thanks for the PR and this is indeed cool feature. But let me think bit more deeper on this.

It's super flexible and good, but maybe most of these meta should be injected automatically. From that perspective, maybe there might be simpler way to customize for users...? I don't know, but I'll think about this a bit more!

@kiaking
Copy link
Member

kiaking commented May 23, 2022

I'll add #551 to 1.0.0 milestone so we don't forget.

@georges-gomes
Copy link
Collaborator Author

@kiaking Great. Happy to revamp another implementation if you come across a better idea 👍


// Open Graph
['meta', { property: 'og:type', content: 'website' }],
['meta', { property: 'og:locale', content: lang }],
Copy link
Contributor

Choose a reason for hiding this comment

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

og:locale uses underscore instead of the BCP 47 hyphen in HTML lang to separate the subtags, so language tags like en-US should be converted to en_US.

@brc-dd brc-dd mentioned this pull request Jul 13, 2022
4 tasks
@treystout
Copy link

@georges-gomes thank you for this PR, hoping it's merged in some form soon. Could you explain what you mean by "Page routing doesn't update head tags." Does this mean crawlers would see the right tags but developers would not?

@brc-dd
Copy link
Member

brc-dd commented Aug 5, 2022

Does this mean crawlers would see the right tags but developers would not?

@treystout The tags will be there in the generated HTML (yeah, crawlers would see the right tags) but it won't be updated (client-side, in browser) when a user moves from one page to another (on reload they'll see the right tags).

@treystout
Copy link

@brc-dd thank you for clarifying. Is the team still deciding if this PR is relevant? The only thing I can think of that would make it more expressive for end-users is ensuring @georges-gomes getHead has sufficient context. But adding arguments to it later would be easy and backward compatible. Anything I can do to help move either this PR forward or a another approach? Meta/Head issues are stopping my team from using vitepress in production, and it seems like something almost everyone would need once hosting in production. Thank you!

@brc-dd
Copy link
Member

brc-dd commented Aug 5, 2022

The only thing I can think of that would make it more expressive for end-users is ensuring getHead has sufficient context.

It still has sufficient context, you can access nearly everything from pageData itself.

Is the team still deciding if this PR is relevant?

The PR is obviously relevant and much demanded TBH (needs rebase though). We're thinking of implementing some more compact API though, but even in its current form the PR is cool. We'll try to get this feature landed as soon as possible.

@georges-gomes
Copy link
Collaborator Author

@brc-dd How do you feel about this PR? Should I rebase it or you are not sure about it?

@georges-gomes
Copy link
Collaborator Author

@georges-gomes thank you for this PR, hoping it's merged in some form soon. Could you explain what you mean by "Page routing doesn't update head tags." Does this mean crawlers would see the right tags but developers would not?

Sorry for the late reply, missed the notification.
Yes, the head tags are good at "arrival" and Google will see them.
But when a user route to another page the head tags are not updated with the values of the new page.
Doesn't affect Google but could be considered as "not good enough" for Vitepress.

We are using this as a patch in production with dynamic meta tags on https://backlight.dev/docs/ and https://backlight.dev/mastery/ if you want to see it in action.

@brc-dd
Copy link
Member

brc-dd commented Nov 10, 2022

How do you feel about this PR? Should I rebase it or you are not sure about it?

@georges-gomes It will need rebase, but not right now. Let's merge #1339 first. Likely there will be conflicts after that. But resolving them on this PR will be easier. Also, I wanted to ask, isn't transformHead behaves similar to what we have in this PR?

@georges-gomes
Copy link
Collaborator Author

How do you feel about this PR? Should I rebase it or you are not sure about it?

@georges-gomes It will need rebase, but not right now. Let's merge #1339 first. Likely there will be conflicts after that. But resolving them on this PR will be easier. Also, I wanted to ask, isn't transformHead behaves similar to what we have in this PR?

Good question, I'm discovering transformHead :) Let me check and I will update here.

@kiaking
Copy link
Member

kiaking commented Jan 27, 2023

Closing due to inactivity. I think this is useful feature, though maybe we might wanna discuss in issues first to figure out the best API for it 👍

@kiaking kiaking closed this Jan 27, 2023
@brc-dd
Copy link
Member

brc-dd commented Jan 27, 2023

People coming to this later, please try transformHead hook. IMO it does the same thing as this PR. Let me know if I'm wrong.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants