React-redux: getState in mapDispatchToProps

[pauldijou]

Since mapDispatchToProps can expose the dispatch method, it means it has access to the store. I was wondering why it didn't also expose, at least, the getState method?

Just like redux-thunk, you might need to access the current state to decide which action to actually dispatch regarding client side cache.

I'm fine with doing a PR but wanted to check if it would be valid. Also, if accepted, I would rather have [mapDispatchToProps(dispatch, [getState], [ownProps]): dispatchProps] signature, but it would be a breaking change compared to [mapDispatchToProps(dispatch, [ownProps], [getState]): dispatchProps]

[cbenz]

What you're asking for seems to be achieved through the mergeProps() function passed to connect().

See last example of the react-redux API doc.

[gaearon]

Yes, I agree this seems covered by mergeProps.

[pauldijou]

Not 100% unfortunately. What I'm trying to do it something like that:

// A common helper for all the app
export const doStuff = (dispatch, state) => () => {
  // Do some internal magic and dispatch dynamic actions depending on the state
  // like managing client-side cache depending on the current data
}

// My code
import { doStuff } from 'helper';
@connect(state => ({
  someProp: state.something,
}), (dispatch) => ({
  someAction: doStuff(dispatch, [state??]),
}))
class MyComponent ...

The problem is that the helper know on its own how to read the current state and which action to dispatch. So I would need to bind the whole state to a prop in order to expose it inside stateProps and access it inside mergeProps. That doesn't sound too good. I would also need to change the function signature but that's ok

// A common helper for all the app
export const doStuff = dispatch => state => () => {
  // Do some internal magic and dispatch dynamic actions depending on the state
}

// My code
import { doStuff } from 'helper';
@connect(state => ({
  someProp: state.something,
  state: state, // Uuuuurrrrggggg
}), (dispatch) => ({
  somePreAction: doStuff(dispatch),
}), (stateProps, dispatchProps, ownProps) => {
  return Object.assign({}, ownProps, {
    someAction: dispatchProps.somePreAction(stateProps.state),
  });
})
class MyComponent ...

I could do a selector exposed by the helper to only assign the minimum possible substate needed for the helper, but still, I would assign to the component props something not needed at all, just a "hack" to make it available in the mergeProps function.

On top of that, but that might be bad practice, not sure about it, I was thinking about replacing the actual state in the helper signature by the getState function itself. That's because the someAction function is eventually called when the user click on a button. Meaning that I need to render the component each time the "helper state" is modified to keep someAction in sync. But by passing the getState function itself, I could squeeze all those renders and, only when the function is actually called, the helper would read the state and dispatch the correct action.

[gaearon]

Have you looked at Redux Thunk?

It lets you write stuff like

function getSuggestions(query) {
  return (dispatch, getState) => {
    if (getState().suggestions[query]) {
      return;
    }

    return API.getSuggestions().then(
      response => dispatch({ type: 'SUGGESTIONS_RESPONSE', response }),
      error => dispatch({ type: 'SUGGESTIONS_ERROR', error })
    )
  };
}

// in component

class Stuff extends Component { /* ... */ }

export default connect(
  mapStateToProps,
  { getSuggestions }
)(Stuff)

[biels]

This is still not solved, but it could easily be just by changing one line of code, adding state or getState to mapDispatchToProps.
I would like to use it to provide functions dynamically to the component based on the state.
Please consider doing this change since it is very simple and adds a lot of functionality.
It may be possible that doing so could encourage bad practices. If it is the case, could you please provide some example as well as a right way to achieve what I and the OP are try to achieve?
Thanks in advance

[markerikson]

@biels : per the prior discussion, if you _really_ need to handle things that way, the "approved" method is to provide the third argument to connect, a mergeProps function, and handle it yourself there.

[biels]

Yes, i took a look at this way of doing it. The problem os that this forces me to expose props that I don't really need. It feels hacky as someone stated earlier.
Could you please explain why isn't the state accessible from mapDispatchToProps? There has to be a reason other than "there is already a hacky workaround for it" am I wrong? Cheers

[markerikson]

I don't have time at the moment to dig through other issues where this has come up, but it's basically not the intended use case. connect focuses on the use case where your component will be extracting varying data from the store, and using a fixed set of callbacks or action creators. If you need to actually customize that process, mergeProps is available.

Can you give an example of what "providing functions dynamically" would look like? Is there any reason why you can't, say, pass an object full of functions into the connected component as a prop, and determine which one to call _inside_ the component?

