Merge pull request #335 from chentsulin/improve-mapStateToProps-invariant

Close #309, Improve  invariant validation error message

Author: gaearon

src/components/connect.js

@@ -18,16 +18,6 @@ function getDisplayName(WrappedComponent) {
   return WrappedComponent.displayName || WrappedComponent.name || 'Component'
 }
 
-function checkStateShape(stateProps, dispatch) {
-  invariant(
-    isPlainObject(stateProps),
-    '`%sToProps` must return an object. Instead received %s.',
-    dispatch ? 'mapDispatch' : 'mapState',
-    stateProps
-  )
-  return stateProps
-}
-
 // Helps track hot reloading.
 let nextVersion = 0
 
@@ -45,17 +35,25 @@ export default function connect(mapStateToProps, mapDispatchToProps, mergeProps,
   // Helps track hot reloading.
   const version = nextVersion++
 
-  function computeMergedProps(stateProps, dispatchProps, parentProps) {
-    const mergedProps = finalMergeProps(stateProps, dispatchProps, parentProps)
-    invariant(
-      isPlainObject(mergedProps),
-      '`mergeProps` must return an object. Instead received %s.',
-      mergedProps
-    )
-    return mergedProps
-  }
-
   return function wrapWithConnect(WrappedComponent) {
+    const connectDisplayName = `Connect(${getDisplayName(WrappedComponent)})`
+
+    function checkStateShape(props, methodName) {
+      invariant(
+        isPlainObject(props),
+        '`%s %s` must return an object. Instead received %s.',
+        connectDisplayName,
+        methodName,
+        props
+      )
+      return props
+    }
+
+    function computeMergedProps(stateProps, dispatchProps, parentProps) {
+      const mergedProps = finalMergeProps(stateProps, dispatchProps, parentProps)
+      return checkStateShape(mergedProps, 'mergeProps')
+    }
+
     class Connect extends Component {
       shouldComponentUpdate() {
         return !pure || this.haveOwnPropsChanged || this.hasStoreStateChanged
@@ -68,9 +66,9 @@ export default function connect(mapStateToProps, mapDispatchToProps, mergeProps,
 
         invariant(this.store,
           `Could not find "store" in either the context or ` +
-          `props of "${this.constructor.displayName}". ` +
+          `props of "${connectDisplayName}". ` +
           `Either wrap the root component in a <Provider>, ` +
-          `or explicitly pass "store" as a prop to "${this.constructor.displayName}".`
+          `or explicitly pass "store" as a prop to "${connectDisplayName}".`
         )
 
         const storeState = this.store.getState()
@@ -88,7 +86,7 @@ export default function connect(mapStateToProps, mapDispatchToProps, mergeProps,
           this.finalMapStateToProps(state, props) :
           this.finalMapStateToProps(state)
 
-        return checkStateShape(stateProps)
+        return checkStateShape(stateProps, 'mapStateToProps')
       }
 
       configureFinalMapState(store, props) {
@@ -100,7 +98,7 @@ export default function connect(mapStateToProps, mapDispatchToProps, mergeProps,
 
         return isFactory ?
           this.computeStateProps(store, props) :
-          checkStateShape(mappedState)
+          checkStateShape(mappedState, 'mapStateToProps')
       }
 
       computeDispatchProps(store, props) {
@@ -113,7 +111,7 @@ export default function connect(mapStateToProps, mapDispatchToProps, mergeProps,
           this.finalMapDispatchToProps(dispatch, props) :
           this.finalMapDispatchToProps(dispatch)
 
-        return checkStateShape(dispatchProps, true)
+        return checkStateShape(dispatchProps, 'mapDispatchToProps')
       }
 
       configureFinalMapDispatch(store, props) {
@@ -125,7 +123,7 @@ export default function connect(mapStateToProps, mapDispatchToProps, mergeProps,
 
         return isFactory ?
           this.computeDispatchProps(store, props) :
-          checkStateShape(mappedDispatch, true)
+          checkStateShape(mappedDispatch, 'mapDispatchToProps')
       }
 
       updateStatePropsIfNeeded() {
@@ -284,7 +282,7 @@ export default function connect(mapStateToProps, mapDispatchToProps, mergeProps,
       }
     }
 
-    Connect.displayName = `Connect(${getDisplayName(WrappedComponent)})`
+    Connect.displayName = connectDisplayName
     Connect.WrappedComponent = WrappedComponent
     Connect.contextTypes = {
       store: storeShape

test/components/connect.spec.js

@@ -1004,71 +1004,71 @@ describe('React', () => {
             {makeContainer(() => 1, () => ({}), () => ({}))}
           </ProviderMock>
         )
-      }).toThrow(/mapState/)
+      }).toThrow(/Connect\(Container\) mapState/)
 
       expect(() => {
         TestUtils.renderIntoDocument(
           <ProviderMock store={store}>
             {makeContainer(() => 'hey', () => ({}), () => ({}))}
           </ProviderMock>
         )
-      }).toThrow(/mapState/)
+      }).toThrow(/Connect\(Container\) mapState/)
 
       expect(() => {
         TestUtils.renderIntoDocument(
           <ProviderMock store={store}>
             {makeContainer(() => new AwesomeMap(), () => ({}), () => ({}))}
           </ProviderMock>
         )
-      }).toThrow(/mapState/)
+      }).toThrow(/Connect\(Container\) mapState/)
 
       expect(() => {
         TestUtils.renderIntoDocument(
           <ProviderMock store={store}>
             {makeContainer(() => ({}), () => 1, () => ({}))}
           </ProviderMock>
         )
-      }).toThrow(/mapDispatch/)
+      }).toThrow(/Connect\(Container\) mapDispatch/)
 
       expect(() => {
         TestUtils.renderIntoDocument(
           <ProviderMock store={store}>
             {makeContainer(() => ({}), () => 'hey', () => ({}))}
           </ProviderMock>
         )
-      }).toThrow(/mapDispatch/)
+      }).toThrow(/Connect\(Container\) mapDispatch/)
 
       expect(() => {
         TestUtils.renderIntoDocument(
           <ProviderMock store={store}>
             {makeContainer(() => ({}), () => new AwesomeMap(), () => ({}))}
           </ProviderMock>
         )
-      }).toThrow(/mapDispatch/)
+      }).toThrow(/Connect\(Container\) mapDispatch/)
 
       expect(() => {
         TestUtils.renderIntoDocument(
           <ProviderMock store={store}>
             {makeContainer(() => ({}), () => ({}), () => 1)}
           </ProviderMock>
         )
-      }).toThrow(/mergeProps/)
+      }).toThrow(/Connect\(Container\) mergeProps/)
 
       expect(() => {
         TestUtils.renderIntoDocument(
           <ProviderMock store={store}>
             {makeContainer(() => ({}), () => ({}), () => 'hey')}
           </ProviderMock>
         )
-      }).toThrow(/mergeProps/)
+      }).toThrow(/Connect\(Container\) mergeProps/)
 
       expect(() => {
         TestUtils.renderIntoDocument(
           <ProviderMock store={store}>
             {makeContainer(() => ({}), () => ({}), () => new AwesomeMap())}
           </ProviderMock>
         )
-      }).toThrow(/mergeProps/)
+      }).toThrow(/Connect\(Container\) mergeProps/)
     })
 
     it('should recalculate the state and rebind the actions on hot update', () => {