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(seo-meta): rework meta tag mapping #220

Merged
merged 2 commits into from Sep 10, 2023
Merged

fix(seo-meta): rework meta tag mapping #220

merged 2 commits into from Sep 10, 2023

Conversation

dargmuesli
Copy link
Contributor

@dargmuesli dargmuesli commented Sep 10, 2023

πŸ”— Linked issue

n/a

❓ Type of change

  • πŸ“– Documentation (updates to the documentation, readme, or JSdoc annotations)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

The new meta tags added in harlan-zw/zhead#18 are not prefixed with og. That way, they are not properly added to the head (see first test commit).
I observe that keys (name or property) us a custom formats:

  • msapplication-TileImage
  • og:audio:secure_url\

So there is no standard way to format such a key. The html standard keys seem to be all lowercase with dashes, so I kept this as standard formatting.
For all other keys, I propose to extend the MetaPackingSchema, dropping the prefix logic that's currently used e.g. for open graph tags as that logic may overlap with future keys (imagine meta information about ogres like ogre:name 😁 you get the idea).
The algorithm I implemented should work for any depth of object nesting.

Please correct me if I could make better use of packrup. It's somehow not trivial to grasp right away for my head if I could utilize that library better.

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@vercel
Copy link

vercel bot commented Sep 10, 2023

The latest updates on your projects. Learn more about Vercel for Git β†—οΈŽ

Name Status Preview Comments Updated (UTC)
unjs-unhead βœ… Ready (Inspect) Visit Preview πŸ’¬ Add feedback Sep 10, 2023 3:22am

@harlan-zw
Copy link
Collaborator

harlan-zw commented Sep 10, 2023

Seems good to me and if tests are passing then I think we're good.

The only downside is that we're adding some weight to the useSeoMeta composable but think that may be unavoidable, meta tags are a mess in terms of conventions.

Thanks for the hard work πŸ‘

@harlan-zw harlan-zw merged commit 8da9d86 into unjs:main Sep 10, 2023
3 checks passed
@dargmuesli dargmuesli deleted the fix/seo-meta/rework branch September 10, 2023 22:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants