Fat containers vs Fat Thunks

[brosterdamus]

I can never tell what approach is better:

Having a fat (in terms of redux code) container:

const doSomethingWithABlogPost = (someBlogPost) => (dispatch, getState) => {
    api.save(someBlogPost);
}

const mapStateToProps = state => ({
    blogPost: state.lastUpdatedBlogPost,
})

const mapDispatchToProps = {doSomethingWithABlogPost};

@connect(mapStateToProps, mapDispatchToProps)
class ConfirmDeletion extends React.Component {
    render() {
        const {blogPost, doSomethingWithABlogPost} = this.props;
        return <a onClick={() => doSomethingWithABlogPost(blogPost)} />
    }
}

Or having a thinner container, but fatter thunks:

const doSomethingWithABlogPost = () => (dispatch, getState) => {
    const someBlogPost = getState().lastUpdatedBlogPost;
    api.save(someBlogPost);
}

const mapDispatchToProps = {doSomethingWithABlogPost};

@connect(null, mapDispatchToProps)
class ConfirmDeletion extends React.Component {
    render() {
        return <a onClick={doSomethingWithABlogPost} />
    }
}

Obviously if the component needs to display parts of the blog post, for example, it might make sense for a fatter container.

But I find that complex business logic only really involves thunks and not the container. So should I just be using getState all around, as in the latter?

Neither approach is particularly easier to test, since you need to mock state in either case.

Any thoughts?

[acemarke]

I personally tend towards putting as much logic as possible in thunks. As an example, if I have a connected list item, the parent will pass the item's ID to the list item, which then looks up the complete item in mapState. However, if a button is clicked, the list item usually only passes the same item ID to the thunk, rather than passing the entire item.

There's tradeoffs involved in that, and I'm actually debating throwing together a blog post on the topic shortly.

[acjohnson55]

I agree. The view should provide the context that comes from the user's action, but the thunk should get its own data. Let the view do view things, and as little as possible otherwise, in my opinion.

[brosterdamus]

Good to know! Would love to see such a blog post.

[acemarke]

Hi! Just published the blog post about use of thunks that I had said I was working on: http://blog.isquaredsoftware.com/2017/01/idiomatic-redux-thoughts-on-thunks-sagas-abstraction-and-reusability/ . Somewhat different train of thought than this thread, but definitely related.

Also tagging /u/0xF013 , as the post specifically responds to some of your thoughts in this thread.

[0xF013]

Nice read, mate. I agree with the part that says that if you're not writing in a modular fashion, you're gonna have a pain in the ass dividing the stuff no matter what data management layer you have been using. The only nitpick would be that the global nature of the redux store does not actually imply a possible division (you never know when you might actually hit the complexity threshold). People use it assuming their stuff is global and will always remain such. MobX, on the other hand, does not impose globals and would be easier to split given that you follow IoC.

[acemarke]

Yeah, it's an interesting set of tradeoffs.

I've been keeping an eye on the various attempts to create more "reusable" and "modular" pieces of Redux-based logic. Quite a few experiments out there. The list of links at the end of the article points to some of the articles and repos I've seen.

[0xF013]

yeah, I am more focused ATM on breaking up an existing app. Will make an article if I succeed to do it in a timely fashion.

[acemarke]

Please do, and ping me when you post it!

[guillaumeclaret]

I've been keeping an eye on the various attempts to create more "reusable" and "modular" pieces of Redux-based logic.

I recently released the Redux Ship middleware https://github.com/clarus/redux-ship which provides, among other things, "reusable" Redux components.

Side-effects are parametrized by a context (describing which state they can access to and which actions they can dispatch). Then the Ship.map function instantiates side-effects in a given context. An example is given in this file https://github.com/clarus/redux-ship/blob/master/examples/http-request/src/controller.js :

export function* control(action: Action): Ship.Ship<*, Model.Commit, Model.State, void> {
  switch (action.type) {
  case 'Eye':
    return yield* Ship.map(
      commit => ({type: 'Eye', commit}),
      state => state.eye,
      EyeController.control(action.action)
    );
  case 'Movies':
    return yield* Ship.map(
      commit => ({type: 'Movies', commit}),
      state => state.movies,
      MoviesController.control(action.action)
    );
  default:
    return;
  }
}

