Refactor SEO by cartogram · Pull Request #370 · Shopify/hydrogen

cartogram · 2023-01-18T20:03:22Z

This PR makes a number of improvements on our SEO abstraction by making the core module agnostic to any framework or implementation in Remix. This is done by exporting a single inferStorefrontSeo function that takes a config and returns an array of object representations for each head tag that should be added for a SEO-optimised storefront.

For example:

// given:
const output = inferStorefrontSeo({title: 'Some shop title'});

console.log(ouput)
// would produce:
// [
//   {
//     "children": "Snowdevil",
//     "key": "title",
//     "props": {},
//     "tag": "title",
//   },
//   {
//     "key": "meta-og:title",
//     "props": {
//       "content": "Snowdevil",
//       "property": "og:title",
//     },
//     "tag": "meta",
//   },
//   {
//     "key": "meta-twitter:title",
//     "props": {
//       "content": "Snowdevil",
//       "name": "twitter:title",
//     },
//     "tag": "meta",
//   },
//   ...etc

Why this approach

A main blocker for implementing SEO in h2 was working within the confines of how Remix manages head tags inside their page module API. Specifying the individual head tags is easy in Remix at first, but quickly becomes tedious when trying to articulate tags of various types (link, meta, script etc...) all at one; which is necessary for "Good" SEO.

Going one layer up and providing a utility that is only responsible for inferring an intermediate representation as shown above, provides users the flexibility to take this into their applications however suits them best. At first, users will be responsible for a thin wrapper around this module that adds the actual JSX or HTML markup to the head of their page (as I have done in the demo-store with an Seo component), though we may one day decide to provide "adapters" for Remix and other frameworks that do this. I think it is okay that this logic lives in user-land for now.

It is easy to take this further and build tooling (ie: a devtool/debugger) to help developers better understand the resulting head tags, and because this is now framework agnostic, I suggest we put this in the Storefront SDK rather than hydrogen itself. This would leave only the example implementation in the demo-store, but the remainder of the code would go in that repo if folks agree with me on this.

Other notes

Tests

I've used snapshots tests to quickly scaffold out tests for all the various config options. I normally am not a fan of snapshot testing, but this allowed me to move fast with a good initial group of tests. We may want to move away from this pattern and write more specific and deliberate assertions, but this is an easy thing to do later on.

Typescript

The types can be made much better here, and is something I plan to iterate on a lot. In the meantime, I am open to feedback and tips as reviewers look through the code.

Robots

I've completely removed the robots configuration from this library. At the moment, I think it doesn't really add anything to paper over the robots meta tags and users can add those separately using Remix meta or whatever. We can change our mind on this later if it becomes an issue. I think less is more with this stuff at first.

Next steps

Docs – I've documented the types pretty well, but will supplement all of this with guides shortly.
More detailed example demo-store – I'm going to add all of the necessary SEO fields to the demo-store and showcase the relationship between this library and the SFAPI. This will help further prove out this abstraction.

github-actions · 2023-01-19T14:20:14Z

We detected some changes in packages/*/package.json or packages/*/src, and there are no updates in the .changeset.
If the changes are user-facing and should cause a version bump, run npm run changeset add to track your changes and include them in the next release CHANGELOG.
If you are making simple updates to examples or documentation, you do not need to add a changeset.

cartogram · 2023-01-19T14:29:30Z


 export {storefrontRedirect} from './routing/redirect';
 export {graphiqlLoader} from './routing/graphiql';
+export {inferStorefrontSeo} from './seo/seo';


Note, this will probably move to storefront kit

Honestly, I'm not in love with the name 🤔. We should bikeshed as a team.

blittle

Great work @cartogram! Just a few comments!

blittle · 2023-01-19T15:27:09Z

-    "@types/recursive-readdir": "^2.2.1"
+    "@types/recursive-readdir": "^2.2.1",
+    "@shopify/hydrogen-react": "*",
+    "vitest": "^0.27.2",


Any issues setting up vitest without vite?

None 🙂 and if we pull this part into the SDK, it should all be set up already.

blittle · 2023-01-19T15:27:46Z


 export {storefrontRedirect} from './routing/redirect';
 export {graphiqlLoader} from './routing/graphiql';
+export {inferStorefrontSeo} from './seo/seo';


Honestly, I'm not in love with the name 🤔. We should bikeshed as a team.

blittle · 2023-01-19T15:31:13Z

