Skip to content

Commit

Permalink
Make sure lastAppProps always have some value. (#829)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
arunoda authored and rauchg committed Jan 20, 2017
1 parent 318f110 commit 399e510
Show file tree
Hide file tree
Showing 11 changed files with 62 additions and 19 deletions.
9 changes: 3 additions & 6 deletions client/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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__: {
Expand Down Expand Up @@ -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 })
}

Expand All @@ -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
Expand All @@ -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) : {}
}
5 changes: 3 additions & 2 deletions examples/with-loading/pages/about.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand Down
5 changes: 3 additions & 2 deletions examples/with-loading/pages/forever.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand Down
3 changes: 2 additions & 1 deletion lib/router/router.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 } = {}) {
Expand Down Expand Up @@ -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
Expand Down
12 changes: 12 additions & 0 deletions lib/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
5 changes: 3 additions & 2 deletions server/render.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Expand Down Expand Up @@ -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'))
])
Expand Down Expand Up @@ -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__: {
Expand Down
4 changes: 4 additions & 0 deletions test/integration/basic/pages/empty-get-initial-props.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
const EmptyInitialPropsPage = () => (<div>My Page</div>)
EmptyInitialPropsPage.getInitialProps = () => null

export default EmptyInitialPropsPage
2 changes: 1 addition & 1 deletion test/integration/basic/pages/nav/about.js
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
7 changes: 6 additions & 1 deletion test/integration/basic/pages/nav/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@ import { Component } from 'react'

let counter = 0

const linkStyle = {
marginRight: 10
}

export default class extends Component {

increase () {
Expand All @@ -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}
Expand Down
23 changes: 19 additions & 4 deletions test/integration/basic/test/client-navigation.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand All @@ -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()

Expand All @@ -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()
})
})
})
}
6 changes: 6 additions & 0 deletions test/integration/basic/test/rendering.js
Original file line number Diff line number Diff line change
Expand Up @@ -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/)
Expand Down

0 comments on commit 399e510

Please sign in to comment.