Proposal: refactor the processing of meta elements

GoodbyeNJN commented 9 months ago

As I said in this issue, I've rearranged the attributes that may appear on the meta element, as well as new test cases for the meta element.

In the following, I have bolded some of the content to be discussed.

Please review it when you are free. @DaniGuardiola is also welcome to review it together. Any suggestions are very welcome.

Attributes (spec)

name: Metadata name
http-equiv: Pragma directive
content: Value of the element
charset: Character encoding declaration
media: Applicable media
itemprop: Microdata, may appear on any HTML element

Additional attributes (spec)

property: Extended attribute by RDFa, commonly used in the Open Graph protocol

Rules

Exactly one of the name, http-equiv, charset, and itemprop attributes must be specified.
If either name, http-equiv, or itemprop is specified, then the content attribute must also be specified. Otherwise, it must be omitted.
If the charset attribute is present, its value must be an ASCII case-insensitive match for the string "utf-8".
There must not be more than one meta element with a charset attribute per document.
The media attribute says which media the metadata applies to.

Additional rules

If property is specified, then the content attribute must also be specified. (not for sure, but it seems to be true)
If name is not specified but property is specified, then the property attribute should be treated as name attribute. (Do users need to be warned about this compatibility process?)
If name and property are specified, then the property attribute should be the same as name attribute. (Does this rule should be added to simplify processing?)

Test cases

renders only valid meta

<!-- invalid -->
<Meta />
<Meta content="foo" />
<Meta media="foo" />
<Meta property="foo" />
<Meta name="foo" http-equiv="bar" content="baz" />
<Meta name="foo" charset="bar" content="baz" />
<Meta name="foo" itemprop="bar" content="baz" />
<Meta http-equiv="foo" charset="bar" content="baz" />
<Meta http-equiv="foo" itemprop="bar" content="baz" />
<Meta charset="foo" itemprop="bar" content="baz" />

<Meta name="foo" />
<Meta http-equiv="foo" />
<Meta itemprop="foo" />
<Meta charset="foo" content="bar" />
<Meta media="foo" content="bar" />

<Meta charset="foo" />
<Meta charset="UTF-8" />

<Meta charset="utf-8" /><Meta charset="utf-8" />

<Meta charset="utf-8" property="foo" />
<!-- invalid -->

<!-- valid -->
<Meta name="foo" content="bar" />
<Meta http-equiv="foo" content="bar" />
<Meta itemprop="foo" content="bar" />
<Meta charset="utf-8" />
<Meta property="foo" content="bar" />

<Meta name="foo" media="bar" content="baz" />
<Meta http-equiv="foo" media="bar" content="baz" />
<Meta itemprop="foo" media="bar" content="baz" />
<Meta charset="utf-8" media="bar" />

<Meta name="foo" property="bar" content="baz" />
<Meta http-equiv="foo" property="bar" content="baz" />
<Meta itemprop="foo" property="bar" content="baz" />
<!-- valid -->

renders only first meta with charset

<Meta charset="utf-8" />
<Meta charset="utf-16" />
<Meta charset="utf-32" />

renders only last meta with the same name/property/http-equiv/itemprop

<Meta name="name1" content="content1" />
<Meta name="name2" content="content2" />
<Meta name="name3" content="content3" />

<Meta http-equiv="http-equiv1" content="content1" />
<Meta http-equiv="http-equiv2" content="content2" />
<Meta http-equiv="http-equiv3" content="content3" />

<Meta itemprop="itemprop1" content="content1" />
<Meta itemprop="itemprop2" content="content2" />
<Meta itemprop="itemprop3" content="content3" />

renders only last meta with same media

<Meta name="name1" media="media1" content="content1" />
<Meta name="name1" media="media1" content="content2" />
<Meta name="name1" media="media1" content="content3" />

renders all meta with different media

