DEV Community

loading...

You've been doing mapDispatchToProps wrong this entire time

mcrowder65 profile image Matt Crowder ・1 min read

Do this:

const mapDispatchToProps = {
  decrement: () => ({ type: "DECREMENT" }),
  increment: () => ({ type: "INCREMENT" })
};

instead of this:

const mapDispatchToProps = dispatch => {
  return {
    decrement: () => dispatch({ type: "DECREMENT" }),
    increment: () => dispatch({ type: "INCREMENT" })
  };
};

The difference is that we rely on react-redux to inject dispatch on each value that is a function in our mapDispatchToProps object, rather than relying on react-redux to inject dispatch into our mapDispatchToProps function.

If you are not effectively testing your code, then this is an easy way to increase code coverage, but it will also prevent bugs, because then you don't have to worry about forwarding extra arguments.

For a working example on github, see this repo: https://github.com/mcrowder65/map-dispatch-to-props

Discussion (11)

Collapse
markerikson profile image
Collapse
joeysino profile image
Joey Clark • Edited

This is really a time-saver!

Especially if you import the decrement() and increment() action creator functions from the actions file. Then your example becomes even shorter:

import { decrement, increment } from '../redux/actions';

const mapDispatchToProps = {
  decrement,
  increment,
};

And if you use TypeScript, it is now trivial to define the types:

interface DispatchProps {
  decrement: typeof decrement;
  increment: typeof increment;
}
Collapse
dougajmcdonald profile image
Doug McDonald

I've been advocating this in all projects we have using Redux. Did discover a nasty issue the other day though where circular references in your modules can cause imported action creators / thunks to be undefined when passing to the shorthand. Using the older function syntax works around this problem.

Collapse
skydevht profile image
Holy-Elie Scaïde

I've started with that method (I was using the redux-actions library to create my action creators), but reverted to the old method while using rematch as the action creators will be children of dispatch. from

connect(null, {
  increment
})

it becomes this

connect(null, ({ Counter: { increment }}) => ({
  increment
}))
Collapse
hugotox profile image
Hugo Pineda

I never use mapDispatchToProps, I find cleaner to use this.props.dispatch(someAction()).
This way the code is explicitly saying it is dispatching an action

Collapse
mcrowder65 profile image
Matt Crowder Author

That's true, and I feel like to someone new, it's more obvious what is going on, since it's explicit, dispatch is just injected into props, whereas using mapDispatchToProps... there's a lot of dependency injection going on behind the scenes that's hard to understand.

Collapse
thekashey profile image
Anton Korzunov

In this case actions you might emit would not a part of component’a public interface (aka props).
Giving ‘dispatch’ and letting component do whatever it wants - might not be a good idea.

Collapse
revskill10 profile image
Truong Hoang Dung

mapAToB seems imperative, while React prefer declarative coding style.

Collapse
moopet profile image
Ben Sinclair

Hah, the joke's on you, I've never done mapDispatchToWhateverItIs at all.

Collapse
pranjalagnihot8 profile image
Pranjal Agnihotri {..❤️}

If you want to bind the action based on some logic then go with the later else go with the former one.

Collapse
worc profile image
worc

i'm not sure there was an argument here why one way is "right" and the other is "wrong"?

Forem Open with the Forem app