+    it('should fill the description', () => {
+      // Given
+      const input = {
+        description: 'A headless storefront',


Curious if there is any auto truncating of the description? Should there be? Or maybe a warning if it exceeds recommended max length?

blittle · 2023-01-19T15:33:46Z

+              "name": "twitter:card",
+            },
+            "tag": "meta",
+          },


Should there also be a meta name="twitter:image" element?

From what I understand, twitter:image will read from og:image so I've omitted it here. If you think it is better to be explicit I can add it.

I don't know. If twitter reads the other, better to keep less meta tags than more ;)

Ah I had the same question.

blittle · 2023-01-19T15:48:44Z

+  const headTags = inferStorefrontSeo(seoConfig);
+
+  /* eslint-disable react/no-children-prop */
+  const html = headTags.map((tag) => {


Nit picky, but I wonder if it instead could be just:

return React.createElement(tag, {key: tag.key, ...tag.props}, tag.children);

chaance

No opinion on the function name, but the config API looks solid to me 👍

frehner · 2023-01-19T23:31:13Z

+          {
+            "key": "meta-twitter:card",
+            "props": {
+              "content": "summary_large_image",


Interesting, so the default choice is always the larger image rather than the summary card? Is there any way to change that if wanted?

frehner · 2023-01-19T23:35:34Z

+            "tag": "title",
+          },
+          {
+            "key": "meta-og:title",


Damn facebook trying to rebrand as meta and getting me confused on whether this is for "meta the company" or a "meta tag".

frehner · 2023-01-19T23:45:02Z

+          {
+            "key": "link-canonical",
+            "props": {
+              "href": "https://hydrogen.shop/collections",


Is there any extra processing done for the canonical URL? Stripping query strings, or checking to ensure that it has https://, etc.?

Right, we might want to "normalize" the URL before building this tag.

frehner · 2023-01-19T23:46:29Z

+          {
+            "key": "script-application/ld+json",
+            "props": {
+              "children": "{\\"@context\\":\\"https://schema.org\\",\\"@type\\":\\"ItemList\\",\\"url\\":\\"https://hydrogen.shop/collections\\"}",


Is this using ItemList because your function was smart enough to know it was a collection URL? Or is that the default?

[edit] I see below that it's smart enough. Nice.

frehner · 2023-01-19T23:49:29Z

+              "name": "twitter:card",
+            },
+            "tag": "meta",
+          },


Ah I had the same question.

frehner · 2023-01-19T23:50:22Z

+          {
+            "key": "script-application/ld+json",
+            "props": {
+              "children": "{\\"@context\\":\\"https://schema.org\\",\\"@type\\":\\"Thing\\",\\"image\\":\\"https://example.com/image-2.jpg\\"}",


Is there any way to specify which image should be the default one for the LDJSON?

There isn't currently.

frehner · 2023-01-19T23:53:52Z

+          {
+            "key": "script-application/ld+json",
+            "props": {
+              "children": "{\\"@context\\":\\"https://schema.org\\",\\"@type\\":\\"Thing\\"}",


MediaObject maybe? https://schema.org/MediaObject

frehner · 2023-01-19T23:59:04Z

+      type: 'Organization',
+      pattern: /\/policies\/([^\/]+)/,
+    },
+  ];


I like this, but also, this falls into the thinking that there are standardized URLs and that custom storefronts will follow them, no? Which is kind of the opposite of custom storefronts and being able to define your own URL structure.

What if we have a localized url? What if they use /prods for products? What if they use /product instead of /products? etc.

I think we try and assume defaults and they can override the value if they want. If this becomes an issue we can try and build on this inference or remove it all together.

github-actions Bot had a problem deploying to preview January 18, 2023 20:04 Failure

github-actions Bot had a problem deploying to preview January 19, 2023 04:11 Failure

cartogram force-pushed the seo-take-1million branch from 9ffbd80 to 925a909 Compare January 19, 2023 04:39

github-actions Bot deployed to preview January 19, 2023 04:41 View deployment

Matt Seccafien added 4 commits January 19, 2023 05:52

Refactor SEO

332470f

Fix build

9010b9c

Add additional comments and support for custom ldjson

785c852

rename everything again

2a65420

cartogram force-pushed the seo-take-1million branch from 925a909 to 2a65420 Compare January 19, 2023 04:53

github-actions Bot had a problem deploying to preview January 19, 2023 04:54 Failure

Fix turbo integrity kets

7f9f14a

github-actions Bot deployed to preview January 19, 2023 13:41 View deployment

Matt Seccafien added 2 commits January 19, 2023 14:45

remove async/await from tests

6b4570c

Delete seo.test.ts

f8d12f9

cartogram marked this pull request as ready for review January 19, 2023 14:20

cartogram requested a review from a team January 19, 2023 14:20

github-actions Bot deployed to preview January 19, 2023 14:21 View deployment

Move schema dts to dev dependency

1047897

cartogram commented Jan 19, 2023

View reviewed changes

github-actions Bot deployed to preview January 19, 2023 14:30 View deployment

Update types.ts

87b7adb

github-actions Bot deployed to preview January 19, 2023 14:36 View deployment

cartogram requested a review from chaance January 19, 2023 14:44

blittle approved these changes Jan 19, 2023

View reviewed changes

chaance approved these changes Jan 19, 2023

View reviewed changes

frehner reviewed Jan 20, 2023

View reviewed changes

cartogram merged commit 1666232 into v0.x-2022-10 Jan 20, 2023

cartogram deleted the seo-take-1million branch January 20, 2023 15:45

cartogram mentioned this pull request Jan 25, 2023

Add generateSeoTags utility Shopify/hydrogen-react#137

Open

4 tasks

Conversation

cartogram commented Jan 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why this approach

Other notes

Tests

Typescript

Robots

Next steps

Uh oh!

github-actions Bot commented Jan 19, 2023

Uh oh!

cartogram Jan 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

blittle left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chaance left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

cartogram commented Jan 18, 2023 •

edited

Loading

cartogram Jan 19, 2023 •

edited

Loading