<Meta name="name1" media="media1" content="content1" />
<Meta name="name1" media="media2" content="content2" />
<Meta name="name1" media="media3" content="content3" />

renders only last meta with same property

<Meta name="property1" property="property1" content="content1" />
<Meta name="property2" property="property2" content="content2" />
<Meta name="property3" property="property3" content="content3" />

<Meta property="property1" content="content1" />
<Meta property="property2" content="content2" />
<Meta property="property3" content="content3" />

WIP: meta with name&property mixed

<!-- all meta should be rendered -->
<Meta name="name1" property="property1" content="content1" />
<Meta name="name2" property="property2" content="content2" />
<Meta name="name3" property="property3" content="content3" />

<!-- **which should be rendered?** -->
<Meta name="name1" property="property1" content="content1" />
<Meta name="name1" property="property2" content="content2" />
<Meta name="name1" property="property3" content="content3" />

<!-- **which should be rendered?** -->
<Meta name="name1" property="property1" content="content1" />
<Meta name="name2" property="property1" content="content2" />
<Meta name="name3" property="property1" content="content3" />

<!-- **which should be rendered?** -->
<Meta name="name1" property="property1" content="content1" />
<Meta name="name1" content="content2" />
<Meta property="name1" content="content3" />

DaniGuardiola commented 9 months ago

Thanks, though honestly, I feel like this is too complicated. I think the less surprising (for users) and easier to maintain (for devs) path is keeping things very simple and flat, with no complicated heuristics and no overriding behavior by default. And for overridability, introduce an opt-in key prop. This, basically: https://github.com/solidjs/solid-meta/pull/39

yume-chan commented 4 months ago

renders only last meta with same property

<Meta name="property1" property="property1" content="content1" />
<Meta name="property2" property="property2" content="content2" />
<Meta name="property3" property="property3" content="content3" />

<Meta property="property1" content="content1" />
<Meta property="property2" content="content2" />
<Meta property="property3" content="content3" />

This is not (always) correct. Open Graph declares arrays using multiple <meta>s with same property:

https://ogp.me/#array

Arrays

If a tag can have multiple values, just put multiple versions of the same tag on your page. The first tag (from top to bottom) is given preference during conflicts.
<meta property="og:image" content="https://example.com/rock.jpg" />
<meta property="og:image" content="https://example.com/rock2.jpg" />

GoodbyeNJN commented 4 months ago

@yume-chan Yes, you are right, I forgot to consider the Open Graph case.

Now that Open Graph has a certain format for <meta property="og:xxx" content="xxx"/>, perhaps we could consider checking if the property starts with og: and applying additional rules to it.

What do you think? Your ideas are welcome.

By the way, it looks like this project is no longer active and I'm not quite sure when this proposal will be adopted by the maintainers...

yume-chan commented 4 months ago

checking if the property starts with og: and applying additional rules to it.

Maybe Open Graph is too complex that it worths its own package.

Open Graph arrays make it impossible for the heuristics to decide whether multiple <meta property="og:image"> tags should co-exist or override each other. I think it's reasonable to support the scenario that a default image is provided in the root component, then overridden by multiple images in each route.

Some user input, like #39, might be required. Then the dedicated Open Graph package can choose to override <OpenGraphTitle> and <OpenGraphDescription> by default, and let the user to choose what to do with <OpenGraphImage>s. (maybe with a <OpenGraphImages> component that accepts one or more <OpenGraphImage>s, only <OpenGraphImages> will override each other).

By the way, it looks like this project is no longer active

I hope it's not 😞. However npm overrides is always an option.

GoodbyeNJN commented 4 months ago

Open Graph arrays make it impossible for the heuristics to decide whether multiple <meta property="og:image"> tags should co-exist or override each other. I think it's reasonable to support the scenario that a default image is provided in the root component, then overridden by multiple images in each route.

I agree with this. There is really no need to re-implement the parsing logic of Open Graph tags in this project.

solidjs / solid-meta