Don't discard component state on error (#741)

* render debug page as overlay

* handle errors occurrred on rendering cycle for HMR

* retrieve props if required on HMR

client/index.js

@@ -0,0 +1,77 @@
+import { createElement } from 'react'
+import ReactDOM from 'react-dom'
+import HeadManager from './head-manager'
+import { rehydrate } from '../lib/css'
+import { createRouter } from '../lib/router'
+import App from '../lib/app'
+import evalScript from '../lib/eval-script'
+
+const {
+  __NEXT_DATA__: {
+    component,
+    errorComponent,
+    props,
+    ids,
+    err,
+    pathname,
+    query
+  }
+} = window
+
+const Component = evalScript(component).default
+const ErrorComponent = evalScript(errorComponent).default
+let lastAppProps
+
+export const router = createRouter(pathname, query, {
+  Component,
+  ErrorComponent,
+  err
+})
+
+const headManager = new HeadManager()
+const container = document.getElementById('__next')
+
+export default (onError) => {
+  if (ids && ids.length) rehydrate(ids)
+
+  router.subscribe(({ Component, props, err }) => {
+    render({ Component, props, err }, onError)
+  })
+
+  render({ Component, props, err }, onError)
+}
+
+export async function render (props, onError = renderErrorComponent) {
+  try {
+    await doRender(props)
+  } catch (err) {
+    await onError(err)
+  }
+}
+
+async function renderErrorComponent (err) {
+  const { pathname, query } = router
+  const props = await getInitialProps(ErrorComponent, { err, pathname, query })
+  await doRender({ Component: ErrorComponent, props, err })
+}
+
+async function doRender ({ Component, props, err }) {
+  if (!props && Component &&
+    Component !== ErrorComponent &&
+    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 })
+  }
+
+  Component = Component || lastAppProps.Component
+  props = props || lastAppProps.props
+
+  const appProps = { Component, props, err, router, headManager }
+  lastAppProps = appProps
+  ReactDOM.render(createElement(App, appProps), container)
+}
+
+function getInitialProps (Component, ctx) {
+  return Component.getInitialProps ? Component.getInitialProps(ctx) : {}
+}

client/next-dev.js

@@ -3,10 +3,20 @@ import patch from './patch-react'
 // apply patch first
 patch((err) => {
   console.error(err)
-  next.renderError(err)
+
+  Promise.resolve().then(() => {
+    onError(err)
+  })
 })
 
 require('react-hot-loader/patch')
 
-const next = require('./next')
-window.next = next
+const next = window.next = require('./')
+
+next.default(onError)
+
+function onError (err) {
+  // just show the debug screen but don't render ErrorComponent
+  // so that the current component doesn't lose props
+  next.render({ err })
+}

client/next.js

@@ -1,59 +1,3 @@
-import { createElement } from 'react'
-import ReactDOM from 'react-dom'
-import HeadManager from './head-manager'
-import { rehydrate } from '../lib/css'
-import { createRouter } from '../lib/router'
-import App from '../lib/app'
-import evalScript from '../lib/eval-script'
+import next from './'
 
-const {
-  __NEXT_DATA__: {
-    component,
-    errorComponent,
-    props,
-    ids,
-    err,
-    pathname,
-    query
-  }
-} = window
-
-const Component = evalScript(component).default
-const ErrorComponent = evalScript(errorComponent).default
-
-export const router = createRouter(pathname, query, {
-  Component,
-  ErrorComponent,
-  ctx: { err }
-})
-
-const headManager = new HeadManager()
-const container = document.getElementById('__next')
-const defaultProps = { Component, ErrorComponent, props, router, headManager }
-
-if (ids && ids.length) rehydrate(ids)
-
-render()
-
-export function render (props = {}) {
-  try {
-    doRender(props)
-  } catch (err) {
-    renderError(err)
-  }
-}
-
-export async function renderError (err) {
-  const { pathname, query } = router
-  const props = await ErrorComponent.getInitialProps({ err, pathname, query })
-  try {
-    doRender({ Component: ErrorComponent, props })
-  } catch (err2) {
-    console.error(err2)
-  }
-}
-
-function doRender (props) {
-  const appProps = { ...defaultProps, ...props }
-  ReactDOM.render(createElement(App, appProps), container)
-}
+next()

client/webpack-hot-middleware-client.js

