next.js icon indicating copy to clipboard operation
next.js copied to clipboard

next/head deduping process can duplicate and overwrite dynamically inserted header tags

Open DevelopersTopHat opened this issue 4 years ago • 13 comments

What example does this report relate to?

Next/Head

What version of Next.js are you using?

^11.1.0

What version of Node.js are you using?

^15.6.1

What browser are you using?

Chrome

What operating system are you using?

Windows

How are you deploying your application?

AWS EC2

Describe the Bug

If you have integrations which dynamically add elements to the html document head, if the item added falls into the "old tags" range for the Next/Head implementation, any tags added dynamically will be duplicated because they no longer fall within the "old tags" range. Additionally, if you then have a re-render which causes the Next/Head to run again, the dynamically inserted elements from the integration will then be deleted.

The issue happens here (credit @tb-campbell) https://github.com/vercel/next.js/blob/f52211bad39c2a29a7caba40bfeb89b600524b41/packages/next/client/head-manager.ts#L73

Expected Behavior

The Next/Head component should manage the state of old and new tags internally and not count backward from an element inside the document head. Any elements that dynamically get inserted into the document head should be unaffected by the dynamic insertion by the Next/Head component, and the Next/Head component should manage the new and old Head tags without relying on the actual DOM.

To Reproduce

We are using google-tag-manager which has some 3rd party integrations which insert their scripts into the head of the document. This would also be applicable for a lot of ad frameworks and trackers as well. To recreate it without these, you could try dynamically injecting meta/link/script/etc tags into the head, until one overlaps in position with the tags set by the next head.

Below would be an example of a Head component usage for a page, where some of the fields are generated client side during runtime. If a script were to inject into the document head at any line where the tags below exist, they would first be duplicated, and on subsequent render be deleted.

<Head>
      <title key="title">{titleText}</title>
      <link key="canonical" rel="canonical" href={canonicalUrl} />
      <meta key="description" name="description" content={descriptionText} />
      <meta key="og:title" property="og:title" content={titleText} />
      <meta key="og:description" property="og:description" content={descriptionText} />
      <meta key="og:url" property="og:type" content="website" />
      <meta key="og:type" property="og:url" content={canonicalUrl} />
      <meta key="og:details" property="og:details" content={details} />
      <meta
        key="place:location:latitude"
        property="place:location:latitude"
        content={searchLocation?.position?.lat?.toString()}
      />
      <meta
        key="place:location:longitude"
        property="place:location:longitude"
        content={searchLocation?.position?.lng?.toString()}
      />
      <script
        key="breadcrumb-schema"
        type="application/ld+json"
        dangerouslySetInnerHTML={{ __html: sanitize(JSON.stringify(breadcrumbs)) }}
      />
</Head>

DevelopersTopHat avatar Nov 19 '21 22:11 DevelopersTopHat

Hey I think I'm running into the exact same issue when using react-facebook-pixel. I extracted the core parts into this minimal example: https://github.com/tills13/next-head-dupe-example

clone the repo, run npm ci, run npx next dev, view localhost:3000, inspect elements

You should see two viewport tags: image

tills13 avatar Nov 20 '21 00:11 tills13

As a "fix", depending on how early in the lifecycle you need your trackers to run, moving them into an effect runs them late enough that Next has already reconciled the head. Here's what that would look like: https://github.com/tills13/next-head-dupe-example/compare/fixed-example

Though I agree that Next should be more explicit about how it manages its head elements. We unfortunately run a lot of 3rd party scripts on our sites and there's no telling what they'll do or when they'll do it so the above only helps us in this very specific case.

tills13 avatar Nov 20 '21 00:11 tills13

That was one of the options I was considering, but unfortunately, we have page events that can cause re-renders that update our dynamic SEO tags, and thus the tracking scripts could get deleted at any time when the Next/Head component does its deduping deletion of old tags.

Currently exploring other libraries like next-seo to see if they handle this issue better, otherwise, we will have to build it custom.

DevelopersTopHat avatar Nov 20 '21 23:11 DevelopersTopHat

Follow-up, in case others run into this issue and need a workaround due to the nature of their project, I've started using https://www.npmjs.com/package/react-helmet-async as the alternative to the next/head component for our app. So far the results are promising, and the migration to using it was as simple as updating all of the next/head usages to use the <Helmet> wrapper instead.

I haven't quite figured out how to get it working correctly on the server side yet (and there is very limited documentation on this topic), but I haven't run into any client-side issues with it yet. It adds 6.1Kb in size to our project, which isn't ideal, but it's not a deal-breaker as it resolves the deduping issue with next/head.

DevelopersTopHat avatar Nov 27 '21 18:11 DevelopersTopHat

Have you followed the directions here?

Let me know how it goes, please -- we are also interested using this over next/head.

tills13 avatar Nov 27 '21 19:11 tills13

