Merge pull request #5 from reduxjs/master

merge in master

Author: cellog

package-lock.json

@@ -33,7 +33,7 @@
     "build:umd:min": "cross-env BABEL_ENV=rollup NODE_ENV=production rollup -c -o dist/react-redux.min.js",
     "build": "npm run build:commonjs && npm run build:es && npm run build:umd && npm run build:umd:min",
     "clean": "rimraf lib dist es coverage",
-    "lint": "eslint src test/utils test/components test/getTestDeps.js",
+    "lint": "eslint src test/utils test/components",
     "prepare": "npm run clean && npm run build",
     "test": "node ./test/run-tests.js",
     "coverage": "codecov"
@@ -81,18 +81,17 @@
     "create-react-class": "^15.6.3",
     "cross-env": "^5.2.0",
     "cross-spawn": "^6.0.5",
-    "enzyme": "^3.3.0",
-    "enzyme-adapter-react-16": "^1.1.1",
     "es3ify": "^0.2.0",
     "eslint": "^4.19.1",
     "eslint-plugin-import": "^2.12.0",
     "eslint-plugin-react": "^7.9.1",
     "glob": "^7.1.1",
     "jest": "^23.4.1",
+    "jest-dom": "^1.12.0",
     "npm-run": "^5.0.1",
-    "react": "^16.3.2",
-    "react-dom": "^16.3.2",
-    "react-test-renderer": "^16.3.2",
+    "react": "^16.4.2",
+    "react-dom": "^16.4.2",
+    "react-testing-library": "^5.0.0",
     "redux": "^4.0.0",
     "rimraf": "^2.6.2",
     "rollup": "^0.61.1",

package.json