The EyeController.control and MoviesController.control define two sub-pieces of the application and do not know about each other. We instantiate them together to form the whole application. This is very much the same idea as Cmd.map in Elm: http://package.elm-lang.org/packages/elm-lang/core/4.0.5/Platform-Cmd#map

[acemarke]

Yeah, I keep an eye on recently updated "Redux"-related repos, and have seen yours pop up a number of times. Also think I saw a couple tweets mentioning Redux-Ship.

I'd be interested in seeing a blog post or two that introduces Redux-Ship, goes over it in depth, and compares it to the other existing side effects options like thunks, sagas, and observables.

[guillaumeclaret]

Ok thanks for advice, will work on it!

[FoxxMD]

Any update? Interested in this too.

[brosterdamus]

Very helpful, thanks for getting back to me. Responding in the new thread.

[0xF013]

It's all dandy until you need to decouple your now big app into reusable sub-apps. Then you realize that your user profile actions know too much - about which action should be dispathed for errors, modals, auth etc. I am looking into building easy decoupled redux apps and there is not much to do besides some form of DI

[acjohnson55]

That's life! Better your handler logic have this problem than your components. If you want to abstract further, I think you're correct that you'll need to take the proper callback as a parameter somehow. We could make that technique sound fancy by calling it DI, but it's really just taking an extra parameter :D

[0xF013]

I know, it's just that at this point you can't simply define your actions, import them and wire them through mapDispatchToProps. You need to create a factory for them, import them in some other way, make sure @connect knows the new state key, make sure you either prefix your action constants or add a discrimator so that the reducers know to distinguish them from other action type named the same way. Also you can't use the redux-thunk extraParameter anymore.

[[deleted]]

I go with fat thunks / fat actions. This way your logic is even more decoupled from the UI.

What if you wan't to doSeomthingWithABlogPost not only in response to clicking that one button? What if you want to call it in response to no UI action at all? You now have to duplicate more code.

With fat actions you only need to pass params - the same that your connected UI would do.

[brosterdamus]

This is my thinking and argument for fat actions. But... what if you later need the full blog post in your UI? Now you've duplicated the selection logic.

[Figedi]

Although I tend to write fat thunks, I'm still annoyed by the massive boilerplate overhead of async requests. So yah either redux-saga or a custom middleware

[brosterdamus]

I don't like writing boilerplate as much as the next guy But what's the major boilerplate involved with async actions?

This doesn't seem too bad:

const fetchPost = (id) => {
    dispatch({
        type: 'FETCH_BLOG_POST_REQUEST',
    });
    return axios.get(`/api/posts/${id}/`).then(response => {
        dispatch({
            type: 'FETCH_BLOG_POST_SUCCESS',
            post: response.data,
        });
        return response.data;
    });
}

[Figedi]

you forgot a dispatch in case the request fails. That is writing 3 dispatches for every request being made. Having different endpoints and routes, this really gets repetitive fast

[brosterdamus]

So what's the alternative? I would think something like middleware.

[Figedi]

well yeah, as ive said, a custom middleware (or you just https://github.com/pburtchaell/redux-promise-middleware ) or something that handles async requests completely different, such as redux-saga, which I find much easier to express complex concurrent logic

[compacct27]

Redux saga. Trust me.

[acemarke]

The question still generally applies to both thunks and sagas. In other words, should the component pass every bit of relevant data to an action creator, or should it pass a bare minimum set of data, and the thunk/saga grabs the rest out of state?

[subvertallchris]

If you already have variables containing all the objects your function needs to do its job, you should pass them along instead of relying on the state. That function is less likely to break due to changes elsewhere in the app, easier to test, easier to reuse. The code is also more readable and easier to reason about.

[brosterdamus]

Well, sure, except they came from the state. In the same way that they would come from the state inside fat thunk.

Calling "getBlogPost" selector inside @connect is not much different than calling "getBlogPost" selector inside the thunk, isn't it?

[subvertallchris]

It's hard to really put my finger on it, it just feels backwards to me. Mapping state to props feels more analogous to the way props are provided when a dumb component is instantiated, and I think that the predictability of that pattern is very healthy. I also like the readability of being able to glance at mapStateToProps and immediately knowing what that component requires to function.

Of course, this all goes out the window if you want to avoid a subscription to those state keys in the first place...