Remove shouldComponentUpdate

docs/troubleshooting.md

@@ -40,28 +40,6 @@ render() {
 Conveniently, this gives your components access to the router state!
 You can also upgrade to React Router 1.0 which shouldn’t have this problem. (Let us know if it does!)
 
-### My views aren’t updating when something changes outside of Redux
-
-If your views depend on global state or [React “context”](http://facebook.github.io/react/docs/context.html), you might find that views decorated with `connect()` will fail to update.
-
->This is because `connect()` implements [shouldComponentUpdate](https://facebook.github.io/react/docs/component-specs.html#updating-shouldcomponentupdate) by default, assuming that your component will produce the same results given the same props and state. This is a similar concept to React’s [PureRenderMixin](https://facebook.github.io/react/docs/pure-render-mixin.html).
-
-The _best_ solution to this is to make sure that your components are pure and pass any external state to them via props. This will ensure that your views do not re-render unless they actually need to re-render and will greatly speed up your application.
-
-If that’s not practical for whatever reason (for example, if you’re using a library that depends heavily on React context), you may pass the `pure: false` option to `connect()`:
-
-```
-function mapStateToProps(state) {
-  return { todos: state.todos }
-}
-
-export default connect(mapStateToProps, null, null, {
-  pure: false
-})(TodoApp)
-```
-
-This will remove the assumption that `TodoApp` is pure and cause it to update whenever its parent component renders. Note that this will make your application less performant, so only do this if you have no other option.
-
 ### Could not find "store" in either the context or props
 
 If you have context issues,

src/components/connectAdvanced.js

@@ -161,10 +161,6 @@ export default function connectAdvanced(
         this.selector.run(nextProps)
       }
 
-      shouldComponentUpdate() {
-        return this.selector.shouldComponentUpdate
-      }
-
       componentWillUnmount() {
         if (this.subscription) this.subscription.tryUnsubscribe()
         this.subscription = null
@@ -194,16 +190,16 @@ export default function connectAdvanced(
 
       initSubscription() {
         if (!shouldHandleStateChanges) return
-        
+
         // parentSub's source should match where store came from: props vs. context. A component
         // connected to the store via props shouldn't use subscription from context, or vice versa.
         const parentSub = (this.propsMode ? this.props : this.context)[subscriptionKey]
         this.subscription = new Subscription(this.store, parentSub, this.onStateChange.bind(this))
-        
+
         // `notifyNestedSubs` is duplicated to handle the case where the component is  unmounted in
         // the middle of the notification loop, where `this.subscription` will then be null. An
         // extra null check every change can be avoided by copying the method onto `this` and then
-        // replacing it with a no-op on unmount. This can probably be avoided if Subscription's 
+        // replacing it with a no-op on unmount. This can probably be avoided if Subscription's
         // listeners logic is changed to not call listeners that have been unsubscribed in the
         // middle of the notification loop.
         this.notifyNestedSubs = this.subscription.notifyNestedSubs.bind(this.subscription)
@@ -218,7 +214,7 @@ export default function connectAdvanced(
           this.componentDidUpdate = this.notifyNestedSubsOnComponentDidUpdate
           this.setState(dummyState)
         }
-      }      
+      }
 
       notifyNestedSubsOnComponentDidUpdate() {
         // `componentDidUpdate` is conditionally implemented when `onStateChange` determines it

src/connect/selectorFactory.js

@@ -1,5 +1,5 @@
 import verifySubselectors from './verifySubselectors'
-  
+
 export function impureFinalPropsSelectorFactory(
   mapStateToProps,
   mapDispatchToProps,
@@ -64,7 +64,7 @@ export function pureFinalPropsSelectorFactory(
     const nextStateProps = mapStateToProps(state, ownProps)
     const statePropsChanged = !areStatePropsEqual(nextStateProps, stateProps)
     stateProps = nextStateProps
-    
+
     if (statePropsChanged)
       mergedProps = mergeProps(stateProps, dispatchProps, ownProps)
 
@@ -92,11 +92,6 @@ export function pureFinalPropsSelectorFactory(
 
 // TODO: Add more comments
 
-// If pure is true, the selector returned by selectorFactory will memoize its results,
-// allowing connectAdvanced's shouldComponentUpdate to return false if final
-// props have not changed. If false, the selector will always return a new
-// object and shouldComponentUpdate will always return true.
-
 export default function finalPropsSelectorFactory(dispatch, {
   initMapStateToProps,
   initMapDispatchToProps,

test/components/connect.spec.js

@@ -6,6 +6,7 @@ import ReactDOM from 'react-dom'
 import TestUtils from 'react-addons-test-utils'
 import { createStore } from 'redux'
 import { connect } from '../../src/index'
+import isEqual from 'lodash/isEqual'
 
 describe('React', () => {
   describe('connect', () => {
@@ -398,6 +399,10 @@ describe('React', () => {
           }
         }
 
+        shouldComponentUpdate(nextProps) {
+          return !isEqual(this.props, nextProps)
+        }
+
         componentDidMount() {
           // Simulate deep object mutation
           const bar = this.state.bar
@@ -1105,94 +1110,6 @@ describe('React', () => {
       expect(spy.calls.length).toBe(3)
     })
 
-    it('should shallowly compare the merged state to prevent unnecessary updates', () => {
-      const store = createStore(stringBuilder)
-      const spy = expect.createSpy(() => ({}))
-      function render({ string, pass }) {
-        spy()
-        return <Passthrough string={string} pass={pass} passVal={pass.val} />
-      }
-
-      @connect(
-        state => ({ string: state }),
-        dispatch => ({ dispatch }),
-        (stateProps, dispatchProps, parentProps) => ({
-          ...dispatchProps,
-          ...stateProps,
-          ...parentProps
-        })
-      )
-      class Container extends Component {
-        render() {
-          return render(this.props)
-        }
-      }
-
-      class Root extends Component {
-        constructor(props) {
-          super(props)
-          this.state = { pass: '' }
-        }
-
-        render() {
-          return (
-            <ProviderMock store={store}>
-              <Container pass={this.state.pass} />
-            </ProviderMock>
-          )
-        }
-      }
-
-      const tree = TestUtils.renderIntoDocument(<Root />)
-      const stub = TestUtils.findRenderedComponentWithType(tree, Passthrough)
-      expect(spy.calls.length).toBe(1)
-      expect(stub.props.string).toBe('')
-      expect(stub.props.pass).toBe('')
-
-      store.dispatch({ type: 'APPEND', body: 'a' })
-      expect(spy.calls.length).toBe(2)
-      expect(stub.props.string).toBe('a')
-      expect(stub.props.pass).toBe('')
-
-      tree.setState({ pass: '' })
-      expect(spy.calls.length).toBe(2)
-      expect(stub.props.string).toBe('a')
-      expect(stub.props.pass).toBe('')
-
-      tree.setState({ pass: 'through' })
-      expect(spy.calls.length).toBe(3)
-      expect(stub.props.string).toBe('a')
-      expect(stub.props.pass).toBe('through')
-
-      tree.setState({ pass: 'through' })
-      expect(spy.calls.length).toBe(3)
-      expect(stub.props.string).toBe('a')
-      expect(stub.props.pass).toBe('through')
-
-      const obj = { prop: 'val' }
-      tree.setState({ pass: obj })
-      expect(spy.calls.length).toBe(4)
-      expect(stub.props.string).toBe('a')
-      expect(stub.props.pass).toBe(obj)
-
-      tree.setState({ pass: obj })
-      expect(spy.calls.length).toBe(4)
-      expect(stub.props.string).toBe('a')
-      expect(stub.props.pass).toBe(obj)
-
-      const obj2 = Object.assign({}, obj, { val: 'otherval' })
-      tree.setState({ pass: obj2 })
-      expect(spy.calls.length).toBe(5)
-      expect(stub.props.string).toBe('a')
-      expect(stub.props.pass).toBe(obj2)
-
-      obj2.val = 'mutation'
-      tree.setState({ pass: obj2 })
-      expect(spy.calls.length).toBe(5)
-      expect(stub.props.string).toBe('a')
-      expect(stub.props.passVal).toBe('otherval')
-    })
-
     it('should throw an error if a component is not passed to the function returned by connect', () => {
       expect(connect()).toThrow(
         /You must pass a component to the function/
@@ -1940,6 +1857,9 @@ describe('React', () => {
 
       @connect(null, mapDispatchFactory, mergeParentDispatch)
       class Passthrough extends Component {
+        shouldComponentUpdate(nextProps) {
+          return !isEqual(this.props, nextProps)
+        }
         componentWillUpdate() {
           updatedCount++
         }