From 10c8af7c72ad3a20c21521c22bba882d1829f044 Mon Sep 17 00:00:00 2001 From: Kyle Holmberg Date: Tue, 11 Dec 2018 00:52:49 -0800 Subject: [PATCH 1/9] Change page export validity check on client and server in development --- packages/next-server/server/render.js | 7 +++++-- packages/next/client/index.js | 7 +++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/packages/next-server/server/render.js b/packages/next-server/server/render.js index 59fd02ef2253..d5231408c3e8 100644 --- a/packages/next-server/server/render.js +++ b/packages/next-server/server/render.js @@ -44,8 +44,11 @@ async function doRender (req, res, pathname, query, { Component = Component.default || Component - if (typeof Component !== 'function') { - throw new Error(`The default export is not a React Component in page: "${pathname}"`) + if (process.env.NODE_ENV === 'development' || process.env.NODE_ENV === 'test') { + 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 diff --git a/packages/next/client/index.js b/packages/next/client/index.js index 892bb9355b7e..5180caa09952 100644 --- a/packages/next/client/index.js +++ b/packages/next/client/index.js @@ -83,8 +83,11 @@ export default async ({ try { Component = await pageLoader.loadPage(page) - if (typeof Component !== 'function') { - throw new Error(`The default export is not a React Component in page: "${page}"`) + if (process.env.NODE_ENV === 'development' || process.env.NODE_ENV === 'test') { + const { isValidElementType } = require('react-is') + if (!isValidElementType(Component)) { + throw new Error(`The default export is not a React Component in page: "${page}"`) + } } } catch (error) { // This catches errors like throwing in the top level of a module From 2a04d29e9534254b770df6c0596ebf772dfa0dcb Mon Sep 17 00:00:00 2001 From: Kyle Holmberg Date: Tue, 11 Dec 2018 00:57:19 -0800 Subject: [PATCH 2/9] Add react-is to dependencies of next --- packages/next/package.json | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/next/package.json b/packages/next/package.json index f7b0c4563a61..7b478b74d145 100644 --- a/packages/next/package.json +++ b/packages/next/package.json @@ -74,6 +74,7 @@ "prop-types": "15.6.2", "prop-types-exact": "1.2.0", "react-error-overlay": "4.0.0", + "react-is": "16.6.3", "recursive-copy": "2.0.6", "resolve": "1.5.0", "strip-ansi": "3.0.1", From ffe62b34de64942d48f27d12c6bae8fcf16b944a Mon Sep 17 00:00:00 2001 From: Kyle Holmberg Date: Tue, 11 Dec 2018 11:00:01 -0800 Subject: [PATCH 3/9] Change page export validity check on router in development --- packages/next-server/lib/router/router.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/next-server/lib/router/router.js b/packages/next-server/lib/router/router.js index ee48b2f50273..3fcbf4e48f24 100644 --- a/packages/next-server/lib/router/router.js +++ b/packages/next-server/lib/router/router.js @@ -260,8 +260,11 @@ export default class Router { const { Component } = routeInfo - if (typeof Component !== 'function') { - throw new Error(`The default export is not a React Component in page: "${pathname}"`) + if (process.env.NODE_ENV === 'development' || process.env.NODE_ENV === 'test') { + 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 } From 26cf2915db86dc7d26fbfa3467ed2ed74500fed6 Mon Sep 17 00:00:00 2001 From: Kyle Holmberg Date: Tue, 11 Dec 2018 11:22:18 -0800 Subject: [PATCH 4/9] Add rendering tests for memo() and forwardRef() --- test/integration/basic/pages/forwardRef-component.js | 7 +++++++ test/integration/basic/pages/memo-component.js | 3 +++ test/integration/basic/test/rendering.js | 10 ++++++++++ 3 files changed, 20 insertions(+) create mode 100644 test/integration/basic/pages/forwardRef-component.js create mode 100644 test/integration/basic/pages/memo-component.js diff --git a/test/integration/basic/pages/forwardRef-component.js b/test/integration/basic/pages/forwardRef-component.js new file mode 100644 index 000000000000..144abe74b934 --- /dev/null +++ b/test/integration/basic/pages/forwardRef-component.js @@ -0,0 +1,7 @@ +import React from 'react' + +export default React.forwardRef((props, ref) => ( + + This is a component with a forwarded ref + +)) diff --git a/test/integration/basic/pages/memo-component.js b/test/integration/basic/pages/memo-component.js new file mode 100644 index 000000000000..f635e9727793 --- /dev/null +++ b/test/integration/basic/pages/memo-component.js @@ -0,0 +1,3 @@ +import { memo } from 'react' + +export default memo((props) => Memo component) diff --git a/test/integration/basic/test/rendering.js b/test/integration/basic/test/rendering.js index a2381afcf551..4985b99c9c7a 100644 --- a/test/integration/basic/test/rendering.js +++ b/test/integration/basic/test/rendering.js @@ -22,6 +22,16 @@ export default function ({ app }, suiteName, render, fetch) { expect(html.includes('My component!')).toBeTruthy() }) + test('renders when component is a forwardRef instance', async () => { + const html = await render('/forwardRef-component') + expect(html.includes('This is a component with a forwarded ref')).toBeTruthy() + }) + + test('renders when component is a memo instance', async () => { + const html = await render('/memo-component') + expect(html).toStrictEqual('') + }) + // default-head contains an empty . test('header renders default charset', async () => { const html = await (render('/default-head')) From 673728e7377d9beda19dfe865410e87113eb12b2 Mon Sep 17 00:00:00 2001 From: Kyle Holmberg Date: Tue, 11 Dec 2018 13:19:13 -0800 Subject: [PATCH 5/9] Make import uniform --- test/integration/basic/pages/memo-component.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/integration/basic/pages/memo-component.js b/test/integration/basic/pages/memo-component.js index f635e9727793..6bb12c461179 100644 --- a/test/integration/basic/pages/memo-component.js +++ b/test/integration/basic/pages/memo-component.js @@ -1,3 +1,3 @@ -import { memo } from 'react' +import React from 'react' -export default memo((props) => Memo component) +export default React.memo((props) => Memo component) From f584d37297a563dea0fed012462fba972574cd4e Mon Sep 17 00:00:00 2001 From: Kyle Holmberg Date: Tue, 11 Dec 2018 14:14:28 -0800 Subject: [PATCH 6/9] Change hmr error recovery tests to account for strings being allowed --- test/integration/basic/test/error-recovery.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/integration/basic/test/error-recovery.js b/test/integration/basic/test/error-recovery.js index c080541ec623..3d496ac1533a 100644 --- a/test/integration/basic/test/error-recovery.js +++ b/test/integration/basic/test/error-recovery.js @@ -211,7 +211,7 @@ export default (context) => { const text = await browser.elementByCss('p').text() expect(text).toBe('This is the about page.') - aboutPage.replace('export default', 'export default "not-a-page"\nexport const fn = ') + aboutPage.replace('export default', 'export default undefined;\nexport const fn =') await check( () => getBrowserBodyText(browser), @@ -250,7 +250,7 @@ export default (context) => { const text = await browser.elementByCss('p').text() expect(text).toBe('This is the about page.') - aboutPage.replace('export default', 'export default () => /search/ \nexport const fn = ') + aboutPage.replace('export default', 'export default () => undefined;\nexport const fn =') await check( () => getBrowserBodyText(browser), From e8daa266fbf9081d3bd5f2ba1d88a5ff7d674238 Mon Sep 17 00:00:00 2001 From: Kyle Holmberg Date: Tue, 11 Dec 2018 14:17:45 -0800 Subject: [PATCH 7/9] Fix memo test --- test/integration/basic/test/rendering.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/basic/test/rendering.js b/test/integration/basic/test/rendering.js index 4985b99c9c7a..e370ef496d8d 100644 --- a/test/integration/basic/test/rendering.js +++ b/test/integration/basic/test/rendering.js @@ -29,7 +29,7 @@ export default function ({ app }, suiteName, render, fetch) { test('renders when component is a memo instance', async () => { const html = await render('/memo-component') - expect(html).toStrictEqual('') + expect(html.includes('Memo component')).toBeTruthy() }) // default-head contains an empty . From 02fe42962794c41cb1193e7f1c1724f628fea896 Mon Sep 17 00:00:00 2001 From: Kyle Holmberg Date: Fri, 14 Dec 2018 11:38:49 -0800 Subject: [PATCH 8/9] Add types for react-is to next-server --- packages/next-server/package.json | 1 + yarn.lock | 9 ++++++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/next-server/package.json b/packages/next-server/package.json index 5825f5f59fec..dc067d6682c0 100644 --- a/packages/next-server/package.json +++ b/packages/next-server/package.json @@ -44,6 +44,7 @@ "@taskr/watch": "1.1.0", "@types/react": "16.7.13", "@types/react-dom": "16.0.11", + "@types/react-is": "16.5.0", "@types/send": "0.14.4", "taskr": "1.1.0", "typescript": "3.1.6" diff --git a/yarn.lock b/yarn.lock index dabcc208e342..e49bf4cdc640 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1426,6 +1426,13 @@ dependencies: "@types/react" "*" +"@types/react-is@16.5.0": + version "16.5.0" + resolved "https://registry.yarnpkg.com/@types/react-is/-/react-is-16.5.0.tgz#6b0dd43e60fa7c82b48faf7b487543079a61015a" + integrity sha512-yUYPioB2Sh5d4csgpW/vJwxWM0RG1/QbGiwYap2m/bEAQKRwbagYRc5C7oK2AM9QC2vr2ZViCgpm0DpDpFQ6XA== + dependencies: + "@types/react" "*" + "@types/react@*", "@types/react@16.7.13": version "16.7.13" resolved "https://registry.yarnpkg.com/@types/react/-/react-16.7.13.tgz#d2369ae78377356d42fb54275d30218e84f2247a" @@ -9233,7 +9240,7 @@ react-error-overlay@4.0.0: resolved "https://registry.yarnpkg.com/react-error-overlay/-/react-error-overlay-4.0.0.tgz#d198408a85b4070937a98667f500c832f86bd5d4" integrity sha512-FlsPxavEyMuR6TjVbSSywovXSEyOg6ZDj5+Z8nbsRl9EkOzAhEIcS+GLoQDC5fz/t9suhUXWmUrOBrgeUvrMxw== -react-is@^16.3.2: +react-is@16.6.3, react-is@^16.3.2: version "16.6.3" resolved "https://registry.yarnpkg.com/react-is/-/react-is-16.6.3.tgz#d2d7462fcfcbe6ec0da56ad69047e47e56e7eac0" integrity sha512-u7FDWtthB4rWibG/+mFbVd5FvdI20yde86qKGx4lVUTWmPlSWQ4QxbBIrrs+HnXGbxOUlUzTAP/VDmvCwaP2yA== From d5bfbef6880f353fe2c644093f21e0e6f5459f3c Mon Sep 17 00:00:00 2001 From: Tim Neutkens Date: Mon, 17 Dec 2018 15:44:27 +0100 Subject: [PATCH 9/9] Fix tests `undefined` is not handled correctly by HMR, so revert this change in favor of the original values --- packages/next-server/lib/router/router.js | 2 +- packages/next-server/server/render.tsx | 4 ++-- packages/next/client/index.js | 2 +- test/integration/basic/test/error-recovery.js | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/next-server/lib/router/router.js b/packages/next-server/lib/router/router.js index 3fcbf4e48f24..82aa924b3980 100644 --- a/packages/next-server/lib/router/router.js +++ b/packages/next-server/lib/router/router.js @@ -260,7 +260,7 @@ export default class Router { const { Component } = routeInfo - if (process.env.NODE_ENV === 'development' || process.env.NODE_ENV === 'test') { + if (process.env.NODE_ENV !== '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/packages/next-server/server/render.tsx b/packages/next-server/server/render.tsx index 7b2600d338d4..c328cf5563b4 100644 --- a/packages/next-server/server/render.tsx +++ b/packages/next-server/server/render.tsx @@ -131,7 +131,7 @@ export async function renderToHTML (req: IncomingMessage, res: ServerResponse, p await Loadable.preloadAll() // Make sure all dynamic imports are loaded - if (process.env.NODE_ENV === 'development' || process.env.NODE_ENV === 'test') { + if (dev) { const { isValidElementType } = require('react-is') if (!isValidElementType(Component)) { throw new Error(`The default export is not a React Component in page: "${pathname}"`) @@ -145,7 +145,7 @@ export async function renderToHTML (req: IncomingMessage, res: ServerResponse, p const router = new Router(pathname, query, asPath) const props = await loadGetInitialProps(App, {Component, router, ctx}) - // the response might be finshed on the getInitialProps call + // the response might be finished on the getInitialProps call if (isResSent(res)) return null const devFiles = buildManifest.devFiles diff --git a/packages/next/client/index.js b/packages/next/client/index.js index 5180caa09952..737c42e1aea7 100644 --- a/packages/next/client/index.js +++ b/packages/next/client/index.js @@ -83,7 +83,7 @@ export default async ({ try { Component = await pageLoader.loadPage(page) - if (process.env.NODE_ENV === 'development' || process.env.NODE_ENV === 'test') { + if (process.env.NODE_ENV !== 'production') { const { isValidElementType } = require('react-is') if (!isValidElementType(Component)) { throw new Error(`The default export is not a React Component in page: "${page}"`) diff --git a/test/integration/basic/test/error-recovery.js b/test/integration/basic/test/error-recovery.js index 3d496ac1533a..57b50a9db36b 100644 --- a/test/integration/basic/test/error-recovery.js +++ b/test/integration/basic/test/error-recovery.js @@ -211,7 +211,7 @@ export default (context) => { const text = await browser.elementByCss('p').text() expect(text).toBe('This is the about page.') - aboutPage.replace('export default', 'export default undefined;\nexport const fn =') + aboutPage.replace('export default', 'export default {};\nexport const fn =') await check( () => getBrowserBodyText(browser), @@ -250,7 +250,7 @@ export default (context) => { const text = await browser.elementByCss('p').text() expect(text).toBe('This is the about page.') - aboutPage.replace('export default', 'export default () => undefined;\nexport const fn =') + aboutPage.replace('export default', 'export default () => /search/;\nexport const fn =') await check( () => getBrowserBodyText(browser),