From 9c987f8b6157889138794af0dc2c7eae5dd5376a Mon Sep 17 00:00:00 2001 From: Spencer Elliott Date: Tue, 4 Sep 2018 19:36:54 -0400 Subject: [PATCH 1/4] Use react-is to check component validity Fixes #4055 --- client/index.js | 3 ++- lib/router/router.js | 3 ++- package.json | 1 + server/render.js | 3 ++- 4 files changed, 7 insertions(+), 3 deletions(-) diff --git a/client/index.js b/client/index.js index 4fb3edcdb6c1..ca94e555f3a4 100644 --- a/client/index.js +++ b/client/index.js @@ -1,5 +1,6 @@ import React from 'react' import ReactDOM from 'react-dom' +import { isValidElementType } from 'react-is' import HeadManager from './head-manager' import { createRouter } from '../lib/router' import EventEmitter from '../lib/EventEmitter' @@ -82,7 +83,7 @@ export default async ({ try { Component = await pageLoader.loadPage(page) - if (typeof Component !== 'function') { + if (!isValidElementType(Component)) { throw new Error(`The default export is not a React Component in page: "${pathname}"`) } } catch (error) { diff --git a/lib/router/router.js b/lib/router/router.js index 61e6a07e7c6f..404a30cae309 100644 --- a/lib/router/router.js +++ b/lib/router/router.js @@ -1,6 +1,7 @@ /* global __NEXT_DATA__ */ import { parse, format } from 'url' +import { isValidElementType } from 'react-is' import EventEmitter from '../EventEmitter' import shallowEquals from '../shallow-equals' import PQueue from '../p-queue' @@ -245,7 +246,7 @@ export default class Router { const { Component } = routeInfo - if (typeof Component !== 'function') { + if (!isValidElementType(Component)) { throw new Error(`The default export is not a React Component in page: "${pathname}"`) } diff --git a/package.json b/package.json index f8852ad6ecbb..4ab111e1bb5a 100644 --- a/package.json +++ b/package.json @@ -100,6 +100,7 @@ "prop-types": "15.6.2", "prop-types-exact": "1.2.0", "react-error-overlay": "4.0.0", + "react-is": "16.4.2", "recursive-copy": "2.0.6", "resolve": "1.5.0", "send": "0.16.1", diff --git a/server/render.js b/server/render.js index f82a7f65983b..ad0037cdb324 100644 --- a/server/render.js +++ b/server/render.js @@ -1,6 +1,7 @@ import { join } from 'path' import React from 'react' import { renderToString, renderToStaticMarkup } from 'react-dom/server' +import { isValidElementType } from 'react-is' import send from 'send' import generateETag from 'etag' import fresh from 'fresh' @@ -87,7 +88,7 @@ async function doRender (req, res, pathname, query, { Component = Component.default || Component - if (typeof Component !== 'function') { + if (!isValidElementType(Component)) { throw new Error(`The default export is not a React Component in page: "${pathname}"`) } From e825944e6d7708d7c1d5a6a0d66017c789800ef3 Mon Sep 17 00:00:00 2001 From: Spencer Elliott Date: Wed, 5 Sep 2018 11:08:19 -0400 Subject: [PATCH 2/4] Fix invalid page test The test previously used the string value "not-a-page" to verify that Next.js renders the error "The default export is not a React Component". This no longer works because "not-a-page" is technically an allowable element now, which renders an HTML element called . Fixed by replacing the value with an object: { not: "a-page" } --- test/integration/basic/test/error-recovery.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/basic/test/error-recovery.js b/test/integration/basic/test/error-recovery.js index 12a9da06d5a1..85cc776642ab 100644 --- a/test/integration/basic/test/error-recovery.js +++ b/test/integration/basic/test/error-recovery.js @@ -177,7 +177,7 @@ export default (context, render) => { expect(text).toBe('This is the about page.') const aboutPage = new File(join(__dirname, '../', 'pages', 'hmr', 'about.js')) - aboutPage.replace('export default', 'export default "not-a-page"\nexport const fn = ') + aboutPage.replace('export default', 'export default { not: "a-page" }\nexport const fn = ') await waitFor(3000) From 51d3b5baab761f0b18ef6ed243e54d8113ac1383 Mon Sep 17 00:00:00 2001 From: Spencer Elliott Date: Thu, 6 Sep 2018 16:38:56 -0400 Subject: [PATCH 3/4] Check isValidElementType only in development --- client/index.js | 8 +++++--- lib/router/router.js | 8 +++++--- server/render.js | 8 +++++--- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/client/index.js b/client/index.js index ca94e555f3a4..bfaa913b9221 100644 --- a/client/index.js +++ b/client/index.js @@ -1,6 +1,5 @@ import React from 'react' import ReactDOM from 'react-dom' -import { isValidElementType } from 'react-is' import HeadManager from './head-manager' import { createRouter } from '../lib/router' import EventEmitter from '../lib/EventEmitter' @@ -83,8 +82,11 @@ export default async ({ try { Component = await pageLoader.loadPage(page) - if (!isValidElementType(Component)) { - throw new Error(`The default export is not a React Component in page: "${pathname}"`) + if (process.env.NODE_ENV === 'development') { + const { isValidElementType } = require('react-is') + if (!isValidElementType(Component)) { + throw new Error(`The default export is not a React Component in page: "${pathname}"`) + } } } catch (error) { // This catches errors like throwing in the top level of a module diff --git a/lib/router/router.js b/lib/router/router.js index 404a30cae309..da49a5a72c4f 100644 --- a/lib/router/router.js +++ b/lib/router/router.js @@ -1,7 +1,6 @@ /* global __NEXT_DATA__ */ import { parse, format } from 'url' -import { isValidElementType } from 'react-is' import EventEmitter from '../EventEmitter' import shallowEquals from '../shallow-equals' import PQueue from '../p-queue' @@ -246,8 +245,11 @@ export default class Router { const { Component } = routeInfo - if (!isValidElementType(Component)) { - throw new Error(`The default export is not a React Component in page: "${pathname}"`) + if (process.env.NODE_ENV === 'development') { + const { isValidElementType } = require('react-is') + if (!isValidElementType(Component)) { + throw new Error(`The default export is not a React Component in page: "${pathname}"`) + } } const ctx = { pathname, query, asPath: as } diff --git a/server/render.js b/server/render.js index ad0037cdb324..e1acc769228b 100644 --- a/server/render.js +++ b/server/render.js @@ -1,7 +1,6 @@ import { join } from 'path' import React from 'react' import { renderToString, renderToStaticMarkup } from 'react-dom/server' -import { isValidElementType } from 'react-is' import send from 'send' import generateETag from 'etag' import fresh from 'fresh' @@ -88,8 +87,11 @@ async function doRender (req, res, pathname, query, { Component = Component.default || Component - if (!isValidElementType(Component)) { - throw new Error(`The default export is not a React Component in page: "${pathname}"`) + if (process.env.NODE_ENV === 'development') { + const { isValidElementType } = require('react-is') + if (!isValidElementType(Component)) { + throw new Error(`The default export is not a React Component in page: "${pathname}"`) + } } App = App.default || App From 5ddd91c07d00fa9cd80cbfc85c8dc1e917a875b1 Mon Sep 17 00:00:00 2001 From: Tim Neutkens Date: Wed, 12 Sep 2018 19:02:29 +0200 Subject: [PATCH 4/4] Ignore react-is in production --- build/webpack.js | 9 +++++++++ lib/router/router.js | 1 + package.json | 2 +- 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/build/webpack.js b/build/webpack.js index 3fa51d755b02..ec333e96af94 100644 --- a/build/webpack.js +++ b/build/webpack.js @@ -245,6 +245,15 @@ export default async function getBaseWebpackConfig (dir: string, {dev = false, i }), dev && !isServer && new FriendlyErrorsWebpackPlugin(), new webpack.IgnorePlugin(/(precomputed)/, /node_modules.+(elliptic)/), + // This removes react-is in production, as it's not used there. + !dev && new webpack.IgnorePlugin({ + checkResource: (resource) => { + return /react-is/.test(resource) + }, + checkContext: (context) => { + return context.indexOf(path.join(NEXT_PROJECT_ROOT_DIST, 'lib', 'router')) !== -1 + } + }), // Even though require.cache is server only we have to clear assets from both compilations // This is because the client compilation generates the build manifest that's used on the server side dev && new NextJsRequireCacheHotReloader(), diff --git a/lib/router/router.js b/lib/router/router.js index da49a5a72c4f..c30bb8a3d057 100644 --- a/lib/router/router.js +++ b/lib/router/router.js @@ -246,6 +246,7 @@ export default class Router { const { Component } = routeInfo if (process.env.NODE_ENV === 'development') { + // This package is ignored for this file using webpack.IgnorePlugin in production const { isValidElementType } = require('react-is') if (!isValidElementType(Component)) { throw new Error(`The default export is not a React Component in page: "${pathname}"`) diff --git a/package.json b/package.json index bb3bda374ee7..5d9abded4589 100644 --- a/package.json +++ b/package.json @@ -100,7 +100,7 @@ "prop-types": "15.6.2", "prop-types-exact": "1.2.0", "react-error-overlay": "4.0.0", - "react-is": "16.4.2", + "react-is": "16.5.0", "recursive-copy": "2.0.6", "resolve": "1.5.0", "send": "0.16.1",