From 86066bd045292aa6623b716fae78de80a0486af5 Mon Sep 17 00:00:00 2001 From: Ty Mick Date: Wed, 27 May 2020 12:10:20 -0400 Subject: [PATCH 1/6] Add initial warning --- packages/next/pages/_document.tsx | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/packages/next/pages/_document.tsx b/packages/next/pages/_document.tsx index 1ba5e18f7eb62..07cc002bd9ece 100644 --- a/packages/next/pages/_document.tsx +++ b/packages/next/pages/_document.tsx @@ -299,10 +299,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: 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>." + ) + } } return child }) From 9e94b13498bbae15b7fe4907a1e5023b596b52ef Mon Sep 17 00:00:00 2001 From: Ty Mick <ty@tymick.me> Date: Wed, 27 May 2020 12:31:17 -0400 Subject: [PATCH 2/6] Add no-document-viewport-meta error page --- errors/no-document-viewport-meta.md | 45 +++++++++++++++++++++++++++++ packages/next/pages/_document.tsx | 2 +- 2 files changed, 46 insertions(+), 1 deletion(-) create mode 100644 errors/no-document-viewport-meta.md diff --git a/errors/no-document-viewport-meta.md b/errors/no-document-viewport-meta.md new file mode 100644 index 0000000000000..9369907134b97 --- /dev/null +++ b/errors/no-document-viewport-meta.md @@ -0,0 +1,45 @@ +# 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 the viewport is handled by `next/head`. + +#### Possible Ways to Fix It + +Set your viewport `meta` tag in `pages/_app.js` instead: + +```js +// pages/_app.js +import App from 'next/app' +import Head from 'next/head' +import React from 'react' + +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> + <meta name="viewport" content="viewport-fit=cover" /> + </Head> + <Component {...pageProps} /> + </> + ) + } +} +``` + +### Useful Links + +- [Issue #13230](https://github.com/zeit/next.js/issues/13230), which led to the creation of this warning. diff --git a/packages/next/pages/_document.tsx b/packages/next/pages/_document.tsx index 07cc002bd9ece..1b2ba3a6757bd 100644 --- a/packages/next/pages/_document.tsx +++ b/packages/next/pages/_document.tsx @@ -309,7 +309,7 @@ export class Head extends Component< child?.props?.name === 'viewport' ) { console.warn( - "Warning: viewport meta tags should not be used in _document.js's <Head>." + "Warning: viewport meta tags should not be used in _document.js's <Head>. https://err.sh/next.js/no-document-viewport-meta" ) } } From b32b3ab8cd4a0870d1f2015ab5be288720646555 Mon Sep 17 00:00:00 2001 From: Ty Mick <ty@tymick.me> Date: Thu, 28 May 2020 11:59:13 -0400 Subject: [PATCH 3/6] Update code snippet to modern custom app design --- errors/no-document-viewport-meta.md | 51 ++++++++++++++--------------- 1 file changed, 25 insertions(+), 26 deletions(-) diff --git a/errors/no-document-viewport-meta.md b/errors/no-document-viewport-meta.md index 9369907134b97..5cf01d89db8ce 100644 --- a/errors/no-document-viewport-meta.md +++ b/errors/no-document-viewport-meta.md @@ -8,36 +8,35 @@ Adding `<meta name="viewport" ...>` in `pages/_document.js` will lead to unexpec Set your viewport `meta` tag in `pages/_app.js` instead: -```js +```jsx // 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> - <meta name="viewport" content="viewport-fit=cover" /> - </Head> - <Component {...pageProps} /> - </> - ) - } +function MyApp({ Component, pageProps }) { + return ( + <> + <Head> + <meta name="viewport" content="viewport-fit=cover" /> + </Head> + <Component {...pageProps} /> + </> + ) } + +// Only uncomment this method if you have blocking data requirements for +// every single page in your application. This disables the ability to +// perform automatic static optimization, causing every page in your app to +// be server-side rendered. +// +// MyApp.getInitialProps = async (appContext) => { +// // calls page's `getInitialProps` and fills `appProps.pageProps` +// const appProps = await App.getInitialProps(appContext); +// +// return { ...appProps } +// } + +export default MyApp ``` ### Useful Links From af0e5a1fded4e0e8c1f25cee35113e6ec28b835e Mon Sep 17 00:00:00 2001 From: Ty Mick <ty@tymick.me> Date: Thu, 28 May 2020 12:00:29 -0400 Subject: [PATCH 4/6] Update "no document title" error to modern _app --- errors/no-document-title.md | 49 ++++++++++++++++++------------------- 1 file changed, 24 insertions(+), 25 deletions(-) diff --git a/errors/no-document-title.md b/errors/no-document-title.md index 62a3cc5d88b0f..fabe3ea7cde4a 100644 --- a/errors/no-document-title.md +++ b/errors/no-document-title.md @@ -10,34 +10,33 @@ 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 - - - - ) - } +function MyApp({ Component, pageProps }) { + return ( + <> + + My new cool app + + + + ) } + +// Only uncomment this method if you have blocking data requirements for +// every single page in your application. This disables the ability to +// perform automatic static optimization, causing every page in your app to +// be server-side rendered. +// +// MyApp.getInitialProps = async (appContext) => { +// // calls page's `getInitialProps` and fills `appProps.pageProps` +// const appProps = await App.getInitialProps(appContext); +// +// return { ...appProps } +// } + +export default MyApp ``` ### Useful Links From 57706c3f32cce34099d2f31e16024f3f3db8ce88 Mon Sep 17 00:00:00 2001 From: Joe Haddad Date: Thu, 28 May 2020 18:43:08 -0400 Subject: [PATCH 5/6] Apply suggestions from code review --- errors/no-document-title.md | 12 ------------ errors/no-document-viewport-meta.md | 15 ++------------- 2 files changed, 2 insertions(+), 25 deletions(-) diff --git a/errors/no-document-title.md b/errors/no-document-title.md index fabe3ea7cde4a..f55a96c4bcf14 100644 --- a/errors/no-document-title.md +++ b/errors/no-document-title.md @@ -24,18 +24,6 @@ function MyApp({ Component, pageProps }) { ) } -// Only uncomment this method if you have blocking data requirements for -// every single page in your application. This disables the ability to -// perform automatic static optimization, causing every page in your app to -// be server-side rendered. -// -// MyApp.getInitialProps = async (appContext) => { -// // calls page's `getInitialProps` and fills `appProps.pageProps` -// const appProps = await App.getInitialProps(appContext); -// -// return { ...appProps } -// } - export default MyApp ``` diff --git a/errors/no-document-viewport-meta.md b/errors/no-document-viewport-meta.md index 5cf01d89db8ce..1e7965d77c847 100644 --- a/errors/no-document-viewport-meta.md +++ b/errors/no-document-viewport-meta.md @@ -2,7 +2,8 @@ #### Why This Error Occurred -Adding `` in `pages/_document.js` will lead to unexpected results since the viewport is handled by `next/head`. +Adding `` 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 @@ -24,18 +25,6 @@ function MyApp({ Component, pageProps }) { ) } -// Only uncomment this method if you have blocking data requirements for -// every single page in your application. This disables the ability to -// perform automatic static optimization, causing every page in your app to -// be server-side rendered. -// -// MyApp.getInitialProps = async (appContext) => { -// // calls page's `getInitialProps` and fills `appProps.pageProps` -// const appProps = await App.getInitialProps(appContext); -// -// return { ...appProps } -// } - export default MyApp ``` From 1893f3de5d0a814c19b0d791b0a0603029be734f Mon Sep 17 00:00:00 2001 From: Ty Mick Date: Fri, 5 Jun 2020 16:48:48 -0400 Subject: [PATCH 6/6] Add test suite for document/head warnings --- .../document-head-warnings/pages/_document.js | 20 +++++++++ .../document-head-warnings/pages/index.js | 14 ++++++ .../document-head-warnings/test/index.test.js | 43 +++++++++++++++++++ 3 files changed, 77 insertions(+) create mode 100644 test/integration/document-head-warnings/pages/_document.js create mode 100644 test/integration/document-head-warnings/pages/index.js create mode 100644 test/integration/document-head-warnings/test/index.test.js diff --git a/test/integration/document-head-warnings/pages/_document.js b/test/integration/document-head-warnings/pages/_document.js new file mode 100644 index 0000000000000..8c94d830bb60b --- /dev/null +++ b/test/integration/document-head-warnings/pages/_document.js @@ -0,0 +1,20 @@ +import Document, { Html, Head, Main, NextScript } from 'next/document' + +class MyDocument extends Document { + render() { + return ( + + + Check out this title! + + + +
+ + + + ) + } +} + +export default MyDocument diff --git a/test/integration/document-head-warnings/pages/index.js b/test/integration/document-head-warnings/pages/index.js new file mode 100644 index 0000000000000..c66f182284ee6 --- /dev/null +++ b/test/integration/document-head-warnings/pages/index.js @@ -0,0 +1,14 @@ +function Hi() { + return ( +
+

Hello world!

+ +
+ ) +} + +export default Hi diff --git a/test/integration/document-head-warnings/test/index.test.js b/test/integration/document-head-warnings/test/index.test.js new file mode 100644 index 0000000000000..dfb6051d59ccb --- /dev/null +++ b/test/integration/document-head-warnings/test/index.test.js @@ -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 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\..*/ + ) + }) + }) +})