Yes, unfortunately those docs are a couple years old and too outdated for the version of Next JS we use. I've found that if the property defer={false} is added to the <Helmet defer={false}> our generic app meta data seems to be used successfully server side, however it causes a brief title flicker when the page level meta data is loading in. Trying to figure out how the HelmetProvider needs to be setup to use the correct page level meta data on SSR.

DevelopersTopHat avatar Nov 27 '21 20:11 DevelopersTopHat

Ended up not being able to use react-helmet-async since I wasn't able to get it to work on SSR with Next.js. This thread actually duplicates a bunch of known existing issues, but the plan at the moment seems like there won't be any fixes for the next/head component until they rework it after the release of React 18. What I ended up doing (and so far I haven't run into any issues although take that with a grain of salt since that implementation didn't actually make it into Nextjs) was to patch the next/head component so that it does a qualifying check before running the deduping process.

I basically took @stefvhuynh 's pr (https://github.com/vercel/next.js/pull/16707) and patched Nextjs with it using patch-package to reduce the risk of the next/head deleting or removing the dynamically injected scripts.

There were some concerns about using data attributes and crawlers not being able to see them, but this use case is just to aid with the deduping process, so it shouldn't cause any problems.

Here is the patch I made (still testing, but so far it has been working like a charm). I'm not sure if it will work the same way if the file is created manually, but it looks something like this: patches/next+11.1.0.patch

Updated as of 12/19/2021. The original did have an issue where a hard refresh could cause an infinite loop, so I added a failsafe. Has been running for about a month, and no issues reported on the patch. This is a temporary workaround for this issue as well #11012


diff --git a/node_modules/next/dist/client/head-manager.js b/node_modules/next/dist/client/head-manager.js
index 859e93c..02a0da8 100644
--- a/node_modules/next/dist/client/head-manager.js
+++ b/node_modules/next/dist/client/head-manager.js
@@ -45,10 +45,18 @@ function updateElements(type, components) {
     }
     const headCount = Number(headCountEl.content);
     const oldTags = [];
