Merge branch 'pre-6.x' into master

package.json

@@ -46,8 +46,7 @@
     "hoist-non-react-statics": "^2.5.5",
     "invariant": "^2.2.4",
     "loose-envify": "^1.1.0",
-    "prop-types": "^15.6.1",
-    "react-lifecycles-compat": "^3.0.0"
+    "prop-types": "^15.6.1"
   },
   "devDependencies": {
     "babel-cli": "^6.26.0",
@@ -89,8 +88,6 @@
     "jest": "^23.4.1",
     "jest-dom": "^1.12.0",
     "npm-run": "^5.0.1",
-    "react": "^16.3.2",
-    "react-dom": "^16.3.2",
     "react-testing-library": "^5.0.0",
     "redux": "^4.0.0",
     "rimraf": "^2.6.2",

src/components/connectAdvanced.js

@@ -1,35 +1,12 @@
 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
 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,
-        }
-      }
-      return {
-        shouldComponentUpdate: false,
-      }
-    } catch (error) {
-      return {
-        shouldComponentUpdate: true,
-        error,
-      }
-    }
-  }
-}
 
 export default function connectAdvanced(
   /*
@@ -88,10 +65,6 @@ export default function connectAdvanced(
     [subscriptionKey]: subscriptionShape,
   }
 
-  function getDerivedStateFromProps(nextProps, prevState) {
-    return prevState.updater(nextProps, prevState)
-  }
-
   return function wrapWithConnect(WrappedComponent) {
     invariant(
       typeof WrappedComponent == 'function',
@@ -134,10 +107,14 @@ export default function connectAdvanced(
           `or explicitly pass "${storeKey}" as a prop to "${displayName}".`
         )
 
+        this.createSelector()
         this.state = {
-          updater: this.createUpdater()
+          updateCount: 0
         }
+        this.storeState = this.store.getState()
         this.initSubscription()
+        this.derivedProps = this.derivedPropsUpdater()
+        this.received = this.props
       }
 
       getChildContext() {
@@ -159,11 +136,17 @@ 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.triggerUpdateOnStoreStateChange()
       }
 
-      shouldComponentUpdate(_, nextState) {
-        return nextState.shouldComponentUpdate
+      shouldComponentUpdate(nextProps) {
+        this.received = nextProps
+        // received a prop update, store state updates are handled in onStateChange
+        const oldProps = this.derivedProps
+        const newProps = this.updateDerivedProps(nextProps)
+        if (this.error) return true
+        const sCU = newProps !== oldProps
+        return sCU
       }
 
       componentWillUnmount() {
@@ -174,6 +157,31 @@ export default function connectAdvanced(
         this.isUnmounted = true
       }
 
+      updateDerivedProps(nextProps) {
+        this.derivedProps = this.derivedPropsUpdater(nextProps)
+        return this.derivedProps
+      }
+
+      derivedPropsUpdater(props = this.props) {
+        // runs when props change, or the store state changes
+        // and generates the derived props for connected components
+        try {
+          const nextProps = this.sourceSelector(this.storeState, props)
+          if (nextProps !== this.derivedProps || this.error) {
+            this.error = null
+            return nextProps
+          }
+          return this.derivedProps
+        } catch (error) {
+          this.error = error
+          return this.derivedProps
+        }
+      }
+
+      createSelector() {
+        this.sourceSelector = selectorFactory(this.store.dispatch, selectorFactoryOptions)
+      }
+
       getWrappedInstance() {
         invariant(withRef,
           `To access the wrapped instance, you need to specify ` +
@@ -186,17 +194,24 @@ export default function connectAdvanced(
         this.wrappedInstance = ref
       }
 
-      createUpdater() {
-        const sourceSelector = selectorFactory(this.store.dispatch, selectorFactoryOptions)
-        return makeUpdater(sourceSelector, this.store)
-      }
-
-      runUpdater(callback = noop) {
+      triggerUpdateOnStoreStateChange(callback = noop) {
+        // runs when an action is dispatched by the store we are listening to
+        // if the store state has changed, we save that and update the component state
+        // to force a re-generation of derived props
         if (this.isUnmounted) {
           return
         }
 
-        this.setState(prevState => prevState.updater(this.props, prevState), callback)
+        this.setState(prevState => {
+          const newState = this.store.getState()
+          if (this.storeState === newState) {
+            return prevState
+          }
+          this.storeState = newState
+          return {
+            updateCount: prevState.updateCount++
+          }
+        }, callback)
       }
 
       initSubscription() {
@@ -217,7 +232,7 @@ export default function connectAdvanced(
       }
 
       onStateChange() {
-        this.runUpdater(this.notifyNestedSubs)
+        this.triggerUpdateOnStoreStateChange(this.notifyNestedSubs)
       }
 
       isSubscribed() {
@@ -238,10 +253,16 @@ export default function connectAdvanced(
       }
 
       render() {
-        if (this.state.error) {
-          throw this.state.error
+        if (this.received !== this.props) {
+          // forceUpdate() was called on this component, which skips sCU
+          // so manually update derived props
+          this.received = this.props
+          this.updateDerivedProps(this.props)
+        }
+        if (this.error) {
+          throw this.error
         } else {
-          return createElement(WrappedComponent, this.addExtraProps(this.state.props))
+          return createElement(WrappedComponent, this.addExtraProps(this.derivedProps))
         }
       }
     }
@@ -251,7 +272,6 @@ export default function connectAdvanced(
     Connect.childContextTypes = childContextTypes
     Connect.contextTypes = contextTypes
     Connect.propTypes = contextTypes
-    Connect.getDerivedStateFromProps = getDerivedStateFromProps
 
     if (process.env.NODE_ENV !== 'production') {
       Connect.prototype.componentDidUpdate = function componentDidUpdate() {
@@ -276,15 +296,12 @@ export default function connectAdvanced(
             oldListeners.forEach(listener => this.subscription.listeners.subscribe(listener))
           }
 
-          const updater = this.createUpdater()
-          this.setState({updater})
-          this.runUpdater()
+          this.createSelector()
+          this.triggerUpdateOnStoreStateChange()
         }
       }
     }
 
-    polyfill(Connect)
-
     return hoistStatics(Connect, WrappedComponent)
   }
 }

test/components/Provider.spec.js

@@ -214,10 +214,12 @@ describe('React', () => {
       }
     }
 
+    const childCalls = []
     @connect((state, parentProps) => {
       childMapStateInvokes++
+      childCalls.push([state, parentProps.parentState])
       // The state from parent props should always be consistent with the current state
-      expect(state).toEqual(parentProps.parentState)
+      //expect(state).toEqual(parentProps.parentState)
       return {}
     })
     class ChildContainer extends Component {
@@ -236,16 +238,31 @@ describe('React', () => {
 
     // The store state stays consistent when setState calls are batched
     store.dispatch({ type: 'APPEND', body: 'c' })
-    expect(childMapStateInvokes).toBe(2)
+    expect(childMapStateInvokes).toBe(3)
+    expect(childCalls).toEqual([
+      ['a',  'a'],
+      ['a',  'ac'], // parent updates first, passes props
+      ['ac', 'ac'] // then store update is processed
+    ])
 
     // setState calls DOM handlers are batched
+
     const button = tester.getByText('change')
     rtl.fireEvent.click(button)
     expect(childMapStateInvokes).toBe(3)
 
     // Provider uses unstable_batchedUpdates() under the hood
     store.dispatch({ type: 'APPEND', body: 'd' })
-    expect(childMapStateInvokes).toBe(4)
+    expect(childCalls).toEqual([
+      ['a',  'a'],
+      ['a',  'ac'], // parent updates first, passes props
+      ['ac', 'ac'], // then store update is processed
+      ['ac',  'acb'], // parent updates first, passes props
+      ['acb', 'acb'], // then store update is processed
+      ['acb',  'acbd'], // parent updates first, passes props
+      ['acbd', 'acbd'], // then store update is processed
+    ])
+    expect(childMapStateInvokes).toBe(7)
   })
 
   it('works in <StrictMode> without warnings (React 16.3+)', () => {

test/components/connect.spec.js

@@ -1773,10 +1773,12 @@ describe('React', () => {
         }
       }
 
+      const childCalls = []
       @connect((state, parentProps) => {
         childMapStateInvokes++
+        childCalls.push([state, parentProps.parentState])
         // The state from parent props should always be consistent with the current state
-        expect(state).toEqual(parentProps.parentState)
+        //expect(state).toEqual(parentProps.parentState)
         return {}
       })
       class ChildContainer extends Component {
@@ -1792,20 +1794,37 @@ describe('React', () => {
       )
 
       expect(childMapStateInvokes).toBe(1)
+      expect(childCalls).toEqual([
+        ['a', 'a']
+      ])
 
       // The store state stays consistent when setState calls are batched
       ReactDOM.unstable_batchedUpdates(() => {
         store.dispatch({ type: 'APPEND', body: 'c' })
       })
-      expect(childMapStateInvokes).toBe(2)
+      expect(childMapStateInvokes).toBe(3)
+      expect(childCalls).toEqual([
+        ['a', 'a'],
+        ['a', 'ac'],
+        ['ac', 'ac'],
+      ])
 
       // setState calls DOM handlers are batched
       const button = tester.getByText('change')
       rtl.fireEvent.click(button)
       expect(childMapStateInvokes).toBe(3)
 
       store.dispatch({ type: 'APPEND', body: 'd' })
-      expect(childMapStateInvokes).toBe(4)
+      expect(childMapStateInvokes).toBe(7)
+      expect(childCalls).toEqual([
+        ['a', 'a'],
+        ['a', 'ac'],
+        ['ac', 'ac'],
+        ['ac', 'acb'],
+        ['acb', 'acb'],
+        ['acb', 'acbd'],
+        ['acbd', 'acbd'],
+      ])
     })
 
     it('should not render the wrapped component when mapState does not produce change', () => {
@@ -2021,7 +2040,7 @@ describe('React', () => {
         return { ...stateProps, ...dispatchProps, name: parentProps.name }
       }
 
-      @connect(null, mapDispatchFactory, mergeParentDispatch)
+      @connect(() => ({}), mapDispatchFactory, mergeParentDispatch)
       class Passthrough extends Component {
         componentDidUpdate() {
           updatedCount++
@@ -2281,8 +2300,9 @@ describe('React', () => {
       @connect() // no mapStateToProps. therefore it should be transparent for subscriptions
       class B extends React.Component { render() { return <C {...this.props} /> }}
 
+      let calls = []
       @connect((state, props) => {
-        expect(props.count).toBe(state)
+        calls.push([state, props.count])
         return { count: state * 10 + props.count }
       })
       class C extends React.Component { render() { return <div>{this.props.count}</div> }}
@@ -2291,6 +2311,12 @@ describe('React', () => {
       rtl.render(<ProviderMock store={store}><A /></ProviderMock>)
 
       store.dispatch({ type: 'INC' })
+
+      expect(calls).toEqual([
+        [0, 0],
+        [0, 1], // props updates first
+        [1, 1], // then state
+      ])
     })
 
     it('should subscribe properly when a new store is provided via props', () => {