-
Notifications
You must be signed in to change notification settings - Fork 176
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
Added keys to tags in <Head> component #417
Added keys to tags in <Head> component #417
Conversation
Next.JS <Head> component is able to extend and replace meta information predefined in a higher component with information defined in child-components. Adding keys to the meta-, title- and link-tags prevents duplicate tag-rendering when overwriting information already set by a higher component. Added keys as best-practice.
@PierreRe is attempting to deploy a commit to the Chapter Three Team on Vercel. A member of the Team first needs to authorize it. |
From my understanding Next.js's Can you explain why this change is needed? And the steps to reproduce? |
Correct, the Example
Using the Umami-Demo, Reproduce
Granted, steps 3 and 4 represent a worst-case scenario, however, step should be quite common. MotivationTo give users a guideline on how to (correctly) set and overwrite |
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.
I'm in the process of upgrading the codebase to Next.js v14 and the app router. However, we will create a new pages_starter
that will still use the pages
routing; I'll make sure it uses key
on meta tags.
Migrating all the examples will take some time, so it will be useful in the interim to have keys in the examples.
Note: this PR fixes #416
@@ -8,7 +8,7 @@ export default function IndexPage() { | |||
return ( | |||
<> | |||
<Head> | |||
<title>Next.js for Drupal | Authentication Example</title> | |||
<title key="head_title">Next.js for Drupal | Authentication Example</title> |
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.
The Next.js docs at https://nextjs.org/docs/pages/api-reference/components/head mention that key should be added so that:
To avoid duplicate tags in your head you can use the key property, which will make sure the tag is only rendered once, as in the following example:
It then shows an example of a meta tag not being duplicated. I think "meta tag" is what they meant by "tag" rather than "html tag".
If Next.js is adding duplicate title tags to the markup, then that's a Next.js bug. I don't think adding key="head_title"
to our codebase makes any sense.
@@ -14,6 +14,7 @@ export function Meta({ title, tags }: MetaProps) { | |||
return ( | |||
<Head> | |||
<link | |||
key="head_link_canonical" |
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.
Let's not prefix our key's with head_
or even with link_
or meta_
. It seems overly verbose. If React is trying to dedupe a link
element, it doesn't need the extra specificity of a prefix that duplicates the HTML structure.
tldr; let's use key="canonical"
here.
<meta | ||
key="head_meta_description" |
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.
key="description"
please.
Cheers @JohnAlbin |
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.
I don't think <title key="title">
is necessary.
@@ -8,7 +8,7 @@ export default function IndexPage() { | |||
return ( | |||
<> | |||
<Head> | |||
<title>Next.js for Drupal | Authentication Example</title> | |||
<title key="title">Next.js for Drupal | Authentication Example</title> |
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.
This code example is from the docs on Next.js that mention keys:
To avoid duplicate tags in your head you can use the key property, which will make sure the tag is only rendered once, as in the following example:
function IndexPage() { return ( <div> <Head> <title>My page title</title> <meta property="og:title" content="My page title" key="title" /> </Head> <Head> <meta property="og:title" content="My new title" key="title" /> </Head> <p>Hello world!</p> </div> ) }
You can see that don't put a key
on the title tag.
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.
Agreed, but not for the docs, but for the function behind deduplication of the <title>
Tag in the actual Head component here: https://github.com/vercel/next.js/blob/c5b5b1e3a3d85d498770a974c250c864e8d2a761/packages/next/src/shared/lib/head.tsx#L84
As per the docs, we could argue that only a single <title>
is declared and deduplication is therefore not strictly necessarry.
However, the actual NextJS <Head>
component handles the <title>
and <base>
tag differently from the <meta>
tag. I guess, my initial problem originates in line 106 of the head component which sets the isUnique
variable to false if a meta-property is not "name" or has no key.
https://github.com/vercel/next.js/blob/c5b5b1e3a3d85d498770a974c250c864e8d2a761/packages/next/src/shared/lib/head.tsx#L106
It was an interesting dive into the head component,
Cheers
- Refactored the page title in several examples to follow a consistent format: "Next.js for Drupal | [Example Name]" - Removed unnecessary key attribute from the title element
Added keys to to tags in Next.JS component to prevent duplication in rendered HTML when rewriting meta-tags in child components as best-practice approach.