@@ -5,9 +5,9 @@ const handlers = {
   reload (route) {
     if (route === '/_error') {
       for (const r of Object.keys(Router.components)) {
-        const { Component } = Router.components[r]
-        if (Component.__route === '/_error-debug') {
-          // reload all '/_error-debug'
+        const { err } = Router.components[r]
+        if (err) {
+          // reload all error routes
           // which are expected to be errors of '/_error' routes
           Router.reload(r)
         }
@@ -29,8 +29,8 @@ const handlers = {
       return
     }
 
-    const { Component } = Router.components[route] || {}
-    if (Component && Component.__route === '/_error-debug') {
+    const { err } = Router.components[route] || {}
+    if (err) {
       // reload to recover from runtime errors
       Router.reload(route)
     }

lib/app.js

@@ -1,83 +1,53 @@
 import React, { Component, PropTypes } from 'react'
 import { AppContainer } from 'react-hot-loader'
+import shallowEquals from './shallow-equals'
 import { warn } from './utils'
 
+const ErrorDebug = process.env.NODE_ENV === 'production'
+  ? null : require('./error-debug').default
+
 export default class App extends Component {
   static childContextTypes = {
-    router: PropTypes.object,
     headManager: PropTypes.object
   }
 
-  constructor (props) {
-    super(props)
-    this.state = propsToState(props)
-    this.close = null
-  }
-
-  componentWillReceiveProps (nextProps) {
-    const state = propsToState(nextProps)
-    try {
-      this.setState(state)
-    } catch (err) {
-      this.handleError(err)
-    }
-  }
-
-  componentDidMount () {
-    const { router } = this.props
-
-    this.close = router.subscribe((data) => {
-      const props = data.props || this.state.props
-      const state = propsToState({
-        ...data,
-        props,
-        router
-      })
-
-      try {
-        this.setState(state)
-      } catch (err) {
-        this.handleError(err)
-      }
-    })
-  }
-
-  componentWillUnmount () {
-    if (this.close) this.close()
-  }
-
   getChildContext () {
-    const { router, headManager } = this.props
-    return { router, headManager }
+    const { headManager } = this.props
+    return { headManager }
   }
 
   render () {
-    const { Component, props } = this.state
+    const { Component, props, err, router } = this.props
+    const containerProps = { Component, props, router }
 
+    return <div>
+      <Container {...containerProps} />
+      {ErrorDebug && err ? <ErrorDebug err={err} /> : null}
+    </div>
+  }
+
+}
+
+class Container extends Component {
+  shouldComponentUpdate (nextProps) {
+    // need this check not to rerender component which has already thrown an error
+    return !shallowEquals(this.props, nextProps)
+  }
+
+  render () {
+    const { Component, props, router } = this.props
+    const url = createUrl(router)
+
+    // includes AppContainer which bypasses shouldComponentUpdate method
+    // https://github.com/gaearon/react-hot-loader/issues/442
     return <AppContainer>
-      <Component {...props} />
+      <Component {...props} url={url} />
     </AppContainer>
   }
-
-  async handleError (err) {
-    console.error(err)
-
-    const { router, ErrorComponent } = this.props
-    const { pathname, query } = router
-    const props = await ErrorComponent.getInitialProps({ err, pathname, query })
-    const state = propsToState({ Component: ErrorComponent, props, router })
-
-    try {
-      this.setState(state)
-    } catch (err2) {
-      console.error(err2)
-    }
-  }
 }
 
-function propsToState (props) {
-  const { Component, router } = props
-  const url = {
+function createUrl (router) {
+  return {
     query: router.query,
     pathname: router.pathname,
     back: () => router.back(),
@@ -98,9 +68,4 @@ function propsToState (props) {
       return router.replace(replaceRoute, replaceUrl)
     }
   }
-
-  return {
-    Component,
-    props: { ...props.props, url }
-  }
 }

lib/error-debug.js

@@ -0,0 +1,67 @@
+import React from 'react'
+import ansiHTML from 'ansi-html'
+import Head from './head'
+
+export default ({ err: { name, message, stack, module } }) => (
+  <div style={styles.errorDebug}>
+    <Head>
+      <meta name='viewport' content='width=device-width, initial-scale=1.0' />
+    </Head>
+    {module ? <div style={styles.heading}>Error in {module.rawRequest}</div> : null}
+    {
+      name === 'ModuleBuildError'
+      ? <pre style={styles.message} dangerouslySetInnerHTML={{ __html: ansiHTML(encodeHtml(message)) }} />
+      : <pre style={styles.message}>{stack}</pre>
+    }
+  </div>
+)
+
+const styles = {
+  errorDebug: {
+    background: '#a6004c',
+    boxSizing: 'border-box',
+    overflow: 'auto',
+    padding: '16px',
+    position: 'fixed',
+    left: 0,
+    right: 0,
+    top: 0,
+    bottom: 0,
+    zIndex: 9999
+  },
+
+  message: {
+    fontFamily: '"SF Mono", "Roboto Mono", "Fira Mono", menlo-regular, monospace',
+    fontSize: '10px',
+    color: '#fbe7f1',
+    margin: 0,
+    whiteSpace: 'pre-wrap',
+    wordWrap: 'break-word'
+  },
+
+  heading: {
+    fontFamily: '-apple-system, BlinkMacSystemFont, Roboto, "Segoe UI", "Fira Sans", Avenir, "Helvetica Neue", "Lucida Grande", sans-serif',
+    fontSize: '13px',
+    fontWeight: 'bold',
+    color: '#ff84bf',
+    marginBottom: '20px'
+  }
+}
+
+const encodeHtml = str => {
+  return str.replace(/</g, '&lt;').replace(/>/g, '&gt;')
+}
+
+// see color definitions of babel-code-frame:
+// https://github.com/babel/babel/blob/master/packages/babel-code-frame/src/index.js
+
+ansiHTML.setColors({
+  reset: ['fff', 'a6004c'],
+  darkgrey: 'e54590',
+  yellow: 'ee8cbb',
+  green: 'f2a2c7',
+  magenta: 'fbe7f1',
+  blue: 'fff',
+  cyan: 'ef8bb9',
+  red: 'fff'
+})

lib/router/router.js

@@ -7,13 +7,13 @@ import { EventEmitter } from 'events'
 import { reloadIfPrefetched } from '../prefetch'
 
 export default class Router extends EventEmitter {
-  constructor (pathname, query, { Component, ErrorComponent, ctx } = {}) {
+  constructor (pathname, query, { Component, ErrorComponent, err } = {}) {
     super()
     // represents the current component key
     this.route = toRoute(pathname)
 
     // set up the component cache (by route keys)
-    this.components = { [this.route]: { Component, ctx } }
+    this.components = { [this.route]: { Component, err } }
 
     this.ErrorComponent = ErrorComponent
     this.pathname = pathname
@@ -168,17 +168,18 @@ export default class Router extends EventEmitter {
     const routeInfo = {}
 
     try {
-      const data = routeInfo.data = await this.fetchComponent(route)
-      const ctx = { ...data.ctx, pathname, query }
-      routeInfo.props = await this.getInitialProps(data.Component, ctx)
+      const { Component, err, xhr } = routeInfo.data = await this.fetchComponent(route)
+      const ctx = { err, xhr, pathname, query }
+      routeInfo.props = await this.getInitialProps(Component, ctx)
     } catch (err) {
       if (err.cancelled) {
         return { error: err }
       }
 
-      const data = routeInfo.data = { Component: this.ErrorComponent, ctx: { err } }
-      const ctx = { ...data.ctx, pathname, query }
-      routeInfo.props = await this.getInitialProps(data.Component, ctx)
+      const Component = this.ErrorComponent
+      routeInfo.data = { Component, err }
+      const ctx = { err, pathname, query }
+      routeInfo.props = await this.getInitialProps(Component, ctx)
 
       routeInfo.error = err
       console.error(err)
@@ -215,10 +216,7 @@ export default class Router extends EventEmitter {
         const url = `/_next/${__NEXT_DATA__.buildId}/pages${route}`
         const xhr = loadComponent(url, (err, data) => {
           if (err) return reject(err)
-          resolve({
-            Component: data.Component,
-            ctx: { xhr, err: data.err }
-          })
+          resolve({ ...data, xhr })
         })
       })
 

pages/_error-debug.js

@@ -1,74 +0,0 @@
-import React from 'react'
-import Head from 'next/head'
-import ansiHTML from 'ansi-html'
-
-export default class ErrorDebug extends React.Component {
-  static getInitialProps ({ err }) {
-    const { name, message, stack, module } = err
-    return { name, message, stack, path: module ? module.rawRequest : null }
-  }
-
-  render () {
-    const { name, message, stack, path } = this.props
-
-    return <div className='errorDebug'>
-      <Head>
-        <meta name='viewport' content='width=device-width, initial-scale=1.0' />
-      </Head>
-      {path ? <div className='heading'>Error in {path}</div> : null}
-      {
-        name === 'ModuleBuildError'
-        ? <pre className='message' dangerouslySetInnerHTML={{ __html: ansiHTML(encodeHtml(message)) }} />
-        : <pre className='message'>{stack}</pre>
-      }
-      <style jsx global>{`
-        body {
-          background: #a6004c;
-          margin: 0;
-        }
-      `}</style>
-      <style jsx>{`
-        .errorDebug {
-          height: 100%;
-          padding: 16px;
-          box-sizing: border-box;
-        }
-
-        .message {
-          font-family: "SF Mono", "Roboto Mono", "Fira Mono", menlo-regular, monospace;
-          font-size: 10px;
-          color: #fbe7f1;
-          margin: 0;
-          white-space: pre-wrap;
-          word-wrap: break-word;
-        }
-
-        .heading {
-          font-family: -apple-system, BlinkMacSystemFont, Roboto, "Segoe UI", "Fira Sans", Avenir, "Helvetica Neue", "Lucida Grande", sans-serif;
-          font-size: 13px;
-          font-weight: bold;
-          color: #ff84bf;
-          margin-bottom: 20pxl
-        }
-      `}</style>
-    </div>
-  }
-}
-
-const encodeHtml = str => {
-  return str.replace(/</g, '&lt;').replace(/>/g, '&gt;')
-}
-
-// see color definitions of babel-code-frame:
-// https://github.com/babel/babel/blob/master/packages/babel-code-frame/src/index.js
-
-ansiHTML.setColors({
-  reset: ['fff', 'a6004c'],
-  darkgrey: 'e54590',
-  yellow: 'ee8cbb',
-  green: 'f2a2c7',
-  magenta: 'fbe7f1',
-  blue: 'fff',
-  cyan: 'ef8bb9',
-  red: 'fff'
-})

server/build/webpack.js

@@ -17,7 +17,6 @@ import getConfig from '../config'
 const documentPage = join('pages', '_document.js')
 const defaultPages = [
   '_error.js',
-  '_error-debug.js',
   '_document.js'
 ]
 

server/render.js

@@ -27,8 +27,7 @@ export async function renderError (err, req, res, pathname, query, opts) {
 }
 
 export function renderErrorToHTML (err, req, res, pathname, query, opts = {}) {
-  const page = err && opts.dev ? '/_error-debug' : '/_error'
-  return doRender(req, res, pathname, query, { ...opts, err, page })
+  return doRender(req, res, pathname, query, { ...opts, err, page: '_error' })
 }
 
 async function doRender (req, res, pathname, query, {
@@ -55,7 +54,7 @@ async function doRender (req, res, pathname, query, {
   ] = await Promise.all([
     Component.getInitialProps ? Component.getInitialProps(ctx) : {},
     readPage(join(dir, '.next', 'bundles', 'pages', page)),
-    readPage(join(dir, '.next', 'bundles', 'pages', dev ? '_error-debug' : '_error'))
+    readPage(join(dir, '.next', 'bundles', 'pages', '_error'))
   ])
 
   // the response might be finshed on the getinitialprops call
@@ -65,6 +64,7 @@ async function doRender (req, res, pathname, query, {
     const app = createElement(App, {
       Component,
       props,
+      err,
       router: new Router(pathname, query)
     })
 
@@ -106,8 +106,7 @@ export async function renderJSON (req, res, page, { dir = process.cwd() } = {})
 }
 
 export async function renderErrorJSON (err, req, res, { dir = process.cwd(), dev = false } = {}) {
-  const page = err && dev ? '/_error-debug' : '/_error'
-  const component = await readPage(join(dir, '.next', 'bundles', 'pages', page))
+  const component = await readPage(join(dir, '.next', 'bundles', 'pages', '_error'))
 
   sendJSON(res, {
     component,

test/integration.test.js

@@ -63,7 +63,7 @@ describe('integration tests', () => {
 
   test('error', async () => {
     const html = await render('/error')
-    expect(html).toMatch(/<pre class=".+">Error: This is an expected error\n[^]+<\/pre>/)
+    expect(html).toMatch(/<pre style=".+">Error: This is an expected error\n[^]+<\/pre>/)
   })
 
   test('error 404', async () => {