Skip to content
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

Duplicate <meta name="viewport"> if the custom one is defined in _document.js #6919

Closed
kachkaev opened this issue Apr 6, 2019 · 25 comments
Closed

Comments

@kachkaev
Copy link
Contributor

kachkaev commented Apr 6, 2019

Bug report

I recently upgraded to the latest canary and noticed horizontal scrolling on some mobile devices. As it turned out, this was caused by #6754 (cc @developit).

Although the default new value of viewport is inserted only when there is no custom one, this logic did not work for me, because my custom viewport tag was in _document.js and not inside <Head />.

<meta name="viewport" content="width=device-width" />

Moving the tag into <Head /> solved the issue, but it took a while to investigate.

Expected behavior

It'd be great if Next.js searched for <meta name="viewport" content="..." /> inside custom _document.js and either did not insert the default one or asked developers to move their tag into <Head />. WDYT folks?

System information

  • Version of Next.js: 8.0.5-canary.17
@tkhg
Copy link

tkhg commented Apr 12, 2019

same issue on 8.0.4
I was trying to set viewport-fit=cover in the viewport meta tag in _document.js, but it was overridden by the default tag.
I believe it will be a better approach if the viewport meta is already set in _document.js, the default tag shouldn't be added

@smackgg
Copy link

smackgg commented Apr 12, 2019

Anybody see this question?

@lisamartin00
Copy link

Our team ran into this problem as well and ended up moving our <meta viewport...> tag from _document.js and into _app.js. In _app.js, we are importing Head from 'next/head'; which seems to remove the duplicate, whereas _document.js is importing { Head } from 'next/document';, which did not remove the duplicate.
_app.js seems like an appropriate place to put a global <meta viewport...> tag since it's not SSR related anyway.

@cesarvarela
Copy link

So the approach at the moment is to use _app instead of _document?

@lisamartin00
Copy link

@cesarvarela yes that is our approach at the moment and does seem to be working..but if anyone has any other suggestions or best practices, we'd love to hear them!

@nblthree
Copy link
Contributor

@timneutkens should I make PR to rise a warning in production mode like for the title

@cheeki
Copy link

cheeki commented Apr 25, 2019

In my case, whenever i use "key" property on <meta name="viewport" ... />, unwanted duplicated meta tag appears like below. even in _app.js

<meta name="viewport" content="width=device-width,minimum-scale=1,initial-scale=1" class="next-head">

@nblthree
Copy link
Contributor

nblthree commented Apr 25, 2019

@cheeki I made a PR to fix that but logically there is no need to use the key property on meta tags (But I am not sure, maybe there is a need because I remember that like there was a situation in which key is necessary)

@vzaidman
Copy link
Contributor

vzaidman commented Apr 28, 2019

key is needed if you have a child component rewriting the father's component "head".

_app:
<meta key="meta_description" name="description" content="general description"/>

ChildPage:
<meta key="meta_description" name="description" content="page description"/>

@nblthree
Copy link
Contributor

nblthree commented May 1, 2019

I would like to confirm the Expected behavior so that I can solve this one as soon as possible

  • If there is a viewport in _document don't add the default one
  • If there is a viewport in _document and another in a page and they don't have the same key value add both of them
  • If there is a viewport in _document and another in a page and they do have the same key value add the one in the page

@vzaidman
Copy link
Contributor

vzaidman commented May 1, 2019

I think it's correct.
but also notice viewport might be in _app as well.
possible in all of them:
in _document in _app and in page

@nblthree
Copy link
Contributor

nblthree commented May 1, 2019

If there is a viewport in _document and another in a page and they do have the same key value add the one in the page

@vzaidman what if they don't have a key value? I am considering it to be the same as the above

@nblthree
Copy link
Contributor

nblthree commented May 1, 2019

@cheeki

In my case, whenever i use "key" property on <meta name="viewport" ... />, unwanted duplicated meta tag appears like below. even in _app.js

<meta name="viewport" content="width=device-width,minimum-scale=1,initial-scale=1" class="next-head">

