-
Notifications
You must be signed in to change notification settings - Fork 10.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(gatsby): Reorder head tags #34030
fix(gatsby): Reorder head tags #34030
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🙏🥳
Actually why don't we just do this in the framework itself? |
Fine by me. Probably a better 'batteries included' experience than needing this plugin configured for that best practice. As long as the plugin wouldn't undo the framework's sorting. |
Yeah we'd do the sorting after all plugins have run. This is in the oddly named static-entry.js files |
b757f66
@KyleAMathews I moved the same sorting logic to right before head components are sent to static HTML in static-entry. One thing I am not sure of is whether this will interfere with this line for determining whether to inline styles. |
I still need to make a contrived example with lots of head tags to compare the sorting order. |
I looked at the diff, what does it do to tags that are not listed in this?
For example, if you have According to MDN, allowed tags inside
Notable that I don't know what tags are disallowed, MDN doesn't explicitly say that, so people might be throwing other tags there too. IMO, re-ordering all tags in the head can have odd consequences, like having template before or after a script tag might make site broken. It would be better if it could be done in order of introduction. Move just meta tags above everything else, and leave the order as is, it would probably break less stuff than ordering all tags. |
@Ciantic Yea, that is a good point about tags not listed there. Technically there is already some opinionated ordering done by when certain tags are pushed to |
Possible other solution in
|
The inline styles are already added by the time you sort right? We should definitely sort rather than rewrite. |
Yea, the inline styles are added via |
In my first bit of testing on this with the sort, the ordering looks correct during SSR, but then the tags are back out of order (meta tags from helmet specifically) in DevTools. Is there something updating the head components during/after hydration? |
So if you curl the page the tags are correct? Once on the client, tags are added by react-helmet in the order of the code so they'll be back to that. But to fix the issue with crawlers we only need to worry about the SSRed html. |
I didn't curl exactly, but yea the first network request with the SSR'd HTML looks correct. Cool, that makes sense about the crawlers! I'm gonna try to repro the actual |
URL with this fork patched in shows the correct tags on the initial request now: go here: do this: see this: I tried testing with the Twitter Card Validator, but I think it may be using a cached version (the image is missing) since I had tested before without the patch. The Facebook sharing tester is showing the correct data. |
Will it be backported/be part of Gatsby 3.x? |
Similar question to @m99coder, will this be backported to Gatsby 2.x ? we have some sites still running on Gatsby 2.19 |
It'll only be backported to v3 |
* group head tags and reorder and spread * move tag ordering to static-entry * change reduce to sort * comment purpose of headComponents sort (cherry picked from commit 10c8227)
Thats a bummer, we have sites which we dont want to upgrade to v3 and would be nice if it could be backported to v2 |
I have just tested v4.5 and I noticed that the meta tags show up on top only on initial page load. Once I click on a link it gets reordered again and certain meta tags are under the styles once again. |
@t2ca crawlers only grab the HTML so you should be fine |
Hi @graysonhicks and @KyleAMathews, I updated my Gatsby dependency to 4.5.2 and deployed the new version. Unfortunately, I still see a suboptimal order of tags in the
I might make something wrong that Source: https://woodist.de/produkt/anhaenger-wolke/ If needed, I can also provide the dependency list, that I use or some other details for debugging. |
@m99coder When I visit that site and look at the network tab, I see a different set of head tags than you've posted above. The meta tags are totally missing when I look, but otherwise the order is correct. How are they being added? The network tab is what a crawler would see. |
The meta tags are added using import { Helmet } from "react-helmet"
const Layout = () => {
return (
<Helmet
title={seo.title}
titleTemplate={seo.title === defaultTitle ? seo.title : titleTemplate}
>
<meta name="description" content={seo.description} />
<meta name="keywords" content={seo.metatags.join(",")} />
<meta name="image" content={seo.image} />
{/* DNS prefetch */}
<link rel="dns-prefetch" href="//images.ctfassets.net/" />
<link rel="dns-prefetch" href="//cdn.shopify.com/" />
<link rel="dns-prefetch" href="//m99.io/" />
{/* Open Graph, see https://ogp.me/ */}
{seo.url && <meta property="og:url" content={seo.url} />}
{seo.title && <meta property="og:title" content={seo.title} />}
{seo.description && (
<meta property="og:description" content={seo.description} />
)}
{seo.image && <meta property="og:image" content={seo.image} />}
{/* Twitter, see https://developer.twitter.com/en/docs/twitter-for-websites/cards/guides/getting-started */}
<meta name="twitter:card" content="summary_large_image" />
{seo.image && <meta name="twitter:image" content={seo.image} />}
</Helmet>
)
} Check the network tab and I also don’t see them. So maybe using Is it maybe only working when I define them as attributes of the |
@m99coder Yes, would be interesting to see if there is a difference between the Helmet attributes vs children. |
@graysonhicks Sadly doesn’t change the result. Even https://github.com/nfl/react-helmet shows that defining the tags as children should work. Now I’m a bit clueless 😕 May it be related to how the SSR works? On the other side I don’t think my setup is anyhow unique, so wonder why it doesn’t happen for others as well. I will take some time to debug and try to build up my setup from scratch to see at which point it stops working. |
Same issues here @graysonhicks. Twitter validator & Facebook Sharing Debugger cannot crawl the meta tags after upgrading to v4.5.2 even tags order shifted already. My SEO components was pretty much copied from another site coming from v3 which is totally working right now. What I discovered is that the crawler completed even before content from the My testing link: Just for reference, my SEO component, placed in
My info:
|
Okay, thanks. This seems like a separate issue. The original issue was tags were out of order (no matter how they were added). This seems like react-helmet isn't getting SSR'd. There shouldn't be a 'too late' because the static markup should have the correct tags before hydration. Can you open a new issue? |
Sorry, I created a reproduction repo and realised |
@desktopofsamuel @graysonhicks I checked the Facebook Developer Echo result you shared, and in my case the import ReactDOM from "react-dom"
import { wrapPageElement as wrapper } from "./src/utils/wrap-page-element"
export const wrapPageElement = wrapper
// this is a hack to fix missing styles on refresh in production
// see: https://github.com/gatsbyjs/gatsby/issues/8560#issuecomment-535265414
// and: https://github.com/gatsbyjs/gatsby/discussions/17914
export function replaceHydrateFunction() {
return (element, container, callback) => {
ReactDOM.render(element, container, callback)
}
} As soon as I find time, I will strip down my setup to the bare minimum and see at which point the functionality is broken. Chances are high that I added some custom logic that killed the expected behaviour 😅 |
Created a bare, unmodified repository like this:
You can find the respective code here: https://github.com/m99coder/gatsby-open-graph-metatags. Now when I run <!DOCTYPE html>
<html>
<head>
<meta charSet="utf-8" />
<meta http-equiv="x-ua-compatible" content="ie=edge" />
<meta name="viewport" content="width=device-width, initial-scale=1, shrink-to-fit=no" />
<meta name="note" content="environment=development" />
<title data-react-helmet="true"></title>
<style>
.gatsby-image-wrapper {
position: relative;
overflow: hidden
}
.gatsby-image-wrapper picture.object-fit-polyfill {
position: static !important
}
.gatsby-image-wrapper img {
bottom: 0;
height: 100%;
left: 0;
margin: 0;
max-width: none;
padding: 0;
position: absolute;
right: 0;
top: 0;
width: 100%;
object-fit: cover
}
.gatsby-image-wrapper [data-main-image] {
opacity: 0;
transform: translateZ(0);
transition: opacity .25s linear;
will-change: opacity
}
.gatsby-image-wrapper-constrained {
display: inline-block;
vertical-align: top
}
</style><noscript>
<style>
.gatsby-image-wrapper noscript [data-main-image] {
opacity: 1 !important
}
.gatsby-image-wrapper [data-placeholder-image] {
opacity: 0 !important
}
</style>
</noscript>
<script
type="module">const e = "undefined" != typeof HTMLImageElement && "loading" in HTMLImageElement.prototype; e && document.body.addEventListener("load", (function (e) { if (void 0 === e.target.dataset.mainImage) return; if (void 0 === e.target.dataset.gatsbyImageSsr) return; const t = e.target; let a = null, n = t; for (; null === a && n;)void 0 !== n.parentNode.dataset.gatsbyImageWrapper && (a = n.parentNode), n = n.parentNode; const o = a.querySelector("[data-placeholder-image]"), r = new Image; r.src = t.currentSrc, r.decode().catch((() => { })).then((() => { t.style.opacity = 1, o && (o.style.opacity = 0, o.style.transition = "opacity 500ms linear") })) }), !0);</script>
<link rel="icon" href="/favicon-32x32.png?v=4a9773549091c227cd2eb82ccd9c5e3a" type="image/png" />
<link rel="manifest" href="/manifest.webmanifest" crossorigin="anonymous" />
<link rel="apple-touch-icon" sizes="48x48" href="/icons/icon-48x48.png?v=4a9773549091c227cd2eb82ccd9c5e3a" />
<link rel="apple-touch-icon" sizes="72x72" href="/icons/icon-72x72.png?v=4a9773549091c227cd2eb82ccd9c5e3a" />
<link rel="apple-touch-icon" sizes="96x96" href="/icons/icon-96x96.png?v=4a9773549091c227cd2eb82ccd9c5e3a" />
<link rel="apple-touch-icon" sizes="144x144" href="/icons/icon-144x144.png?v=4a9773549091c227cd2eb82ccd9c5e3a" />
<link rel="apple-touch-icon" sizes="192x192" href="/icons/icon-192x192.png?v=4a9773549091c227cd2eb82ccd9c5e3a" />
<link rel="apple-touch-icon" sizes="256x256" href="/icons/icon-256x256.png?v=4a9773549091c227cd2eb82ccd9c5e3a" />
<link rel="apple-touch-icon" sizes="384x384" href="/icons/icon-384x384.png?v=4a9773549091c227cd2eb82ccd9c5e3a" />
<link rel="apple-touch-icon" sizes="512x512" href="/icons/icon-512x512.png?v=4a9773549091c227cd2eb82ccd9c5e3a" />
<script src="/socket.io/socket.io.js"></script>
<link rel="stylesheet" href="/commons.css" />
</head>
<body>
<div id="___gatsby"></div>
<script src="/polyfill.js" nomodule=""></script>
<script src="/framework.js"></script>
<script src="/commons.js"></script>
</body>
</html> As you can see there are no SEO-related metatags included. <head>
<meta charSet="utf-8" />
<meta http-equiv="x-ua-compatible" content="ie=edge" />
<meta name="viewport" content="width=device-width, initial-scale=1, shrink-to-fit=no" />
<meta data-react-helmet="true" name="twitter:description"
content="Kick off your next, great Gatsby project with this default starter. This barebones starter ships with the main Gatsby configuration files you might need." />
<meta data-react-helmet="true" name="twitter:title" content="Home" />
<meta data-react-helmet="true" name="twitter:creator" content="@gatsbyjs" />
<meta data-react-helmet="true" name="twitter:card" content="summary" />
<meta data-react-helmet="true" property="og:type" content="website" />
<meta data-react-helmet="true" property="og:description"
content="Kick off your next, great Gatsby project with this default starter. This barebones starter ships with the main Gatsby configuration files you might need." />
<meta data-react-helmet="true" property="og:title" content="Home" />
<meta data-react-helmet="true" name="description"
content="Kick off your next, great Gatsby project with this default starter. This barebones starter ships with the main Gatsby configuration files you might need." />
<meta name="generator" content="Gatsby 4.5.4" />
<!-- more non-meta tags -->
</head> So there is clearly an issue somewhere in my original project. I will try to narrow it down. Update
Dependencies, plugins and features used |
@graysonhicks @KyleAMathews I might have found the issue 🤭 With my last commit m99coder/gatsby-open-graph-metatags@3f63d12 I create a dynamic page like this in exports.createPages = async ({ actions }) => {
const { createPage } = actions
createPage({
path: "/using-dsg",
component: require.resolve("./src/templates/using-dsg.js"),
context: {},
defer: true,
})
createPage({
path: "/test",
component: require.resolve("./src/templates/test.js"),
context: {},
})
} When the path is requested (with
This is in contrast to the Finally, I assume that the social share tools work similar to using $ curl -s http://localhost:9000/test | tidy 2>&1 | grep "og:"
$ curl -s http://localhost:9000/test/ | tidy 2>&1 | grep "og:"
<meta data-react-helmet="true" property="og:type" content=
<meta data-react-helmet="true" property="og:description" content=
<meta data-react-helmet="true" property="og:title" content=
$ curl -L -s http://localhost:9000/test | tidy 2>&1 | grep "og:"
<meta data-react-helmet="true" property="og:type" content=
<meta data-react-helmet="true" property="og:description" content=
<meta data-react-helmet="true" property="og:title" content= Never mind, I see that also on my production page the redirect happens, but in this case the version provided under the path with the trailing slash, doesn’t have the correct meta tag order neither 😞 |
Human error… pure human error 🚯 In my export const wrapPageElement = ({ element }) => {
// configure Sentry
Sentry.configureScope(scope => {
scope.addEventProcessor(event => {
// eslint-disable-next-line no-unused-vars
const { user, ...redacted } = event
return redacted
})
})
return (
<Sentry.ErrorBoundary fallback={"An error has occurred"}>
{isBrowser && (
<ListOptionsProvider>{element}</ListOptionsProvider>
)}
{/* the next line was missing before */}
{!isBrowser && element}
</Sentry.ErrorBoundary>
)
} I really apologize for all the hassle. This feature works as intended. |
No worries, thanks @m99coder ! |
I understand from this thread, that Gatsby should now sort the head tags automatically. Or is there something I have to specify in my code for the sorting to be enabled? I'm using Gatsby 4.10.1, gatsby-react-helmet-plugin and SEO component as defined in official Gatsby site template. My pages are created in gatsby-node.js with createPages. I'm using static site generation as rendering method. And Chrome Network tab shows that meta tags are still in random places in the head, mostly after scripts, links and styles. |
@mixdmark Could you please open a new issue with a reproduction? Should be working by default. |
@mixdmark, I just tried it with [email protected] and can confirm the production build contains meta tags correctly ordered (at the very top of the |
Description
Reorder tags to e.g. move css below meta tags as apparently larger stylesheets block the parsers
Documentation
Related Issues
Fixes #22206