Preferred way of writing action creators

[ismay]

I was refactoring some actions that crossed our max param limit. The actions accepted multiple parameters and built that into a payload for the action. When refactoring the action creator to accept a single object instead, this is how they'd look initially (for example):

export const displaySnackMessage = ({
    message,
    onSnackActionClick,
    onSnackRequestClose,
    snackType,
}) =>
    createAction(actions.DISPLAY_SNACK_MESSAGE, {
        message,
        onSnackActionClick,
        onSnackRequestClose,
        snackType,
    })

Which felt a bit redundant to me, as you could then also do:

export const displaySnackMessage = (payload) =>
    createAction(actions.DISPLAY_SNACK_MESSAGE, payload)

But then again, the destructuring is also nice and explicit about what the action creator expects. So my questions was what everyone prefers.

See the start of the thread here: https://dhis2.slack.com/archives/G451J9KGR/p1558013834027600. Some quotes/points in response to the first approach (above, the more explicit one):

Pros

States clearly what the action creator expects to receive

I think it follows the decision we made to be “liberal in what you accept”, so in this case the action creator accepts an object with unrelated properties and filters them down to just the relevant ones when creating the action. It also clones the payload object so it can’t be affected by modifications elsewhere (though redux probably does this under-the-hood anyway, I haven’t confirmed)

Cons

If we want to add an new property, this now creates another place to do so

Let me know what you think, and if I've missed anything!

[ismay]

I don't think this is terribly important by the way, in the grand scheme of things. But it's nice to know if there's a general consensus on what approach to take, just so I can be consistent with it.

[Mohammer5]

Interesting discussion! Here's what I think about it:

1. I think we should generally try to avoid having that many params for actions. In most cases it's a code smell (at least that's my experience).

The functions for the snackbar in your example could (should?!) be created in selectors / the component rendering the snackbar, they can be computed from the given state, so they're derived state. Also functions are not serializable and therefore shouldn't be in redux state. @HendrikThePendric might remember how I was vehemently against putting functions in the state, and he came up with a very nice solution with simple actions.

Of course in the end it's up to the developer whether state should be serializable or not. Apps can work perfectly fine if the state isn't serializable, but I still think that it's good practice to try to keep the state as serializable as possible, especially when functions, symbols, etc can be derived.

But that's only about the example, of course it's not always possible.

2. Using the way of the example doesn't guarantee that those properties are passed

Unlike with typescript, if one of those properties is missing, undefined will be passed through, so it's up to the reducer to catch the missing payload values. So it's all about documentation, and that's a perfect use case for a jsdoc block

/**
 * @param {Object} payload
 * @param {string} payload.message
 * @param {string} payload.snackType
 * @param {Function} payload.onSnackActionClick
 * @param {Function} payload.onSnackRequestClose
 */
export const displaySnackMessage = payload =>
    createAction(
        actions.DISPLAY_SNACK_MESSAGE,
        payload,
    )

Conclusion

I don't think it really matters as long as there's some kind of documentation for the dev, even if there's repetition. The most important part is that we can work with the code without having to think too much about it, reducing the repetition is a nice extra in this case. At least I don't have a strong opinion on this. With the jsdoc block, we still can have the best of both worlds:

1. All properties documented
2. One part of the code that has to be edited when there are new properties
3. A collapsible comment for easier reading in editors/IDEs

[varl]

Another option to de-duplicate information and reduce redundant typing is passing in a parameter object, and manually passing through relevant props to the action creator:

export const displaySnackMessage = payload =>
    createAction(actions.DISPLAY_SNACK_MESSAGE, {
        message: payload.message,
        onSnackActionClick: payload.onSnackActionClient,
        onSnackRequestClose: payload.onSnackRequestClose,
        snackType: payload.snackType,
    })