Skip to content

Recommendations for best practices regarding action-creators, reducers, and selectors #1171

Closed
@bvaughn

Description

@bvaughn

My team has been using Redux for a couple of months now. Along the way I've occasionally found myself thinking about a feature and wondering "does this belong in an action-creator or a reducer?". The documentation seems a bit vague on this fact. (Or perhaps I've just missed where it's covered, in which case I apologize.) But as I've written more code and more tests I've come to have stronger opinions about where things should be and I thought it would be worth sharing and discussing with others.

So here are my thoughts.

Use selectors everywhere

This first one is not strictly related to Redux but I'll share it anyway since it's indirectly mentioned below. My team uses rackt/reselect. We typically define a file that exports selectors for a given node of our state tree (eg. MyPageSelectors). Our "smart" containers then use those selectors to parameterize our "dumb" components.

Over time we've realized that there is added benefit to using these same selectors in other places (not just in the context of reselect). For example, we use them in automated tests. We also use them in thunks returned by action-creators (more below).

So my first recommendation is- use shared selectors everywhere- even when synchronously accessing data (eg. prefer myValueSelector(state) over state.myValue). This reduces the likelihood of mistyped variables that lead to subtle undefined values, it simplifies changes to the structure of your store, etc.

Do more in action-creators and less in reducers

I think this one is very important although it may not be immediately obvious. Business logic belongs in action-creators. Reducers should be stupid and simple. In many individual cases it does not matter- but consistency is good and so it's best to consistently do this. There are a couple of reasons why:

  1. Action-creators can be asynchronous through the use of middleware like redux-thunk. Since your application will often require asynchronous updates to your store- some "business logic" will end up in your actions.
  2. Action-creators (more accurately the thunks they return) can use shared selectors because they have access to the complete state. Reducers cannot because they only have access to their node.
  3. Using redux-thunk, a single action-creator can dispatch multiple actions- which makes complicated state updates simpler and encourages better code reuse.

Imagine your state has metadata related to a list of items. Each time an item is modified, added to, or removed from the list- the metadata needs to be updated. The "business logic" for keeping the list and its metadata in sync could live in a few places:

  1. In the reducers. Each reducer (add, edit, remove) is responsible for updating the list as well as the metadata.
  2. In the views (container/component). Each view that invokes an action (add, edit, remove) it is also responsible for invoking an updateMetadata action. This approach is terrible for (hopefully) obvious reasons.
  3. In the action-creators. Each action-creator (add, edit, remove) returns a thunk that dispatches an action to update the list and then another action to updates the metadata.

Given the above choices, option 3 is solidly better. Both options 1 and 3 support clean code sharing but only option 3 supports the case where list and/or metadata updates might be asynchronous. (For example maybe it relies on a web worker.)

Write "ducks" tests that focus on Actions and Selectors

The most efficient way to tests actions, reducers, and selectors is to follow the "ducks" approach when writing tests. This means you should write one set of tests that cover a given set of actions, reducers, and selectors rather than 3 sets of tests that focus on each individually. This more accurately simulates what happens in your real application and it provides the most bang for the buck.

Breaking it down further I've found that it's useful to write tests that focus on action-creators and then verify the outcome using selectors. (Don't directly test reducers.) What matters is that a given action results in the state you expect. Verifying this outcome using your (shared) selectors is a way of covering all three in a single pass.

Activity

cpsubrian

cpsubrian commented on Dec 23, 2015

@cpsubrian

Curious if you use Immutable.js or other. In the handful of redux things I've built I couldn't imagine not using immutable, but I do have a pretty deeply nested structure that Immutable helps tame.

bvaughn

bvaughn commented on Dec 23, 2015

@bvaughn
Author

Wow. What an oversight for me not to mention that. Yes! We use Immutable! As you say, it's hard to imagine not using it for anything substantial.

cpsubrian

cpsubrian commented on Dec 23, 2015

@cpsubrian

@bvaughn One area I've struggled with is where to draw the line between Immutable and the components. Passing immutable objects into the Components lets you use pure-render decorators/mixins very easily but then you end up with IMmutable code in your components (which I don't like). So far I have just caved and done that but I suspect you use selectors in the render() methods instead of directly accessing Immutable.js' methods?

bvaughn

bvaughn commented on Dec 23, 2015

@bvaughn
Author

To be honest this is something we haven't defined a hard policy on yet. Often we use selectors in our "smart" containers to extra native values from our immutable objects and then pass the native values to our components as strings, booleans, etc. Occasionally we'll pass an Immutable object but when we do- we almost always pass a Record type so that the component can treat it like a native object (with getters).

winstonewert

