feat(remix-react): support Open Graph "verticals" in meta function

[donavon]

According to the Open Graph spec you can have "verticals" that have a prefix other than og:. These verticals have a prefix that matches the value specified in og:type up until the first period.

For example, there is a vertical of "music". Within "music" you can have an og:type of "music.song".

When you specify og:type of "music.song" you can now have additional meta elements that are prefixed with music: (not og:) that must also use "property" instead of "name".

For example, this is valid:

<title>Baby's Got Back</title>
<meta property="og:type" content="music.song" />
<meta property="music:musician" content="Sir Mix-a-Lot" />
<meta property="music:album" content="Mack Daddy" />

But currently, Remix renders the two "music" metas as name.

<meta name="music:musician" content="Sir Mix-a-Lot" />
<meta name="music:album" content="Mack Daddy" />

This PR will fix this.

Production issue

I have a real-life example of this bug. My blogging engine uses an og:type of "article" and erroneously renders this in production. (see https://donavon.com/blog/remix-locale)

NOTE: I've patched components.js so I no longer have this issue in prod.

<meta property="og:type" content="article">
<meta name="article:published_time" content="2022-03-25T00:00:00.000Z">

With this PR, it will render correctly as:

<meta property="og:type" content="article">
<meta property="article:published_time" content="2022-03-25T00:00:00.000Z">

[kiliman]

This PR #2331 extends the meta function to allow for object syntax.

export function meta() {
  return {
    // Special cases
    title: 'My title', // <title>My title</title>
    charset: "utf-8", // <meta charset="utf-8" />
    "og:xxx": "url", // <meta property="og:xxx" content="url" />
    // content => name
    description: 'My description', // <meta name="description" content="My description" />
    "twitter:card": "body", // twitter uses content and name, so I can do that
    // <meta {...value} />
    "fb:app_id": { property: 'fb:app_id', content: '1234' }, // <meta property="fb:app_id" content="1234" />
    "refresh": { httpEquiv: 'refresh', content: '3;url=https://www.mozilla.org' }, // <meta http-equiv="refresh" content="3;url=https://www.mozilla.org" />
  }
}

So you'll be able to do this. Remix simply spreads the object into a new meta element.

"article:published_type": { property: "article:published_type", content: "..." }

This way Remix will no longer need to special case property names.

[donavon]

You can never remove og: or it will be a breaking change. This PR simply corrects the special assumption.

I don't see that #2331 negates this PR at all.

[donavon]

And why should a user suddenly need to know that Open Graph uses "property" instead of "name"? They didn't before.

With my fix, this just works:

export function meta() {
  return {
    "og:type": "article",
    "article:published_time": "2022-03-25T00:00:00.000Z"
  }
}

Without it (and with #2331) they need to jump through hoops:

export function meta() {
  return {
    "og:type": "article",
    "article:published_time": { property: "article:published_time", content: "2022-03-25T00:00:00.000Z" }
  }
}

I love the fact that #2331 adds so much flexibility (and it should definitely be part of remix ASAP), but what a hack compared to my fix. "article:published_time" defined twice? That should have been a red flag right there.

IMO, both PRs should be merged.

[donavon]

If I were part of the discussion on #2331, I would have liked to see this for your last two examples:

"property": { // define one or more elements that use "property" vs "name"
  'fb:app_id', '1234' // <meta property="fb:app_id" content="1234" />
  'foo', 'bar' // <meta property="foo" content="bar" />
},
"httpEquiv": { // define one or more elements that use "httpEquiv" vs "name"
  'refresh', '3;url=https://www.mozilla.org' // <meta http-equiv="refresh" content="3;url=https://www.mozilla.org" />
}, 

[sergiodxa]

I agree on merging this one, the code is really simple, and between just using the meta tag of OG verticals and the other approach using objects this is way simpler.

[kiliman]

I believe the issue they were trying to solve is to eliminate the need for all these special cases. Would have been nice if the HTML spec simply said any key that includes ":" should use property instead of name, or better yet, why the distinction?

And why should a user suddenly need to know that Open Graph uses "property" instead of "name"? They didn't before.

They would have to do <meta property="article:published_time" ...>, so they should already know they need property instead of name.

"article:published_time" defined twice?

Unfortunately the object syntax requires a unique key. Remix doesn't care about the key name if you're using the object syntax, so you could name it anything.

"property": {
  'fb:app_id', '1234' // <meta property="fb:app_id" content="1234" />
  'foo', 'bar' // <meta property="foo" content="bar" />
},
"httpEquiv": {
  'refresh', '3;url=https://www.mozilla.org' // <meta http-equiv="refresh" content="3;url=https://www.mozilla.org" />
}, 

Hmm.. this makes property have a special meaning, as well as httpEquiv. So what happens if some meta tag uses property but also more than just content=value? The object syntax allows you to spread all values. Maybe it would have been better if meta worked like links and just returned an array. Then you could just do:

export const meta = () => [
  { title: 'my title' },
  { charset: "utf-8" },
  { property: "og:xxx", content: "url" },
  { property: "article:published_time", content: "..." },
  { name: "description", content: "..." },
  { name: "twitter:card", content: "..." },
  { property: "fb:app_id", content: "..." },
  { httpEquiv: "refresh", content: "..." },
  ...
]

[donavon]

There's no reason that meta can't take an array in addition to an object. Meta could do something different if it found an array. At this point, I'd like to see #2331 reverted. I've already reserved the npm name remix-better-meta to translate a simpler meta syntax into what was merged in #2331 if it's not. (the double/unused key thing is still nagging at me)

[kiliman]

Yeah, I think they'd accept a PR for an array version of meta. Then the only special case is title, since it manipulates the <title> element. Everything else just spreads props onto <meta>.

I think this should be the preferred method going forward and deprecate the object syntax.

[donavon]

Then the only special case is title

Not true. They can't remove og:* without a breaking change and a major version bump.

[kiliman]

I meant for the array version. Sorry for not being clear.

[donavon]

If I had my druthers, they would revert #2331 (as it hasn't shipped), add support for arrays, and merge this PR, though I doubt I will get my druthers.

[kiliman]

Something like this

const metaValues = route.meta({data: route.loaderData))
if (Array.isArray(metaValues)) {
  return metaValues.map(entry => {
    if (entry.hasOwnProperty('title')) {
      return <title>{entry.title}</title>
    } else {
      return <meta {...entry}/>
    }
  })
}

[donavon]

a quick code review... ;)

You should allow title as an attribute when spreading into meta. use <title/> only if title is the only key.

const metaValues = route.meta({ data: route.loaderData });
if (Array.isArray(metaValues)) {
  return metaValues.map((entry) =>
    entry.hasOwnProperty("title") && Object.keys(entry).length == 1 ? (
      <title>{entry.title}</title>
    ) : (
      <meta {...entry} />
    )
  );
}

[kiliman]

Ha, well not bad for browser code.

[donavon]

oops! we forgot key. I always forget key. :)

