remix icon indicating copy to clipboard operation
remix copied to clipboard

feat(remix-react): Add array-syntax for `meta` export

Open kiliman opened this issue 3 years ago • 8 comments
trafficstars

This PR adds array-syntax to the meta export.

export const meta: MetaFunction = ({data}) => [
  { name: "description", content: data.description },
  { property: "og:image", content: "https://remix.run/logo.png" },
  { property: "og:type", content: "image/png" },
  { key: "http-equiv:refresh", httpEquiv: "refresh", content: "5;url=https://google.com" },
  { title: data.title },
  { name: "viewport", content: "width=device-width, initial-scale=1" },
];

kiliman avatar Apr 01 '22 20:04 kiliman

@kiliman Could you fix conflicts here please?

machour avatar Sep 03 '22 14:09 machour

Hello team, I want to mention a scenario I found, where open graph og:image accept multiple metatags with the same name, as described at: https://ogp.me/#array

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" />

Put structured properties after you declare their root tag. Whenever another root element is parsed, that structured property is considered to be done and another one is started.

For example:

<meta property="og:image" content="https://example.com/rock.jpg" />
<meta property="og:image:width" content="300" />
<meta property="og:image:height" content="300" />
<meta property="og:image" content="https://example.com/rock2.jpg" />
<meta property="og:image" content="https://example.com/rock3.jpg" />
<meta property="og:image:height" content="1000" />

means there are 3 images on this page, the first image is 300x300, the middle one has unspecified dimensions, and the last one is 1000px tall.

Just want to mention that this PR might allow this open graph usage. Thank you.

cc https://github.com/remix-run/remix/discussions/4260

felquis avatar Sep 22 '22 13:09 felquis

⚠️ No Changeset found

Latest commit: 87594af3f1f6f0ef06c1a712b67f5ef1657b5137

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

changeset-bot[bot] avatar Sep 22 '22 19:09 changeset-bot[bot]

@machour I've rebased onto latest dev

@felquis I added support for duplicate property names. It dedupes by property+content. If you need to override dupes, you can provide a unique key for each.

https://github.com/remix-run/remix/blob/87594af3f1f6f0ef06c1a712b67f5ef1657b5137/integration/meta-test.ts#L146-L159

https://github.com/remix-run/remix/blob/87594af3f1f6f0ef06c1a712b67f5ef1657b5137/integration/meta-test.ts#L253-L260

kiliman avatar Sep 22 '22 19:09 kiliman

It dedupes by property+content. If you need to override dupes, you can provide a unique key for each.

@kiliman This seems like it could be error-prone across nested route boundaries. In the vast majority of cases you want your child route's meta to override the parent's. For this to work would users need to provide keys for every tag, or just tags that support duplicates for array values?

I'm starting to wonder if we shouldn't hold off on this until v2. We're already planning some changes that incorporate this change, but if we continue auto-merging meta in nested routes I think array values introduce some potential gnarly edge cases that will be hard to work around.

chaance avatar Oct 28 '22 22:10 chaance

Just want to mention that this PR might allow this open graph usage. Thank you.

@felquis Just so you know, you can already do this today with the existing API. You just need to pass an array to the tag's content.

export function meta() {
  return {
    "og:image": [
      "https://picsum.photos/300/300",
      "https://picsum.photos/200/200",
    ],
  };
}

It does get a little weird with structured properties though, since we'd need to reorder those according to the protocol. Array values would help there, but it could get weird with nesting.

chaance avatar Oct 28 '22 23:10 chaance

If we're going to support having duplicate items (e.g. multiple og:image entries), it's going to be difficult to deal with child route merges. How do you know if these entries should be added to existing or overwritten? With single keys, it was easy, since it was always replace.

export function meta() {
  return {
    "og:image": [
      "https://picsum.photos/300/300",
      "https://picsum.photos/200/200",
    ],
  };
}

The problem with this is that you can have multiple og:image:height and it refers to the previous og:image. Refer to my example. These sub props can be interleaved and are position dependent, So even allowing arrays for content woudln't support the (very complex and poorly designed) OpenGraph spec.

So I would either recommend adding keys to allow child routes to override, or add an explicit mode: "append" | "override" prop as needed.

kiliman avatar Oct 31 '22 18:10 kiliman

For all interested, we have an RFC that will address a lot of the issues mentioned in this thread. https://github.com/remix-run/remix/discussions/4462

chaance avatar Oct 31 '22 21:10 chaance

Closed in favor of #4610

kiliman avatar Nov 17 '22 18:11 kiliman