I think that it's the right behavior check @vzaidman #6919 (comment) so if you want to overwrite the default meta you shouldn't use the key

@vzaidman
Copy link
Contributor

vzaidman commented May 2, 2019

The expected behavior:

  • every meta component without "key" is added to the head.

  • every meta component with key overrides all the other meta components with the same key so only one is added to the head. the overrides will happen in the following order:

    1. page - most important (most specific)
    2. _app
    3. _document - least important (most general)

good to have:

  • console.warn about the possibility to add "key" in development if two meta headers with the same name exists.

@Janpot
Copy link
Contributor

Janpot commented May 4, 2019

I think I'm seeing the same issue with

<meta charSet='utf-8' />

https://github.com/mui-org/material-ui/blob/4516cafd6959fafccae96ca0bc70527d0fd5cf02/examples/nextjs-next/pages/_document.js#L12

@Timer
Copy link
Member

Timer commented Jun 10, 2019

Hi! After careful review we've decided this is the intended behavior. Head elements that are expected to be deduped should be added using next/head.
The reason this surfaced is because we started defaulting a more commonly used <meta> tag -- not that something is working unexpectedly.


The correct solution in this scenario is to move to using _app over _document.

@Timer Timer closed this as completed Jun 10, 2019
@Soundvessel
Copy link

It might help to mention this in the documentation https://nextjs.org/docs#custom-document

@Timer
Copy link
Member

Timer commented Jun 30, 2019

Feel free to send a PR, @Soundvessel!

@samilyak
Copy link

samilyak commented Aug 8, 2019

Head elements that are expected to be deduped should be added using next/head

A downside of this approach is whenever you need some npm package to build global <meta>s which are always the same (e.g. mobile-detect package to add <meta name="viewport"> conditionally) you're forced to include it on both server and client side.
Even though the package is actually needed only once in _document on server side (as this type of <meta>s don't need to be re-rendered in _app).

peacefulseeker pushed a commit to peacefulseeker/posture-estimator that referenced this issue Jan 22, 2020
For some reason duplicate was created if
meta viewport was added in _document.

See issue: vercel/next.js#6919
jeremyBanks added a commit to jeremyBanks/speedruns that referenced this issue Jan 30, 2020
lmuller18 added a commit to lmuller18/keycapsets.com that referenced this issue Aug 16, 2020
vercel/next.js#6919 (comment)

Adding key attribute to meta descriptions inside of <Head/> will stop duplicates.

This fixes the issue of linking /set/[someid] not showing the correct preview title or image like intended.
@stratboy
Copy link

I have the same issue. Quite crazy it's still not really solved.

@digitaltim-de
Copy link

Same problem... and can't fix it.

@stratboy
Copy link

stratboy commented Feb 7, 2021

Same problem... and can't fix it.

Hi, for now, I used _app.js and that solved the issue. Obviously, it's not exactly how it should be...

@muratozgul
Copy link

<meta charSet="utf-8" />
or
<meta key="viewport" name="viewport" content="initial-scale=1.0, width=device-width" />
should be direct child of Head. I was returning many tags within a react fragment and I was getting duplicates as well.
I inspected PR#7155 and then moved the meta tags under Head and fixed it.

For example:

const CustomHead = ({ children }: CustomHeadProps) => {
  return (
    <Head>
      <meta charSet="utf-8" />
      <meta key="viewport" name="viewport" content="initial-scale=1.0, width=device-width" />
      <CommonTags />
      <SEOTags />
      <OpenGraphTags />
      <CompatibilityTags />
      <link rel="icon" href="/favicon.ico" />
      {children}
    </Head>
  );
};

@Toxiapo
Copy link

Toxiapo commented Apr 15, 2021

I was getting a duplicate <link" rel='icon' href='/favicon.ico' /, adding a "key" to it solved the problem.

@balazsorban44
Copy link
Member

This issue has been automatically locked due to no recent activity. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@vercel vercel locked as resolved and limited conversation to collaborators Jan 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.