[MichaelDeBoey]

@donavon Could you please rebase your branch onto latest dev & resolve conflicts?

I think this is a great addition together with #2331.
Having meta function support an array would indeed be another great addition imo.

[donavon]

@MichaelDeBoey done

[ryanflorence]

I wish I had seen #2331 before it got merged. Please remember not to merge things that change the public API without the approval from me or @mjackson.

When I first made meta I thought we might be able to simplify it for people with the key:value approach and a couple one-offs. Technically property isn't part of the spec and as far as I could tell "og:" was the only thing that broke it. For the more strange ones (like refresh) I figured you'd just render a normal <meta> in root.tsx.

As we've seen a handful of adjustments to our one-offs for meta, I've been expecting to add the ability to return an array and just render it like we do links and deprecate the object. This way, Remix doesn't have to know anything special and folks can render any non-standard meta tag they want. External libraries can convert objects to remix meta arrays if people want more convenience.

export function meta() {
  return [
    { title: "okay, still one off" }, // 👈 still need one weirdo w/ arrays though
    { beef: "who cares", content: "not remix" }
    // <meta beef="who cares" content="not remix" />
  ]
]

I'd actually like to

• revert feat(remix-react): add support for http-equiv in meta function #2331 ASAP before it gets any traction
• not merge this PR either
• do merge a PR that accepts an array as the return value
• continue to accept the object so we don't break apps, but console.warn that it has been deprecated

@donavon you up for it? 🙏

[ryanflorence]

Also note, it's not as simple as links where we literally just render all of them.

We'll need to still do a "merge" where the child meta wins. We can't have two of the same meta tags:

🚫
<title>parent</title>
<title>child</title>
<meta name="description" content="parent" />
<meta name="description" content="child" />

[donavon]

@ryanflorence

I'm good with not accepting this PR and reverting object syntax in lieu of an array PR and understand on needing to "merge". But what would you use as a "key" when you merged? name? property? (🤮) special case of title? a combination of all?

[ryanflorence]

what would you use as a "key" when you merged

I dunno, that's probably why I used an object in the first place 😂

[donavon]

of source, technically, you would need to bump the major version as object syntax is out there.

[donavon]

playing devil's advocate for a second, a merge key is exactly what object syntax provides. 🤔

[ryanflorence]

Like the 10 second rule when food falls on the floor, semver has a 10 day rule 😜

[ryanflorence]

If you can't figure out a good key, another way around this is to only use meta from the leaf route and warn when meta is used in a layout route. That way there is no merge. Only the leaf route gets to add meta to avoid the problem altogether (this includes index routes)

[MichaelDeBoey]

only use meta from the leaf route

This would mean we would break having charset or viewport in the root

[donavon]

Just spitballing here...

if a property called key exists, use it (like React's key). if would not make it into the HTML.

else if name exists, use that as the key
else if property exists, use that as the key
else if title exists, use that as the key
else don't use a key and warn "no key found, you should set a key"

function warn() { console.warn("no key found, you should set a key") }

const { name, property, title, key=name ?? property ?? title ?? warn()} = obj;

if (key) //merge here

[kiliman]

BTW: I'm working on the Array version of meta.

I'm converting the special-case props to the array version, so the <meta> tag generation is simple.

[kiliman]

Ok, I have a draft PR #2598.

I've added tests, but for some reason, I can't get the integration tests to run (even on a clean build, I get errors).

I have stand-alone test that shows that it properly handles both object-syntax and array-syntax with correct overrides.

I will update the documentation after the initial review.

[machour]

Closing this since we have the new meta API since 1.8.0