[jimbolla]

mapDispatchToProps will never be dependent on state, because doing so would mean it would get invoked every time the store state changes, creating new functions each time, causing the component to rerender every time. mapDispatchToProps already does a lot of logic to minimize this when props change, which is bad enough. I personally never even use the version that accepts props, to avoid unnecessary rerenders. You're a million bajillion times better off exposing a few props you don't really need than generating new functions and renders on every state change.

[markerikson]

Good points, Jim.

My own advice would be to move all this conditional logic outside the component into thunks, per Dan's comment earlier. Set up the component using the object shorthand for action creators ( like connect(null, {doSomeThings})(MyComponent)), have the component always call this.props.doSomeThings(a, b, c), and let the thunk figure out what the right operations are from there.

[biels]

I am working on a hateoas api bound application and I am trying to standardize the way any resource is presented and managed entirely so I have a reducer creator that provides a custom reducer for every resource view. I call those ActiveViews. Those ActiveViews are saved inside the state branch managed by its reducer. One of the attributes contains the available actions in a specific format. I would like to create a custom connect function like connectActiveView('activeViewName')(Component) which internally calls the react-redux connect function. It would provide the attributes of the resource in mapStateToProps and a function for every defined action in the ActiveView in mapDispatchToProps. A function provided in mapDispatchToProps would dispatch an action that targets the ActiveView's reducer by adding its name into the action's name.
To do this it is necessary to access the state (in a read-only way) during the mapping process.
This operation does not belong to mergeProps because it is not a post-processing to the props. It is actually "mapping dispatch to props", just with help of the state and therefore to keep things consistent it looks clear to me that it belongs to mapDispatchToProps.
Please, feel free to ask for any clarification on the use case. If I am successful, I will opensource the project as an addon to redux. Thanks!

[markerikson]

Yeah, that's where I think you're heading in the wrong direction a bit. Don't create a new function just to add an extra field into an action. Have _one_ thunk that takes the action name as an argument. Or, for that matter, remember that thunks have access to the application state via getState.

[biels]

Okay, after posting my explanation I read the two last comments. Hadn't thought about the performance implications. But how is referring to the state on mergeProps any different than doing it on mapDispatchToProps?

[biels]

I tried to use thunks to achieve it as well. The problem I see with this approach a part from the less nice api that I will get is the existence is the existence testing.
Let's suppose an action named fetch can be available. I could do if(this.props.fetch) showFetchButton and then call the action using this.props.fetch(). Actions would be of the type update, delete, accept, cancel... Basically representing buttons on the ui plus basic crud. You can't say that it isn't a nice api to have in a resource oriented application :) .

[markerikson]

To be honest, use of mergeProps _will_ probably result in the same perf implications that Jim mentioned. Recreating functions as props is generally a perf anti-pattern in React, no matter whether you're doing it in a parent's render method, or in mapDispatch/mergeProps.

I'm about 99% sure that moving the logic out to thunks is the right answer here. The API _might_ not be quite as "pretty" from a correspondence-with-CRUD-ops perspective, but it's going to be a whole lot simpler and more performant.

[biels]

Okay, thanks for the explanation and for your time. I will try to use thunks and post the solution when I get it. I hope I am still able to get a reasonably nice api.

[biels]

Would it be possible to only recreate the functions when the source object for the actions is updated while ignoring any other action? In this way the performance impact would be minuscule and comparable to the overhead an additional thunk would add.
React is especially good at doing these kinds of things. Would the same be applied to redux?

[markerikson]

I suppose you could do it using a memoized selector function:

import {createSelector} from "reselect";

const selectStuff = state => state.stuff;

const selectMoreStuff = createSelector(
    selectStuff,
    stuff => stuff.more
);

const createFunctions = createSelector(
    selectMoreStuff,
    more => {
        return () => {
            /// some function here that only is created when "more" changes"
        }
    }
);

[jimbolla]

But how is referring to the state on mergeProps any different than doing it on mapDispatchToProps.

You'd only be dependent on the slice of state that is relevant instead of the entire state.

[al6x]

Have you looked at Redux Thunk?

Why sync and async (thunk) versions of dispatch supposed to have different method signature? Given that conceptually they are the same things and the only difference is that one is async?

I mean - one has getState and another doesn't. There could be a valid reasons why there's should be no getState in sync version of dispatch, but then - why should it be available in async?