winstonewert commented on Dec 27, 2015

@winstonewert

I've been moving in the opposite direction, making action creators more trivial. But I'm really just starting out with redux. Some questions about your approach:

  1. How do you test your action creators? I like moving as much logic as possible to pure, synchronous functions that don't depend on external services because its easier to test.
  2. Do you use the time travel with hot reloading? One of the neat things with with react redux devtools is that when hot reloading is setup, the store will rerun all of the actions against the new reducer. If I were to move my logic into the action creators, I'd lose that.
  3. If your action creators dispatch multiple times to bring about an effect, does that mean that your state is briefly in an invalid state? (I'm thinking here of multiple synchronous dispatches, not ones dispatch asynchronously at a later point)
slorber

slorber commented on Dec 27, 2015

@slorber
Contributor

Use selectors everywhere

Yes that seems like saying your reducers are an implementation detail of your state and that you expose your state to your component through a query API.
Like any interface it permits to decouple and make it easy to refactor the state.

Use ImmutableJS

IMO with new JS syntax it's not so much useful to use ImmutableJS anymore as you can easily modify lists and objects with normal JS. Unless you have very large lists and objects with lots of properties and you need structural sharing for performance reasons ImmutableJS is not a strict requirement.

Do more in actionCreators

@bvaughn you should really look at this project: https://github.com/yelouafi/redux-saga
When I started discussing about sagas (initially backend concept) to @yelouafi it was to solve this kind of problem. In my case I first tried to use sagas while plugging in an user onboarding on an existing app.

bvaughn

bvaughn commented on Dec 28, 2015

@bvaughn
Author
  1. How do you test your action creators? I like moving as much logic as possible to pure, synchronous functions that don't depend on external services because its easier to test.

I tried to describe this above, but basically... I think it makes the most sense (to me so far) to test your action-creators with a "ducks"-like approach. Begin a test by dispatching the result of an action-creator and then verify the state using selectors. This way- with a single test you can cover the action-creator, its reducer(s), and all related selectors.

  1. Do you use the time travel with hot reloading? One of the neat things with with react redux devtools is that when hot reloading is setup, the store will rerun all of the actions against the new reducer. If I were to move my logic into the action creators, I'd lose that.

No, we don't use time-travel. But why would your business logic being in an action-creator have any impact here? The only thing that updates your application's state is your reducers. And so re-running the created actions would achieve the same result either way.

  1. If your action creators dispatch multiple times to bring about an effect, does that mean that your state is briefly in an invalid state? (I'm thinking here of multiple synchronous dispatches, not ones dispatch asynchronously at a later point)

Transient invalid state is something you can't really avoid in some cases. So long as there is eventual consistency then it's usually not a problem. And again, your state could be temporarily invalid regardless of your business logic being in the action-creators or the reducers. It has more to do with side-effects and the specifics of your store.

bvaughn

bvaughn commented on Dec 28, 2015

@bvaughn
Author

IMO with new JS syntax it's not so much useful to use ImmutableJS anymore as you can easily modify lists and objects with normal JS. Unless you have very large lists and objects with lots of properties and you need structural sharing for performance reasons ImmutableJS is not a strict requirement.

The primary reasons for using Immutable (in my eyes) aren't performance or syntactic sugar for updates. The primary reason is that it prevents you (or someone else) from accidentally mutating your incoming state within a reducer. That's a no-no and it's unfortunately easy to do with plain JS objects.

@bvaughn you should really look at this project: https://github.com/yelouafi/redux-saga
When I started discussing about sagas (initially backend concept) to @yelouafi it was to solve this kind of problem. In my case I first tried to use sagas while plugging in an user onboarding on an existing app.

I have actually checked out that project before :) Although I haven't yet used it. It does look neat.

winstonewert

winstonewert commented on Dec 28, 2015

@winstonewert

I tried to describe this above, but basically... I think it makes the most sense (to me so far) to test your action-creators with a "ducks"-like approach. Begin a test by dispatching the result of an action-creator and then verify the state using selectors. This way- with a single test you can cover the action-creator, its reducer(s), and all related selectors.

Sorry, I got that part. What I was wondering about is the part of tests that interacts with the asynchronicity. I might write a test something like this:

var store = createStore();
store.dispatch(actions.startRequest());
store.dispatch(actions.requestResponseReceived({...});
strictEqual(isLoaded(store.getState());

But what does your test look like? Something like this?

var mock = mockFetch();
store.dispatch(actions.request());
mock.expect("/api/foo.bar").andRespond("{status: OK}");
strictEqual(isLoaded(store.getState());

No, we don't use time-travel. But why would your business logic being in an action-creator have any impact here? The only thing that updates your application's state is your reducers. And so re-running the created actions would achieve the same result either way.

What if the code is changed? If I change the reducer, the same actions are replayed but with the new reducer. Whereas if I change the action creator, that the new versions aren't replayed. So to consider two scenarios:

With a reducer:

  1. I try an action in my app.
  2. There is a bug in my reducer, resulting in the wrong state.
  3. I fix the bug in the reducer and save
  4. Time travel loads the new reducer and puts me in the state I should have been in.

Whereas with an action creator

  1. I try an action in my app.
  2. There is a bug in the action creator, resulting in the wrong action being created
  3. I fix the bug in the action creator and save
  4. I'm still in the incorrect state, which will require me to at least try the action again, and possibly refresh if it put me in a completely broken state.

Transient invalid state is something you can't really avoid in some cases. So long as there is eventual consistency then it's usually not a problem. And again, your state could be temporarily invalid regardless of your business logic being in the action-creators or the reducers. It has more to do with side-effects and the specifics of your store.

I guess my way of thinking of redux insists that the store is always in a valid state. The reducer always takes a valid state and produces a valid state. What cases do you think forces one to allow some inconsistent states?

sompylasar

sompylasar commented on Dec 28, 2015

@sompylasar
slorber

slorber commented on Dec 28, 2015

@slorber
Contributor

What do you mean by transient state in Redux @bvaughn and @sompylasar ? Weither the dispatch finishes, or it throws. If it throws then the state do not change.

Unless your reducer has code issues, Redux only has states that are consistent with the reducer logic. Somehow all actions dispatched are handled in a transaction: weither the whole tree updates, or the state does not change at all.

If the whole tree updates but not in an appropriate way (like a state that React can't render), it's just you don't have done your job correctly :)

In Redux the current state is to consider that a single dispatch is a transaction boundary.

However I understand the concern of @winstonewert that seems to want to dispatch 2 actions synchronously in a same transaction. Because sometimes actionCreators dispatch multiple actions and expect that all the actions are executed correctly. If 2 actions are dispatched and then the second one fail, then only the 1st one will be applied, leading to a state that we could consider "inconsistent". Maybe @winstonewert wants that if the 2nd action dispatch failes, then we rollback the 2 actions.

@winstonewert I've implemented something like that in our internal framework here and it works fine until now: https://github.com/stample/atom-react/blob/master/src/atom/atom.js
I also wanted to handle rendering errors: if a state can't be rendered successfully I wanted my state to be rollbacked to avoid blocking the UI. Unfortunatly until next release React does a very bad job when the render methods throw errors so it was not that much useful but may be in the future.

I'm pretty sure that we can allow a store to accept multiple sync dispatches in a transaction with a middleware.

However I'm not sure it would be possible to rollback the state in case of rendering error, as generally the redux store has already "commited" when we try to render its state. In my framework there is a "beforeTransactionCommit" hook that I use to trigger the rendering and to eventually rollback on any render error.

slorber

slorber commented on Dec 28, 2015

@slorber
Contributor

@gaearon I wonder if you plan to support these kind of features and if it would be possible with the current API.

It seems to me that redux-batched-subscribe does not permit to do real transaction but just reduce the number of renderings. What I see is that the store "commit" after each dispatch even if the subscription listener is only fired once at the end

gaearon

gaearon commented on Dec 28, 2015

@gaearon
Contributor

Why do we need complete transaction support? I don't think I understand the use case.

slorber

slorber commented on Dec 28, 2015

@slorber
Contributor

@gaearon I'm not really sure yet but would be happy to know more of @winstonewert usecase.

The idea is that you could do dispatch([a1,a2]) and if a2 fails, then we rollback to the state before a1 was dispatched.

In the past I've often been dispatching multiple actions synchronously (on a single onClick listener for example, or in an actionCreator) and primarily implemented transactions as a way to call render only at the end of all actions being dispatched, but this has been solved in a different way by the redux-batched-subscribe project.

In my usecases the actions I used to fire on a transaction was mostly to avoid unnecessary renderings, but the actions did make sense independently so even if the dispatch failed for the 2nd action, not rollbacking the 1st action would still give me a consistent state (but maybe not the one that was planned...). I don't really know if someone can come up with a usecase where a full rollback would be useful

However when the rendering fails doesn't it make sense to try to rollback to the last state for which the render does not fail instead of trying to make progress on an unrenderable state?

116 remaining items

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @bcardi@bvaughn@denis-sokolov@sacho@dtinth

        Issue actions

          Recommendations for best practices regarding action-creators, reducers, and selectors · Issue #1171 · reduxjs/redux