mergeProps example will never shallowEqual

[davidjbradshaw]

Do you want to request a feature or report a bug?

Documentation bug

What is the current behavior?

The example of mergeProps in the documentation creates a new function each time it is called, so the shallowEqual test will always fail leading to the component always rerendering.

This is the current example:

import * as actionCreators from './actionCreators'

function mapStateToProps(state) {
  return { todos: state.todos }
}

function mergeProps(stateProps, dispatchProps, ownProps) {
  return Object.assign({}, ownProps, {
    todos: stateProps.todos[ownProps.userId],
    addTodo: (text) => dispatchProps.addTodo(ownProps.userId, text)
  })
}

export default connect(mapStateToProps, actionCreators, mergeProps)(TodoApp)

What is the expected behavior?

I would expect the component to update only if ownProps.userId changes.

Suggested fix

If you're happy with this I can make a PR, but not sure if your happy to use Ramda in an example.

import { identity, memoizeWith } from 'ramda'
import * as actionCreators from './actionCreators'

const memoize = memoizeWith(identity)
const addTodo = memoize(
  (userId, dispatchProps) => (text) => dispatchProps.addTodo(userId, text)
)

function mapStateToProps(state) {
  return { todos: state.todos }
}

function mergeProps(stateProps, dispatchProps, ownProps) {
  return Object.assign({}, ownProps, {
    todos: stateProps.todos[ownProps.userId],
    addTodo: addTodo(ownProps.userId, dispatchProps)
  })
}

export default connect(mapStateToProps, actionCreators, mergeProps)(TodoApp)

Which versions of React, ReactDOM/React Native, Redux, and React Redux are you using? Which browser and OS are affected by this issue? Did this work in previous versions of React Redux? 7

[markerikson]

I think that's actually kind of the point - creating new values in mergeProps will cause re-renders. That's one of the reasons why we generally discourage use of mergeProps.

If anything, I'd rather add a note that this is not a recommended pattern, and you need to be cautious when using it.

[davidjbradshaw]

But doesn't mergeProps get called every time the store.subscribe function in Redux gets called, in the same way that mapState does?

My point here is that when mergeProps gets called with the same values as the previous call it still creates an object that fails shallowEqual and indeed would also always return false from deepEqual as well. As (() => null) !== (() => null).

Using memorise on the function here has the same effect as using createSelector to return a new object in mapState. So the returned object would now pass the shallowEqual check when called with the same values.

[markerikson]

I think we're kinda saying the same thing here.

Yes, this will re-run every time. Yes, that function declaration will cause re-renders. No, that's not a good thing. Yes, it's up to the user to memoize things. No, I don't think I want to specifically show a memoization example here - I'd rather leave a note warning them they would need to do that themselves.

[davidjbradshaw]

I think a note would be a big help, because without a deep understanding of how react-redux works it is a very subtle thing that when I shared with others before posting here no one could spot the issue.

Thinking about this a bit more if stateProps and ownProps have been already checked with shallowEqual and mergeProps is pure function. Then could the default function for areStatePropsEqual just be () => true? As if the inputs are changed we are rerendering anyway, and if they have not, then the output of mergeProps should be the same anyway, except for a new copy of the functions.

Or maybe the note could suggest that as an easy get out for this.