Change page export validity check on client and server in development (#5857)

Resolves #4055 

Credit: https://github.com/zeit/next.js/pull/5095

I didn't use the ignore webpack plugin from the original PR and tested bundle size with https://github.com/zeit/next.js/pull/5339 - seems to be safe on that front.

Was able to get tests to pass locally, unsure of what goes wrong in CI 🤷‍♂️ 

**Questions**
1) The initial PR didn't include changes to `next-server/lib/router` in `getRouteInfo()`. Should the same changes be made within?

2) Should we add a test for rendering a component created via `forwardRef()`?

`component-with-forwardedRef`:
```javascript
export default React.forwardRef((props, ref) => <span {...props} forwardedRef={ref}>This is a component with a forwarded ref</span>);
```

some test:
```javascript
test('renders from forwardRef', async () => {
  const $ = await get$('/component-with-forwardedRef')
  const span = $('span')
  expect(span.text()).toMatch(/This is a component with a forwarded ref/)
})
```

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 !== 'production') {
+        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 }

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"

packages/next-server/server/render.tsx

@@ -106,7 +106,7 @@ function renderDocument(Document: React.ComponentType, {
       files={files}
       dynamicImports={dynamicImports}
       assetPrefix={assetPrefix}
-      {...docProps} 
+      {...docProps}
     />
   )
 }
@@ -131,9 +131,13 @@ export async function renderToHTML (req: IncomingMessage, res: ServerResponse, p
 
   await Loadable.preloadAll() // Make sure all dynamic imports are loaded
 
-  if (typeof Component !== 'function') {
-    throw new Error(`The default export is not a React Component in page: "${pathname}"`)
+  if (dev) {
+    const { isValidElementType } = require('react-is')
+    if (!isValidElementType(Component)) {
+      throw new Error(`The default export is not a React Component in page: "${pathname}"`)
+    }
   }
+
   if (!Document.prototype || !Document.prototype.isReactComponent) throw new Error('_document.js is not exporting a React component')
 
   const asPath = req.url
@@ -141,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
@@ -163,14 +167,14 @@ export async function renderToHTML (req: IncomingMessage, res: ServerResponse, p
       return render(renderElementToString, <ErrorDebug error={err} />)
     }
 
-    return render(renderElementToString, 
+    return render(renderElementToString,
       <LoadableCapture report={(moduleName) => reactLoadableModules.push(moduleName)}>
         <EnhancedApp
           Component={EnhancedComponent}
           router={router}
           {...props}
         />
-      </LoadableCapture>  
+      </LoadableCapture>
     )
   }
 

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 !== 'production') {
+      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

packages/next/package.json

@@ -75,6 +75,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",

test/integration/basic/pages/forwardRef-component.js

@@ -0,0 +1,7 @@
+import React from 'react'
+
+export default React.forwardRef((props, ref) => (
+  <span {...props} forwardedRef={ref}>
+    This is a component with a forwarded ref
+  </span>
+))

test/integration/basic/pages/memo-component.js

@@ -0,0 +1,3 @@
+import React from 'react'
+
+export default React.memo((props) => <span {...props}>Memo component</span>)

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 {};\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 () => /search/;\nexport const fn =')
 
         await check(
           () => getBrowserBodyText(browser),

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.includes('Memo component')).toBeTruthy()
+    })
+
     // default-head contains an empty <Head />.
     test('header renders default charset', async () => {
       const html = await (render('/default-head'))

yarn.lock

@@ -1433,6 +1433,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"
@@ -9245,7 +9252,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==