Next.js: Multiple viewport meta tags

Created on 22 May 2020  ·  15Comments  ·  Source: vercel/next.js

Bug report

Describe the bug

Custom viewport meta tags in the _document.js page are ignored by the browser because next.js pushes it's own viewport meta tag without checking if any exists already. As a result, we end up with this:

<head>
  ...
  <meta name="viewport" content="viewport-fit=cover"/> <!-- ignored 🤕 -->
  ...
  <meta name="viewport" content="width=device-width" />
  ...
</head>

I could trace the problem down to this part of the code:

https://github.com/zeit/next.js/blob/86160a5190c50ea315c7ba91d77dfb51c42bc65f/packages/next/next-server/lib/head.tsx#L13-L15

To Reproduce

  1. Define a viewport meta tag in the _document.js page.
  2. Open any page of your app and look for <meta name="viewport"

I also made a full reproducible example, but I faced issues with Codesandbox. If you decide to run the reproducible, please download the source code and run it on your machine instead of the online sandbox.

Screenshots

image

System information

  • OS: macOS
  • Browser: tested in chrome, safari and firefox
  • Version of Next.js: 9.4.2
  • Version of Node.js: 12.16.3
good first issue bug
Source
picture zvictor

Most helpful comment

...And as per further discussion in #13408, I'm now working on a PR to warn users when they add a viewport tag to next/document's <Head>, as viewport is supposed to be handled by next/head. 👍🏼

picture tywmick on 27 May 2020
🚀5 ❤5 👎1

All 15 comments

Taking a stab at this one! My apologies if I'm over-documenting my investigation process. 🙈

I don't think the defaultHead function @zvictor mentioned...

https://github.com/zeit/next.js/blob/91b1548d3296dae4306ad68b97fd61d092c06ebe/packages/next/next-server/lib/head.tsx#L11-L17

...is the problem, because farther down, after these default head elements are added to the page's other head elements, duplicates are filtered out (e.g., only the first <meta name="viewport" element in the list should make it through).

Downloading @zvictor's example, I found that if I comment out the meta tag in _document.js (where { Head } is imported from "next/document")...

<Head>
  {/* <meta name="viewport" content="viewport-fit=cover" /> */}
</Head>

...and add it in a new Head component in index.js (importing Head from "next/head")...

export default () => (
  <>
    <Head>
      <meta name="viewport" content="viewport-fit=cover" />
    </Head>
    <Viewport />
    <Environment />
  </>
)

...it works correctly! Only the correct viewport element shows up on the page.

So the problem has to lie in "next/document" somewhere. I'll head there next!

tywmick picture tywmick on 25 May 2020
👍2 🚀1 ❤1

Alright, in _document.tsx's Head component, both <meta name="viewport"s are being inserted into the <head> (among several other things) here:

<head {...this.props}>
  ...
  {children}
  {head}
  ...
</head>
  • <meta name="viewport" content="viewport-fit=cover" /> (the user-entered one) is part of children

    • children is this.props.children

  • <meta name="viewport" content="width=device-width" /> (the default one) is part of head

    • head comes from this.context._documentProps

I'm losing the trail a bit trying to track backwards, but I think this.context._documentProps ultimately comes from the individual page's headers as determined by next-server/lib/head.tsx, which includes the "next/head" export. So I'm guessing we can't just call those methods from head.tsx directly to deduplicate these meta tags or else we'd run into some circular dependencies or whatever that would be called.

As far as how to fix this... Is this something that should only be done in _app.js using "next/head"? We already warn users not to put a title tag in their _document.js because it leads to unexpected results.

https://github.com/zeit/next.js/blob/2828b01950f30e2790a870908255e6c28d96eebb/packages/next/pages/_document.tsx#L302-L306

I'm afraid I still haven't quite wrapped my head around the difference between _app.js and _document.js...

So which way should I move forward:

  1. Add an additional console warning not to include viewport meta tags in _document.js.

    • It may be best to discourage this anyway, if Document is only rendered in the server as the docs say. But is it really only rendered in the server if head elements added in _document.js show up in the client?

  2. Add some similar deduplicating functionality to _document.tsx to merge children (user-entered head elements from _document.js) and head (default head elements and user-entered head elements from _app.js or any individual page) before inserting.

Wait a minute, though: I just realized that if I do #‌2, user-entered head elements in _document.js will override matching user-entered head elements from _app.js or individual pages... Should adding children to <Head> in _document.js even be allowed?! I'm going to go compose my thoughts and probably ask the question over in Discussions.

tywmick picture tywmick on 26 May 2020

