Revert "Merge branch 'pre-6.x' into master"

This reverts commit eb960c4, reversing
changes made to b255a46.

Author: cellog

package-lock.json

@@ -46,7 +46,8 @@
     "hoist-non-react-statics": "^2.5.5",
     "invariant": "^2.2.4",
     "loose-envify": "^1.1.0",
-    "prop-types": "^15.6.1"
+    "prop-types": "^15.6.1",
+    "react-lifecycles-compat": "^3.0.0"
   },
   "devDependencies": {
     "babel-cli": "^6.26.0",
@@ -88,6 +89,8 @@
     "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",

package.json

@@ -1,12 +1,35 @@
 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(
   /*
@@ -65,6 +88,10 @@ export default function connectAdvanced(
     [subscriptionKey]: subscriptionShape,
   }
 
+  function getDerivedStateFromProps(nextProps, prevState) {
+    return prevState.updater(nextProps, prevState)
+  }
+
   return function wrapWithConnect(WrappedComponent) {
     invariant(
       typeof WrappedComponent == 'function',
@@ -107,14 +134,10 @@ export default function connectAdvanced(
           `or explicitly pass "${storeKey}" as a prop to "${displayName}".`
         )
 
-        this.createSelector()
         this.state = {
-          updateCount: 0
+          updater: this.createUpdater()
         }
-        this.storeState = this.store.getState()
         this.initSubscription()
-        this.derivedProps = this.derivedPropsUpdater()
-        this.received = this.props
       }
 
       getChildContext() {
@@ -136,17 +159,11 @@ 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.triggerUpdateOnStoreStateChange()
+        this.runUpdater()
       }
 
-      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
+      shouldComponentUpdate(_, nextState) {
+        return nextState.shouldComponentUpdate
       }
 
       componentWillUnmount() {
@@ -157,31 +174,6 @@ 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 ` +
@@ -194,24 +186,17 @@ export default function connectAdvanced(
         this.wrappedInstance = ref
       }
 
-      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
+      createUpdater() {
+        const sourceSelector = selectorFactory(this.store.dispatch, selectorFactoryOptions)
+        return makeUpdater(sourceSelector, this.store)
+      }
+
+      runUpdater(callback = noop) {
         if (this.isUnmounted) {
           return
         }
 
-        this.setState(prevState => {
-          const newState = this.store.getState()
-          if (this.storeState === newState) {
-            return prevState
-          }
-          this.storeState = newState
-          return {
-            updateCount: prevState.updateCount++
-          }
-        }, callback)
+        this.setState(prevState => prevState.updater(this.props, prevState), callback)
       }
 
       initSubscription() {
@@ -232,7 +217,7 @@ export default function connectAdvanced(
       }
 
       onStateChange() {
-        this.triggerUpdateOnStoreStateChange(this.notifyNestedSubs)
+        this.runUpdater(this.notifyNestedSubs)
       }
 
       isSubscribed() {
@@ -253,16 +238,10 @@ export default function connectAdvanced(
       }
 
       render() {
-        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
+        if (this.state.error) {
+          throw this.state.error
         } else {
-          return createElement(WrappedComponent, this.addExtraProps(this.derivedProps))
+          return createElement(WrappedComponent, this.addExtraProps(this.state.props))
         }
       }
     }
@@ -272,6 +251,7 @@ 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() {
@@ -296,12 +276,15 @@ export default function connectAdvanced(
             oldListeners.forEach(listener => this.subscription.listeners.subscribe(listener))
           }
 
-          this.createSelector()
-          this.triggerUpdateOnStoreStateChange()
+          const updater = this.createUpdater()
+          this.setState({updater})
+          this.runUpdater()
         }
       }
     }
 
+    polyfill(Connect)
+
     return hoistStatics(Connect, WrappedComponent)
   }
 }

src/components/connectAdvanced.js

@@ -214,12 +214,10 @@ 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 {
@@ -238,31 +236,16 @@ describe('React', () => {
 
     // The store state stays consistent when setState calls are batched
     store.dispatch({ type: 'APPEND', body: 'c' })
-    expect(childMapStateInvokes).toBe(3)
-    expect(childCalls).toEqual([
-      ['a',  'a'],
-      ['a',  'ac'], // parent updates first, passes props
-      ['ac', 'ac'] // then store update is processed
-    ])
+    expect(childMapStateInvokes).toBe(2)
 
     // 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(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)
+    expect(childMapStateInvokes).toBe(4)
   })
 
   it('works in <StrictMode> without warnings (React 16.3+)', () => {

test/components/Provider.spec.js

@@ -1773,12 +1773,10 @@ 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 {
@@ -1794,37 +1792,20 @@ 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(3)
-      expect(childCalls).toEqual([
-        ['a', 'a'],
-        ['a', 'ac'],
-        ['ac', 'ac'],
-      ])
+      expect(childMapStateInvokes).toBe(2)
 
       // 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(7)
-      expect(childCalls).toEqual([
-        ['a', 'a'],
-        ['a', 'ac'],
-        ['ac', 'ac'],
-        ['ac', 'acb'],
-        ['acb', 'acb'],
-        ['acb', 'acbd'],
-        ['acbd', 'acbd'],
-      ])
+      expect(childMapStateInvokes).toBe(4)
     })
 
     it('should not render the wrapped component when mapState does not produce change', () => {
@@ -2040,7 +2021,7 @@ describe('React', () => {
         return { ...stateProps, ...dispatchProps, name: parentProps.name }
       }
 
-      @connect(() => ({}), mapDispatchFactory, mergeParentDispatch)
+      @connect(null, mapDispatchFactory, mergeParentDispatch)
       class Passthrough extends Component {
         componentDidUpdate() {
           updatedCount++
@@ -2300,9 +2281,8 @@ 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) => {
-        calls.push([state, props.count])
+        expect(props.count).toBe(state)
         return { count: state * 10 + props.count }
       })
       class C extends React.Component { render() { return <div>{this.props.count}</div> }}
@@ -2311,12 +2291,6 @@ 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', () => {