-    for(let i = 0, j = headCountEl.previousElementSibling; i < headCount; i++, j = j.previousElementSibling){
-        if (j.tagName.toLowerCase() === type) {
-            oldTags.push(j);
+    let loop = 0;
+    let i = 0
+    let j = headCountEl.previousElementSibling
+    while (i < headCount && loop < headEl.childElementCount) {
+      if (j?.getAttribute('data-next-head')) {
+        if (j?.tagName.toLowerCase() === type) {
+          oldTags.push(j)
         }
+        i++
+      }
+      j = j?.previousElementSibling
+      loop++;
     }
     const newTags = components.map(reactElementToDOM).filter((newTag)=>{
         for(let k = 0, len = oldTags.length; k < len; k++){
diff --git a/node_modules/next/dist/pages/_document.js b/node_modules/next/dist/pages/_document.js
index b648269..cc90a26 100644
--- a/node_modules/next/dist/pages/_document.js
+++ b/node_modules/next/dist/pages/_document.js
@@ -318,6 +318,7 @@ class Head extends _react.Component {
                 };
                 newProps['data-href'] = newProps['href'];
                 newProps['href'] = undefined;
+                newProps['data-next-head'] = "true";
                 return(/*#__PURE__*/ _react.default.cloneElement(c, newProps));
             } else if (c.props && c.props['children']) {
                 c.props['children'] = this.makeStylesheetInert(c.props['children']);
diff --git a/node_modules/next/dist/shared/lib/head.js b/node_modules/next/dist/shared/lib/head.js
index 84af91a..c35a4cf 100644
--- a/node_modules/next/dist/shared/lib/head.js
+++ b/node_modules/next/dist/shared/lib/head.js
@@ -40,13 +40,15 @@ function _interopRequireWildcard(obj) {
 function defaultHead(inAmpMode = false) {
     const head = [
         /*#__PURE__*/ _react.default.createElement("meta", {
-            charSet: "utf-8"
+            charSet: "utf-8",
+            "data-next-head": "true"
         })
     ];
     if (!inAmpMode) {
         head.push(/*#__PURE__*/ _react.default.createElement("meta", {
             name: "viewport",
-            content: "width=device-width"
+            content: "width=device-width",
+            "data-next-head": "true"
         }));
     }
     return head;
@@ -155,11 +157,13 @@ const METATYPES = [
                 newProps['href'] = undefined;
                 // Add this attribute to make it easy to identify optimized tags
                 newProps['data-optimized-fonts'] = true;
+                newProps['data-next-head'] = "true";
                 return(/*#__PURE__*/ _react.default.cloneElement(c, newProps));
             }
         }
         return(/*#__PURE__*/ _react.default.cloneElement(c, {
-            key
+            key,
+            "data-next-head": "true"
         }));
     });
 }

and then my package.json is configured like so:


"scripts": {
...
"postinstall": "patch-package"
}
"dependencies": {
...
    "patch-package": "^6.4.7",
    "postinstall-postinstall": "^2.1.0",
}

DevelopersTopHat avatar Nov 29 '21 22:11 DevelopersTopHat

I can confirm this bug - we are facing duplicated meta description (and some other) tags & we also have quite a lot "necessary" external scripts (which seems to be causing unwanted interference with nextjs code as described above).

czekFREE avatar Dec 02 '21 13:12 czekFREE

In my case, viewport meta tag is duplicated. I have custom _document.tsx and _app.tsx. Viewport tag alongside with all other Head tags is placed inside _app.tsx. _document.tsx only contains Head component with Google font imports.

shehi avatar Feb 20 '22 20:02 shehi

Has there been any movement on this bug? @DevelopersTopHat's patch worked great but I'd like to update to the latest version of Next to get access to the middleware features

mgoebs52 avatar Oct 18 '22 14:10 mgoebs52

I've been experiencing almost my entire

tag being duped when using next/head and trying to run some necessary third party scripts synchronously per page. ([email protected]) Using unique key or id attributes did not solve. next/head has become unusable for us now and we've adopted react-helmet-async

spreadpando avatar Oct 22 '22 20:10 spreadpando

not sure if anyone(@spreadpando ?) can provide a reproduce minimal repo for this bug, since I cannot reproduce it by using https://github.com/tills13/next-head-dupe-example from @tills13 with latest version of next(12.3.2-canary.35) & react(18.2.0).

I cannot get duplicated viewpoint meta tag from it, and this is my result:

<head>
    <meta charset="utf-8">
    <meta name="viewport" content="width=device-width">
    <script src="https://cdnjs.cloudflare.com/ajax/libs/react/18.0.0-beta-149b420f6-20211119/umd/react.production.min.js"></script>
    <meta charset="utf-8">
    <meta name="next-head-count" content="3">
    <noscript data-n-css=""></noscript>
    <script defer="" nomodule="" src="/_next/static/chunks/polyfills.js?ts=1666578004120"></script>
    <script src="/_next/static/chunks/webpack.js?ts=1666578004120" defer=""></script>
    <script src="/_next/static/chunks/main.js?ts=1666578004120" defer=""></script>
    <script src="/_next/static/chunks/pages/_app.js?ts=1666578004120" defer=""></script>
    <script src="/_next/static/chunks/pages/index.js?ts=1666578004120" defer=""></script>
    <script src="/_next/static/development/_buildManifest.js?ts=1666578004120" defer=""></script>
    <script src="/_next/static/development/_ssgManifest.js?ts=1666578004120" defer=""></script>
    <noscript id="__next_css__DO_NOT_USE__"></noscript>
</head>

teobler avatar Oct 24 '22 02:10 teobler

Here's a minimal example.

{
//...
 "next": "13.0.2",
 "react": "18.2.0",
 "react-dom": "18.2.0",
//...
}
import Head from "next/head";
import { useState } from "react";

const attachCss = () => {
  const charsetTag = document.head.querySelector("meta[charset]");
  const style = document.createElement("style");
  style.appendChild(document.createTextNode(".hello {color: red;}"));
  if (charsetTag) {
    document.head.insertBefore(style, charsetTag.nextSibling);
  }
};

export default function Home() {
  const [renderHead, setRenderHead] = useState(false);
  return (
    <div>
      <Head>
        <title>{renderHead ? 'Page Title B' : 'Page Title A'}</title>
      </Head>

      <p>Open the dev console and observe the DOM.</p>
      <ol>
        <li>
          <button onClick={attachCss}>Add Style-Element to head</button>
          <br />A style Element should be added to just below the charset.
        </li>
        <li>
          <button onClick={() => setRenderHead((prev) => !prev)}>
            Trigger rerender
          </button>
          <br />
          Title should be updated. The Style-Element is gone, but an additional
          charset Meta-Element popped up.
        </li>

        <li>
          <button onClick={attachCss}>Add Style-Element again</button>
          <br />A style Element should be added to just below the charset.
        </li>

        <li>
          <button onClick={() => setRenderHead((prev) => !prev)}>
            Trigger rerender
          </button>
          <br />
          Title should be updated. The Style-Element is still there.
        </li>
      </ol>

      <p>
        Interesting. The Title shouldn't be updated, since the next/head element
        is there twice.
      </p>
    </div>
  );
}

Purii avatar Nov 10 '22 13:11 Purii

I've been experiencing almost my entire tag being duped when using next/head and trying to run some necessary third party scripts synchronously per page. ([email protected]) Using unique key or id attributes did not solve. next/head has become unusable for us now and we've adopted react-helmet-async

we tried react-helmet-async but still encounter the duplication issue (prob because we're still using next-seo)

ivanwidj avatar Oct 17 '23 18:10 ivanwidj