@@ -38,8 +38,8 @@ export function createProvider(storeKey = 'store') {
     }
 
     if (process.env.NODE_ENV !== 'production') {
-      Provider.prototype.componentDidUpdate = function () {
-        if (this[storeKey] !== this.props.store) {
+      Provider.prototype.componentWillReceiveProps = function (nextProps) {
+        if (this[storeKey] !== nextProps.store) {
           warnAboutReceivingStore()
         }
       }

src/components/Provider.js

@@ -1,34 +1,32 @@
 import hoistStatics from 'hoist-non-react-statics'
 import invariant from 'invariant'
 import { Component, createElement } from 'react'
-import { polyfill } from 'react-lifecycles-compat'
 
 import Subscription from '../utils/Subscription'
 import { storeShape, subscriptionShape } from '../utils/PropTypes'
 
 let hotReloadingVersion = 0
+const dummyState = {}
 function noop() {}
-function makeUpdater(sourceSelector, store) {
-  return function updater(props, prevState) {
-    try {
-      const nextProps = sourceSelector(store.getState(), props)
-      if (nextProps !== prevState.props || prevState.error) {
-        return {
-          shouldComponentUpdate: true,
-          props: nextProps,
-          error: null,
+function makeSelectorStateful(sourceSelector, store) {
+  // wrap the selector in an object that tracks its results between runs.
+  const selector = {
+    run: function runComponentSelector(props) {
+      try {
+        const nextProps = sourceSelector(store.getState(), props)
+        if (nextProps !== selector.props || selector.error) {
+          selector.shouldComponentUpdate = true
+          selector.props = nextProps
+          selector.error = null
         }
-      }
-      return {
-        shouldComponentUpdate: false,
-      }
-    } catch (error) {
-      return {
-        shouldComponentUpdate: true,
-        error,
+      } catch (error) {
+        selector.shouldComponentUpdate = true
+        selector.error = error
       }
     }
   }
+
+  return selector
 }
 
 export default function connectAdvanced(
@@ -88,10 +86,6 @@ export default function connectAdvanced(
     [subscriptionKey]: subscriptionShape,
   }
 
-  function getDerivedStateFromProps(nextProps, prevState) {
-    return prevState.updater(nextProps, prevState)
-  }
-
   return function wrapWithConnect(WrappedComponent) {
     invariant(
       typeof WrappedComponent == 'function',
@@ -118,11 +112,15 @@ export default function connectAdvanced(
       WrappedComponent
     }
 
+    // TODO Actually fix our use of componentWillReceiveProps
+    /* eslint-disable react/no-deprecated */
+
     class Connect extends Component {
       constructor(props, context) {
         super(props, context)
 
         this.version = version
+        this.state = {}
         this.renderCount = 0
         this.store = props[storeKey] || context[storeKey]
         this.propsMode = Boolean(props[storeKey])
@@ -134,9 +132,7 @@ export default function connectAdvanced(
           `or explicitly pass "${storeKey}" as a prop to "${displayName}".`
         )
 
-        this.state = {
-          updater: this.createUpdater()
-        }
+        this.initSelector()
         this.initSubscription()
       }
 
@@ -159,19 +155,25 @@ export default function connectAdvanced(
         // dispatching an action in its componentWillMount, we have to re-run the select and maybe
         // re-render.
         this.subscription.trySubscribe()
-        this.runUpdater()
+        this.selector.run(this.props)
+        if (this.selector.shouldComponentUpdate) this.forceUpdate()
+      }
+
+      componentWillReceiveProps(nextProps) {
+        this.selector.run(nextProps)
       }
 
-      shouldComponentUpdate(_, nextState) {
-        return nextState.shouldComponentUpdate
+      shouldComponentUpdate() {
+        return this.selector.shouldComponentUpdate
       }
 
       componentWillUnmount() {
         if (this.subscription) this.subscription.tryUnsubscribe()
         this.subscription = null
         this.notifyNestedSubs = noop
         this.store = null
-        this.isUnmounted = true
+        this.selector.run = noop
+        this.selector.shouldComponentUpdate = false
       }
 
       getWrappedInstance() {
@@ -186,17 +188,10 @@ export default function connectAdvanced(
         this.wrappedInstance = ref
       }
 
-      createUpdater() {
+      initSelector() {
         const sourceSelector = selectorFactory(this.store.dispatch, selectorFactoryOptions)
-        return makeUpdater(sourceSelector, this.store)
-      }
-
-      runUpdater(callback = noop) {
-        if (this.isUnmounted) {
-          return
-        }
-
-        this.setState(prevState => prevState.updater(this.props, prevState), callback)
+        this.selector = makeSelectorStateful(sourceSelector, this.store)
+        this.selector.run(this.props)
       }
 
       initSubscription() {
@@ -217,7 +212,24 @@ export default function connectAdvanced(
       }
 
       onStateChange() {
-        this.runUpdater(this.notifyNestedSubs)
+        this.selector.run(this.props)
+
+        if (!this.selector.shouldComponentUpdate) {
+          this.notifyNestedSubs()
+        } else {
+          this.componentDidUpdate = this.notifyNestedSubsOnComponentDidUpdate
+          this.setState(dummyState)
+        }
+      }
+
+      notifyNestedSubsOnComponentDidUpdate() {
+        // `componentDidUpdate` is conditionally implemented when `onStateChange` determines it
+        // needs to notify nested subs. Once called, it unimplements itself until further state
+        // changes occur. Doing it this way vs having a permanent `componentDidUpdate` that does
+        // a boolean check every time avoids an extra method call most of the time, resulting
+        // in some perf boost.
+        this.componentDidUpdate = undefined
+        this.notifyNestedSubs()
       }
 
       isSubscribed() {
@@ -238,26 +250,31 @@ export default function connectAdvanced(
       }
 
       render() {
-        if (this.state.error) {
-          throw this.state.error
+        const selector = this.selector
+        selector.shouldComponentUpdate = false
+
+        if (selector.error) {
+          throw selector.error
         } else {
-          return createElement(WrappedComponent, this.addExtraProps(this.state.props))
+          return createElement(WrappedComponent, this.addExtraProps(selector.props))
         }
       }
     }
 
+    /* eslint-enable react/no-deprecated */
+
     Connect.WrappedComponent = WrappedComponent
     Connect.displayName = displayName
     Connect.childContextTypes = childContextTypes
     Connect.contextTypes = contextTypes
     Connect.propTypes = contextTypes
-    Connect.getDerivedStateFromProps = getDerivedStateFromProps
 
     if (process.env.NODE_ENV !== 'production') {
-      Connect.prototype.componentDidUpdate = function componentDidUpdate() {
+      Connect.prototype.componentWillUpdate = function componentWillUpdate() {
         // We are hot reloading!
         if (this.version !== version) {
           this.version = version
+          this.initSelector()
 
           // If any connected descendants don't hot reload (and resubscribe in the process), their
           // listeners will be lost when we unsubscribe. Unfortunately, by copying over all
@@ -275,16 +292,10 @@ export default function connectAdvanced(
             this.subscription.trySubscribe()
             oldListeners.forEach(listener => this.subscription.listeners.subscribe(listener))
           }
-
-          const updater = this.createUpdater()
-          this.setState({updater})
-          this.runUpdater()
         }
       }
     }
 
-    polyfill(Connect)
-
     return hoistStatics(Connect, WrappedComponent)
   }
 }