improve performance (avoid unnecessary re-renders)

package.json

@@ -1,6 +1,6 @@
 {
   "name": "ion-router",
-  "version": "0.11.1",
+  "version": "0.11.2",
   "description": "elegant powerful routing based on the simplicity of storing url as state",
   "main": "lib/index.js",
   "homepage": "https://cellog.github.io/ion-router",

src/NullComponent.jsx

@@ -0,0 +1,29 @@
+import React, { PureComponent } from 'react'
+import PropTypes from 'prop-types'
+
+export default function NullComponent(Loading, Component, ElseComponent, debug, cons = console) {
+  class Toggle extends PureComponent {
+    static propTypes = {
+      '@@__loaded': PropTypes.bool,
+      '@@__isActive': PropTypes.bool
+    }
+
+    constructor(props) {
+      super(props)
+      this.rendered = 0
+    }
+
+    render() {
+      this.rendered++
+      const { '@@__loaded': loadedProp, ...nullProps } = this.props
+      if (debug) {
+        cons.log(`Toggle: loaded: ${loadedProp}, active: ${nullProps['@@__isActive']}`)
+        cons.log('Loading component', Loading, 'Component', Component, 'Else', ElseComponent)
+      }
+      return !loadedProp ? <Loading {...nullProps} />  // eslint-disable-line
+        : (nullProps['@@__isActive'] ? <Component {...nullProps} /> : <ElseComponent {...nullProps} />)
+    }
+  }
+
+  return Toggle
+}

src/Toggle.jsx

@@ -1,7 +1,7 @@
-import React from 'react'
-import PropTypes from 'prop-types'
+import React, { Component as ReactComponent } from 'react'
 
 import DisplaysChildren from './DisplaysChildren'
+import NullComponent from './NullComponent'
 
 export const error = () => {
   throw new Error('call connectToggle with the connect function from react-redux to ' +
@@ -14,35 +14,6 @@ export function connectToggle(c) {
   connect = c
 }
 
-export const NullComponent = (Loading, Component, ElseComponent, debug, cons = console) => {
-  if (process.env.NODE_ENV !== 'production') {
-    const Toggle = ({ '@@__loaded': loadedProp, ...nullProps }) => {
-      if (debug) {
-        cons.log(`Toggle: loaded: ${loadedProp}, active: ${nullProps['@@__isActive']}`)
-        cons.log('Loading component', Loading, 'Component', Component, 'Else', ElseComponent)
-      }
-      return !loadedProp ? <Loading {...nullProps} />  // eslint-disable-line
-        : (nullProps['@@__isActive'] ? <Component {...nullProps} /> : <ElseComponent {...nullProps} />)
-    }
-
-    Toggle.propTypes = {
-      '@@__loaded': PropTypes.bool,
-      '@@__isActive': PropTypes.bool
-    }
-    return Toggle
-  }
-  const Toggle = ({ '@@__loaded': loadedProp, ...nullProps }) => (
-    !loadedProp ? <Loading {...nullProps} />  // eslint-disable-line
-      : (nullProps['@@__isActive'] ? <Component {...nullProps} /> : <ElseComponent {...nullProps} />)
-  )
-
-  Toggle.propTypes = {
-    '@@__loaded': PropTypes.bool,
-    '@@__isActive': PropTypes.bool
-  }
-  return Toggle
-}
-
 export default (isActive, loaded = () => true, componentLoadingMap = {}, debug = false, storeKey = 'store') => {
   const scaffold = (state, rProps) => {
     const loadedTest = !!loaded(state, rProps)
@@ -61,40 +32,81 @@ export default (isActive, loaded = () => true, componentLoadingMap = {}, debug =
   defaults.else.displayName = 'null'
   defaults.loadingComponent.displayName = 'null'
 
-  const lastProps = {
-    component: null,
-    else: null,
-    loadingComponent: null
-  }
+  class Toggle extends ReactComponent {
 
-  function Toggle({ component: Component = defaults.component, else: ElseComponent = defaults.else, // eslint-disable-line
-      loadingComponent: Loading = defaults.loadingComponent, children, ...props }) {  // eslint-disable-line
-    const useProps = { ...props }
-    const map = ['component', 'loadingComponent', 'else']
-    map.forEach((item) => {
-      if (componentLoadingMap[item]) {
-        useProps[item] = props[componentLoadingMap[item]]
-        useProps[componentLoadingMap[item]] = undefined
+    constructor(props) {
+      super(props)
+      this.lastComponents = {
+        component: false,
+        else: false,
+        loadingComponent: false
       }
-    })
+      this.makeHOC(props)
+      this.rendered = 0
+    }
 
-    if (Component !== lastProps.component || ElseComponent !== lastProps.else
-      || Loading !== lastProps.loadingComponent) {
-      lastProps.component = Component
-      lastProps.else = ElseComponent
-      lastProps.loadingComponent = Loading
+    makeHOC(props) {
+      const {
+        component: Component = defaults.component, else: ElseComponent = defaults.else, // eslint-disable-line
+        loadingComponent: Loading = defaults.loadingComponent // eslint-disable-line
+      } = props
+      this.lastComponents = {
+        component: Component,
+        else: ElseComponent,
+        loadingComponent: Loading
+      }
       const Switcher = NullComponent(Loading, Component, ElseComponent, debug)
-      Toggle.HOC = connect(scaffold, undefined, undefined, { storeKey })(Switcher)
+      const HOC = connect(scaffold, undefined, undefined, { storeKey })(Switcher)
       const elseName = ElseComponent.displayName || ElseComponent.name || 'Component'
       const componentName = Component.displayName || Component.name || 'Component'
       const loadingName = Loading.displayName || Loading.name || 'Component'
-      Toggle.HOC.displayName = `Toggle(component:${componentName},else:${elseName},loading:${loadingName})`
+      HOC.displayName = `Toggle(component:${componentName},else:${elseName},loading:${loadingName})`
+      this.HOC = HOC
+    }
+
+    init(newprops) {
+      const {
+        component: Component = defaults.component, else: ElseComponent = defaults.else,
+        loadingComponent: Loading = defaults.loadingComponent
+      } = newprops
+      if (Component !== this.lastComponents.component || ElseComponent !== this.lastComponents.else
+        || Loading !== this.lastComponents.loadingComponent) {
+        this.makeHOC(newprops)
+      }
     }
 
-    const HOC = Toggle.HOC
-    return (<HOC {...useProps}>
-      {children}
-    </HOC>)
+    shouldComponentUpdate(newprops) {
+      const {
+        component: Component = defaults.component, else: ElseComponent = defaults.else,
+        loadingComponent: Loading = defaults.loadingComponent, children
+      } = newprops
+      if (Component !== this.lastComponents.component || ElseComponent !== this.lastComponents.else
+        || Loading !== this.lastComponents.loadingComponent || children !== this.props.children) {
+        this.init(newprops)
+        return true
+      }
+      return false
+    }
+
+    render() {
+      this.rendered++
+      const {
+        component, else: unused, loadingComponent, children, ...props // eslint-disable-line
+      } = this.props
+      const useProps = { ...props }
+      const map = ['component', 'loadingComponent', 'else']
+      map.forEach((item) => {
+        if (componentLoadingMap[item]) {
+          useProps[item] = props[componentLoadingMap[item]]
+          useProps[componentLoadingMap[item]] = undefined
+        }
+      })
+
+      const HOC = this.HOC
+      return (<HOC {...useProps}>
+        {children}
+      </HOC>)
+    }
   }
 
   const map = ['component', 'loadingComponent', 'else']

test/Toggle.test.js

@@ -1,5 +1,6 @@
 import React from 'react'
-import Toggle, { connectToggle, error, NullComponent } from '../src/Toggle'
+import Toggle, { connectToggle, error } from '../src/Toggle'
+import NullComponent from '../src/NullComponent'
 import DisplaysChildren from '../src/DisplaysChildren'
 import { renderComponent, connect } from './test_helper'
 
@@ -94,40 +95,53 @@ describe('Toggle', () => {
     })
     it('default displayName', () => {
       const R = Toggle(() => true, () => true)
-      expect(R({}).type.displayName).eqls('Toggle(component:DisplaysChildren,else:null,loading:null)')
+      const thing = renderComponent(R)
+      expect(thing.find(R).unwrap().HOC.displayName).eqls('Toggle(component:DisplaysChildren,else:null,loading:null)')
     })
     it('uses a displayName', () => {
       const Comp = () => <div>hi</div>
       const Load = () => <div>foo</div>
       const Else = () => <div>else</div>
       const R = Toggle(() => true, () => true)
-      const thing = R({ component: Comp, loadingComponent: Load, else: Else })
-      expect(thing.type.displayName).eqls('Toggle(component:Comp,else:Else,loading:Load)')
+      const thing = renderComponent(R, { component: Comp, loadingComponent: Load, else: Else })
+      expect(thing.find(R).unwrap().HOC.displayName).eqls('Toggle(component:Comp,else:Else,loading:Load)')
     })
     it('re-generates the HOC', () => {
       const Load = () => <div>foo</div>
       const R = Toggle(() => true, () => false)
+      console.log('before')
       const container = renderComponent(R)
-      const First = R.HOC
+      console.log('after')
+      const First = container.find(R).unwrap().HOC
       expect(container.text()).eqls('')
+      console.log('before')
       container.props({ loadingComponent: Load })
-      expect(R.HOC).is.not.equal(First)
+      console.log('after')
+      expect(container.find(R).unwrap().HOC).is.not.equal(First)
       expect(container.text()).eqls('foo')
     })
     it('does not regenerate the HOC if unnecessary', () => {
       const R = Toggle(() => true, () => true)
-      expect(R.HOC).is.undefined
       const container = renderComponent(R)
-      expect(R.HOC.displayName).eqls('Toggle(component:DisplaysChildren,else:null,loading:null)')
-      const First = R.HOC
+      expect(container.find(R).unwrap().HOC.displayName).eqls('Toggle(component:DisplaysChildren,else:null,loading:null)')
+      const First = container.find(R).unwrap().HOC
       expect(container.text()).eqls('')
+      console.log('before set')
       container.props({ children: 'hi' })
-      expect(R.HOC).equals(First)
+      console.log('after set')
+      expect(container.find(R).unwrap().HOC).equals(First)
       expect(container.text()).eqls('hi')
       container.props({ children: 'foo' })
-      expect(R.HOC).equals(First)
+      expect(container.find(R).unwrap().HOC).equals(First)
       expect(container.text()).eqls('foo')
     })
+    it('does not re-render if props do not change', () => {
+      const R = Toggle(() => true, () => true)
+      const container = renderComponent(R, { foo: 'bar' })
+      expect(container.find(R).unwrap().rendered).eqls(1)
+      container.props({ foo: 'bar' })
+      expect(container.find(R).unwrap().rendered).eqls(1)
+    })
   })
   describe('NullComponent', () => {
     const tests = (type) => {
@@ -146,6 +160,16 @@ describe('Toggle', () => {
         const comp = renderComponent(tester(false), { '@@__loaded': true, '@@__isActive': false })
         expect(comp.text()).eqls('else')
       })
+      it(`${type} does not re-render if props do not change`, () => {
+        const container = renderComponent(tester(false), { '@@__loaded': true, '@@__isActive': false })
+        expect(container.find('Toggle').unwrap().rendered).eqls(1)
+        container.props({ '@@__loaded': true, '@@__isActive': false })
+        expect(container.find('Toggle').unwrap().rendered).eqls(1)
+        container.props({ '@@__loaded': true, '@@__isActive': true })
+        expect(container.find('Toggle').unwrap().rendered).eqls(2)
+        container.props({ '@@__loaded': true, '@@__isActive': true })
+        expect(container.find('Toggle').unwrap().rendered).eqls(2)
+      })
     }
     describe('production', () => {
       before(() => {