lumeland / lume

🔥 Static site generator for Deno 🦕
https://lume.land
MIT License
1.75k stars 74 forks source link

feat(metas): add `other` tags field #604

Closed niklasmtj closed 1 month ago

niklasmtj commented 2 months ago

Description

This PR adds the possibilty to add custom meta tags via the metas plugin. It is possible via the other attribute in the metas field.

An code example:

metas:
  title: "My great blog post"
  other:
    "twitter:label1": Reading time
    "twitter:data1": 1 minute

Which results in:

<meta name="twitter:label1" content="Reading time">
<meta name="twitter:data1" content="1 minute">

The background of this PR was adding twitter:label1 and twitter:data1 meta tags to use the preview features that are now available to build previews with. It uses the twitter prefix but is also available on e.g. Slack.

Examples:

X

levelsio on X

See Nomad Score and Nomad Cost here.

Slack

Add support for twitter label meta

From an implementation point of view I took the way that NextJS does it to make them able to add them via the other attribute in the metadata fields. See this discussion

I'm open for feedback to get this feature landed ☺️

Related Issues

none - yet

Check List

niklasmtj commented 2 months ago

After having another look on the getDataValue function I think we have to extend it to be able to work with arrays. Right now I'm not supporting the possibilty to use something like =others. But I was not 100% how the best way to implement would look like.

oscarotero commented 2 months ago

Thank! Your implementation looks great! The only problem is the metas object has one more level of depth that will cause merging issues.

For example, if in the root _data.yml file you have some defaults:

metas:
  other:
    "twitter:label1": Reading time

And you want to customize the value in a nested _data file:

metas:
  other:
    "twitter:data1": 1 minute

The complete other object will be overrided, losing the twitter:label1 key, because Lume only merges the first level of the object.

I can think of different solutions:

  1. Allow extra keys in the root of the metas object. Any key different to the standard keys will be added as is.
metas:
  title: "My great blog post"
  "twitter:label1": Reading time
  "twitter:data1": 1 minute
  1. Or create a new mode to merge objects recursively. For example recursiveObject and assign this mode by default to the metas key.

I guess the solution 1 is the most simple to implement, but I would like to hear your opinion.

niklasmtj commented 2 months ago

Oh thanks for the feedback. This totally makes sense! I forgot about the merging "only" on the first level.

I think extending the MetaData with something like

interface MetaData {
...
   [key: string]: string
}

To get it to accept dynamic keys in the type. Based on the stack overflow answer. And then maybe destructure the incoming object to get the non fixed defined keys.

I have to try it in the next few days that might be a good idea. Sounds like a first plan to start with. Thanks!

oscarotero commented 1 month ago

@niklasmtj Hey there! Have you been able to make any progress in this? No problem if you need more time, just want to know it because I want to release a new Lume version soon and I'd like to include this feature. Or let me know if you are not going to work on this (because reasons) so I can continue your work. Thanks!

niklasmtj commented 1 month ago

@niklasmtj Hey there! Have you been able to make any progress in this? No problem if you need more time, just want to know it because I want to release a new Lume version soon and I'd like to include this feature. Or let me know if you are not going to work on this (because reasons) so I can continue your work. Thanks!

Hey sorry, had a busy week. We can also collaborate on this! So if you have an idea on how to implement it easily, go forward :)

oscarotero commented 1 month ago

@niklasmtj I cannot commit to your PR (obviously) so created this new branch and PR #608 with your commits and additional changes to make the metas object flat. So I'm closing this. Thanks so much!