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

Add warning when viewport meta tag is added to _document.js #13452

Merged
merged 8 commits into from
Jul 7, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 12 additions & 25 deletions errors/no-document-title.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,34 +10,21 @@ Set `<title>` in `pages/_app.js` instead:

```js
// pages/_app.js
import App from 'next/app'
import Head from 'next/head'
import React from 'react'
import Head from 'next/head'

export default class MyApp extends App {
static async getInitialProps({ Component, ctx }) {
let pageProps = {}

if (Component.getInitialProps) {
pageProps = await Component.getInitialProps(ctx)
}

return { pageProps }
}

render() {
const { Component, pageProps } = this.props

return (
<>
<Head>
<title>My new cool app</title>
</Head>
<Component {...pageProps} />
</>
)
}
function MyApp({ Component, pageProps }) {
return (
<>
<Head>
<title>My new cool app</title>
</Head>
<Component {...pageProps} />
</>
)
}

export default MyApp
```

### Useful Links
Expand Down
33 changes: 33 additions & 0 deletions errors/no-document-viewport-meta.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# Viewport `meta` tags should not be used in `_document.js`'s `<Head>`

#### Why This Error Occurred

Adding `<meta name="viewport" ...>` in `pages/_document.js` will lead to unexpected results since it cannot be deduped.
The viewport tag should be handled by `next/head` in `pages/_app.js`.

#### Possible Ways to Fix It

Set your viewport `meta` tag in `pages/_app.js` instead:

```jsx
// pages/_app.js
import React from 'react'
import Head from 'next/head'

function MyApp({ Component, pageProps }) {
return (
<>
<Head>
<meta name="viewport" content="viewport-fit=cover" />
</Head>
<Component {...pageProps} />
</>
)
}

export default MyApp
```

### Useful Links

- [Issue #13230](https://github.com/zeit/next.js/issues/13230), which led to the creation of this warning.
17 changes: 13 additions & 4 deletions packages/next/pages/_document.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -256,10 +256,19 @@ export class Head extends Component<
if (process.env.NODE_ENV !== 'production') {
children = React.Children.map(children, (child: any) => {
const isReactHelmet = child?.props?.['data-react-helmet']
if (child?.type === 'title' && !isReactHelmet) {
console.warn(
"Warning: <title> should not be used in _document.js's <Head>. https://err.sh/next.js/no-document-title"
)
if (!isReactHelmet) {
if (child?.type === 'title') {
console.warn(
"Warning: <title> should not be used in _document.js's <Head>. https://err.sh/next.js/no-document-title"
)
} else if (
child?.type === 'meta' &&
child?.props?.name === 'viewport'
) {
console.warn(
"Warning: viewport meta tags should not be used in _document.js's <Head>. https://err.sh/next.js/no-document-viewport-meta"
)
}
}
return child
})
Expand Down
20 changes: 20 additions & 0 deletions test/integration/document-head-warnings/pages/_document.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import Document, { Html, Head, Main, NextScript } from 'next/document'

class MyDocument extends Document {
render() {
return (
<Html>
<Head crossOrigin="anonymous">
<title>Check out this title!</title>
<meta name="viewport" content="viewport-fit=cover" />
</Head>
<body>
<Main />
<NextScript />
</body>
</Html>
)
}
}

export default MyDocument
14 changes: 14 additions & 0 deletions test/integration/document-head-warnings/pages/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
function Hi() {
return (
<div>
<p>Hello world!</p>
<style jsx>{`
p {
font-size: 16.4px;
}
`}</style>
</div>
)
}

export default Hi
43 changes: 43 additions & 0 deletions test/integration/document-head-warnings/test/index.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/* eslint-env jest */

import { join } from 'path'
import { renderViaHTTP, findPort, launchApp, killApp } from 'next-test-utils'

jest.setTimeout(1000 * 60)
const appDir = join(__dirname, '..')
let output

describe('Custom Document Head Warnings', () => {
beforeAll(async () => {
const handleOutput = (msg) => {
output += msg
}
const appPort = await findPort()
const app = await launchApp(appDir, appPort, {
onStdout: handleOutput,
onStderr: handleOutput,
})
await renderViaHTTP(appPort, '/')
await killApp(app)
})

describe('development mode', () => {
it('warns when using a <title> in document/head', () => {
expect(output).toMatch(
/.*Warning: <title> should not be used in _document.js's <Head>\..*/
)
})

it('warns when using viewport meta tags in document/head', () => {
expect(output).toMatch(
/.*Warning: viewport meta tags should not be used in _document.js's <Head>\..*/
)
})

it('warns when using a crossOrigin attribute on document/head', () => {
expect(output).toMatch(
/.*Warning: `Head` attribute `crossOrigin` is deprecated\..*/
)
})
})
})