Make sure lastAppProps always have some value. (vercel#829)

* Make sure lastAppProps always have some value.

* Revert "Make sure lastAppProps always have some value."

This reverts commit b4ae722.

* Throw an error, if we found an empty object from getInitialProps.

* Add proper tests for getInitialProps empty check.

Author: arunoda

client/index.js

@@ -5,6 +5,7 @@ import { rehydrate } from '../lib/css'
 import { createRouter } from '../lib/router'
 import App from '../lib/app'
 import evalScript from '../lib/eval-script'
+import { loadGetInitialProps } from '../lib/utils'
 
 const {
   __NEXT_DATA__: {
@@ -51,7 +52,7 @@ export async function render (props, onError = renderErrorComponent) {
 
 async function renderErrorComponent (err) {
   const { pathname, query } = router
-  const props = await getInitialProps(ErrorComponent, { err, pathname, query })
+  const props = await loadGetInitialProps(ErrorComponent, { err, pathname, query })
   await doRender({ Component: ErrorComponent, props, err })
 }
 
@@ -61,7 +62,7 @@ async function doRender ({ Component, props, err }) {
     lastAppProps.Component === ErrorComponent) {
     // fetch props if ErrorComponent was replaced with a page component by HMR
     const { pathname, query } = router
-    props = await getInitialProps(Component, { err, pathname, query })
+    props = await loadGetInitialProps(Component, { err, pathname, query })
   }
 
   Component = Component || lastAppProps.Component
@@ -71,7 +72,3 @@ async function doRender ({ Component, props, err }) {
   lastAppProps = appProps
   ReactDOM.render(createElement(App, appProps), container)
 }
-
-function getInitialProps (Component, ctx) {
-  return Component.getInitialProps ? Component.getInitialProps(ctx) : {}
-}

examples/with-loading/pages/about.js

@@ -3,10 +3,11 @@ import Header from '../components/Header'
 
 export default class About extends Component {
   // Add some delay
-  static getInitialProps () {
-    return new Promise((resolve) => {
+  static async getInitialProps () {
+    await new Promise((resolve) => {
       setTimeout(resolve, 500)
     })
+    return {}
   }
 
   render () {

examples/with-loading/pages/forever.js

@@ -3,10 +3,11 @@ import Header from '../components/Header'
 
 export default class Forever extends Component {
   // Add some delay
-  static getInitialProps () {
-    return new Promise((resolve) => {
+  static async getInitialProps () {
+    await new Promise((resolve) => {
       setTimeout(resolve, 3000)
     })
+    return {}
   }
 
   render () {

lib/router/router.js

@@ -5,6 +5,7 @@ import evalScript from '../eval-script'
 import shallowEquals from '../shallow-equals'
 import { EventEmitter } from 'events'
 import { reloadIfPrefetched } from '../prefetch'
+import { loadGetInitialProps } from '../utils'
 
 export default class Router extends EventEmitter {
   constructor (pathname, query, { Component, ErrorComponent, err } = {}) {
@@ -234,7 +235,7 @@ export default class Router extends EventEmitter {
     const cancel = () => { cancelled = true }
     this.componentLoadCancel = cancel
 
-    const props = await (Component.getInitialProps ? Component.getInitialProps(ctx) : {})
+    const props = await loadGetInitialProps(Component, ctx)
 
     if (cancel === this.componentLoadCancel) {
       this.componentLoadCancel = null

lib/utils.js

@@ -41,3 +41,15 @@ export function printAndExit (message, code = 1) {
 
   process.exit(code)
 }
+
+export async function loadGetInitialProps (Component, ctx) {
+  if (!Component.getInitialProps) return {}
+
+  const props = await Component.getInitialProps(ctx)
+  if (!props) {
+    const compName = Component.displayName || Component.name
+    const message = `"${compName}.getInitialProps()" should resolve to an object. But found "${props}" instead.`
+    throw new Error(message)
+  }
+  return props
+}

server/render.js

@@ -9,6 +9,7 @@ import requireModule from './require'
 import resolvePath from './resolve'
 import readPage from './read-page'
 import { Router } from '../lib/router'
+import { loadGetInitialProps } from '../lib/utils'
 import Head, { defaultHead } from '../lib/head'
 import App from '../lib/app'
 
@@ -52,7 +53,7 @@ async function doRender (req, res, pathname, query, {
     component,
     errorComponent
   ] = await Promise.all([
-    Component.getInitialProps ? Component.getInitialProps(ctx) : {},
+    loadGetInitialProps(Component, ctx),
     readPage(join(dir, '.next', 'bundles', 'pages', page)),
     readPage(join(dir, '.next', 'bundles', 'pages', '_error'))
   ])
@@ -80,7 +81,7 @@ async function doRender (req, res, pathname, query, {
     return { html, head }
   }
 
-  const docProps = await Document.getInitialProps({ ...ctx, renderPage })
+  const docProps = await loadGetInitialProps(Document, { ...ctx, renderPage })
 
   const doc = createElement(Document, {
     __NEXT_DATA__: {

test/integration/basic/pages/empty-get-initial-props.js

@@ -0,0 +1,4 @@
+const EmptyInitialPropsPage = () => (<div>My Page</div>)
+EmptyInitialPropsPage.getInitialProps = () => null
+
+export default EmptyInitialPropsPage

test/integration/basic/pages/nav/about.js

@@ -3,7 +3,7 @@ import Link from 'next/link'
 export default () => (
   <div className='nav-about'>
     <Link href='/nav'>
-      <a>Go Back</a>
+      <a id='home-link'>Go Back</a>
     </Link>
 
     <p>This is the about page.</p>

test/integration/basic/pages/nav/index.js

@@ -3,6 +3,10 @@ import { Component } from 'react'
 
 let counter = 0
 
+const linkStyle = {
+  marginRight: 10
+}
+
 export default class extends Component {
 
   increase () {
@@ -13,7 +17,8 @@ export default class extends Component {
   render () {
     return (
       <div className='nav-home'>
-        <Link href='/nav/about'><a>About</a></Link>
+        <Link href='/nav/about'><a id='about-link' style={linkStyle}>About</a></Link>
+        <Link href='/empty-get-initial-props'><a id='empty-props' style={linkStyle}>Empty Props</a></Link>
         <p>This is the home.</p>
         <div id='counter'>
           Counter: {counter}

test/integration/basic/test/client-navigation.js

@@ -8,7 +8,7 @@ export default (context) => {
       it('should navigate the page', async () => {
         const browser = await webdriver(context.appPort, '/nav')
         const text = await browser
-          .elementByCss('a').click()
+          .elementByCss('#about-link').click()
           .waitForElementByCss('.nav-about')
           .elementByCss('p').text()
 
@@ -21,9 +21,9 @@ export default (context) => {
 
         const counterText = await browser
           .elementByCss('#increase').click()
-          .elementByCss('a').click()
+          .elementByCss('#about-link').click()
           .waitForElementByCss('.nav-about')
-          .elementByCss('a').click()
+          .elementByCss('#home-link').click()
           .waitForElementByCss('.nav-home')
           .elementByCss('#counter').text()
 
@@ -36,13 +36,28 @@ export default (context) => {
       it('should navigate the page', async () => {
         const browser = await webdriver(context.appPort, '/nav/about')
         const text = await browser
-          .elementByCss('a').click()
+          .elementByCss('#home-link').click()
           .waitForElementByCss('.nav-home')
           .elementByCss('p').text()
 
         expect(text).toBe('This is the home.')
         await browser.close()
       })
     })
+
+    describe('with empty getInitialProps()', () => {
+      it('should render an error', async () => {
+        const browser = await webdriver(context.appPort, '/nav')
+        const preText = await browser
+          .elementByCss('#empty-props').click()
+          .waitForElementByCss('pre')
+          .elementByCss('pre').text()
+
+        const expectedErrorMessage = '"EmptyInitialPropsPage.getInitialProps()" should resolve to an object. But found "null" instead.'
+        expect(preText.includes(expectedErrorMessage)).toBeTruthy()
+
+        await browser.close()
+      })
+    })
   })
 }

test/integration/basic/test/rendering.js

@@ -55,6 +55,12 @@ export default function ({ app }, suiteName, render) {
       expect(link.text()).toBe('About')
     })
 
+    test('getInitialProps resolves to null', async () => {
+      const $ = await get$('/empty-get-initial-props')
+      const expectedErrorMessage = '"EmptyInitialPropsPage.getInitialProps()" should resolve to an object. But found "null" instead.'
+      expect($('pre').text().includes(expectedErrorMessage)).toBeTruthy()
+    })
+
     test('error', async () => {
       const $ = await get$('/error')
       expect($('pre').text()).toMatch(/This is an expected error/)