I guess I'm not the only one musing about this: #12290

tywmick picture tywmick on 26 May 2020

Alrighty, just started a dedicated discussion #13408. Its thesis: should we discourage adding any children to next/document's Head like we're currently discouraging adding <title> tags to _document.js?

tywmick picture tywmick on 26 May 2020

But is it _really_ only rendered in the server if head elements added in _document.js show up in the client?

Now I know what "render" actually means (or at least what it doesn't). 🤦🏼‍♂️ Learning! ✊🏼

tywmick picture tywmick on 26 May 2020

Alright, I take from the response to #13408 that <meta name="viewport" content="viewport-fit=cover" /> isn't a weird thing to add to _document.js, after all. I'll get to brainstorming how to deduplicate cleanly.

tywmick picture tywmick on 26 May 2020

...And as per further discussion in #13408, I'm now working on a PR to warn users when they add a viewport tag to next/document's <Head>, as viewport is supposed to be handled by next/head. 👍🏼

tywmick picture tywmick on 27 May 2020
🚀5 ❤5 👎1

I would like to thank all the collaborators involved in solving this issue and releasing it in the new versions, in special @tywmick. Even though it doesn't seem that the issue was affecting him directly, he has put a lot of effort into investigating the issue and studying all possible solutions, not stopping at the first easy fix for it. Great job! 🎉

Open-source communities keep amazing me ❤️

I can confirm that I now (v9.4.5-canary.42) see the warning message in my app, as expected:

Warning: viewport meta tags should not be used in _document.js's . https://err.sh/next.js/no-document-viewport-meta

The only problem I found is that the error url is actually redirecting to a 404 destination.

zvictor picture zvictor on 22 Jul 2020
👍2 ❤1

The only problem I found is that the error url is actually redirecting to a 404 destination.

This is because the branch was not released yet, when it's on stable it will link correctly.

timneutkens picture timneutkens on 23 Jul 2020
👍3

Ok so where are we supposed to put our viewport tags? The warning without a solution is not very useful.

nickatrokit picture nickatrokit on 30 Jul 2020

Warning: viewport meta tags should not be used in _document.js's . https://err.sh/next.js/no-document-viewport-meta

@nickatrokit did you open the link attached to the warning, it says to put it in _app.

timneutkens picture timneutkens on 31 Jul 2020
👍1

@timneutkens So where is the proper place to put site-wide metadata tags? In _document.tsx or _app.tsx?

nickatrokit picture nickatrokit on 5 Aug 2020

@timneutkens So where is the proper place to put site-wide metadata tags? In _document.tsx or _app.tsx?

_app.js/_app.tsx

chrishrtmn picture chrishrtmn on 5 Aug 2020

@chrishrtmn This seems like an odd place to put them since developers like me are used to putting them next to the html tag which is in _document. So I think most people would start by putting them in _document.

nickatrokit picture nickatrokit on 5 Aug 2020

For those like me who had custom components for head's children in App; I found a pattern that doesn't change much code, and is very clean => use arrays inside a wrapping function.

Before :

const ViewportMetaLink = () => (
  <meta
    name='viewport'
    content='minimum-scale=1, initial-scale=1, width=device-width'
    key='viewport-meta'
  />
)

const SansSerifFontLink = () => (
  <link
    rel='stylesheet'
    href={FONTS.sansSerif.link}
    key='sans-serif-font-link'
  />
)

const App = ({ Component, pageProps }: AppProps) => (
  <>
    <Head>
      <ViewportMetaLink />
      <SansSerifFontLink />
    </Head>

    <div />
  </>
)

After :

const ViewportMetaLink = () => (
  <meta
    name='viewport'
    content='minimum-scale=1, initial-scale=1, width=device-width'
    key='viewport-meta'
  />
)

const SansSerifFontLink = () => (
  <link
    rel='stylesheet'
    href={FONTS.sansSerif.link}
    key='sans-serif-font-link'
  />
)

const links = () => ([ViewportMetaLink(), SansSerifFontLink()])

const App = ({ Component, pageProps }: AppProps) => (
  <>
    <Head>
      {links()}
    </Head>

    <div />
  </>
)
cyrilchapon picture cyrilchapon on 14 Nov 2020
Was this page helpful?
4 / 5 - 1 ratings

Related issues

Knaackee picture Knaackee  ·  122Comments
timneutkens picture timneutkens  ·  250Comments
robinvdvleuten picture robinvdvleuten  ·  74Comments
dunika picture dunika  ·  58Comments
Timer picture Timer  ·  60Comments