-
-
Notifications
You must be signed in to change notification settings - Fork 15.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Recommendations for best practices regarding action-creators, reducers, and selectors #1171
Comments
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. |
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. |
@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? |
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 |
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:
|
Use selectors everywhereYes 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. Use ImmutableJSIMO 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 |
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.
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.
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. |
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.
I have actually checked out that project before :) Although I haven't yet used it. It does look neat. |
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:
But what does your test look like? Something like this?
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:
Whereas with an action creator
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? |
Sorry for interruption, but what do you mean by invalid and valid states
here? Data being loaded or action being executed but not yet finished look
like a valid transient state to me.
|
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'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. |
@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 |
Why do we need complete transaction support? I don't think I understand the use case. |
@gaearon I'm not really sure yet but would be happy to know more of @winstonewert usecase. The idea is that you could do 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? |
Would a simple reducer enhancer work? e.g. const enhanceReducerWithTheAbilityToConsumeMultipleActions = (reducer =>
(state, actions) => (typeof actions.reduce === 'function'
? actions.reduce(reducer, state)
: reducer(state, actions)
)
) With that, you can dispatch an array to the store. The enhancer unpacks individual action and feeds it to each reducer. |
Yes, and it exists: https://github.com/tshelburne/redux-batched-actions |
Ohh @gaearon I did not know that. I did not notice there were 2 distinct projects that try to solve a quite similar usecase in different ways:
Both will permit to avoid unnecessary renderings, but the 1st one would rollback all the batched actions while the 2nd one would only not apply the failing action. |
@gaearon Ouch, my bad for not looking at that. 😳 Action Creators represent Impure CodeI probably haven’t had as much hands-on experience with Redux as most people, but at first sight, I have to disagree with “do more in action creators and do less in reducers,” I’ve had some similar discussion inside our company. In Hacker Way: Rethinking Web App Development at Facebook where the Flux pattern is introduced, the very problem that leads to Flux being invented is imperative code. In this case, an Action Creator that does I/O is that imperative code. We’re not using Redux at work, but at where I work we used to have fine grained actions (that, of course, all make sense on its own) and trigger them in a batch. For example, when you click on a message, three actions are triggered: Then it turns out that these “low-level” actions are no more than a “command” or “setter” or “message” to set some value inside a store. We might as well go back and use MVC and end up with simpler code if we keep doing it like this. In a sense, Action Creators represent impure code, while Reducers (and Selectors) represent pure code. Haskell people have figured out that it’s better to have less impure code and more pure code. For instance, in my side project (using Redux), I use webkit’s speech recognition API. It emits
I went with number two: Just send the raw event object into the store. However, it seems that Redux dev-tools doesn’t like it when non-plain objects are sent into the store, so I added some tiny logic in the action creator to transform these event objects into plain objects. (The code in the action creator is so trivial that it can’t go wrong.) Then the reducer can combine these very primitive events and build up the transcript of what’s spoken. Because that logic lives inside pure code, I can very easily tweak it live (by hot-reloading the reducer). |
I’d like to support @dtinth. Actions should represent events that happened from the real world, not how we want to react to these events. In particular, see CQRS: we want to log as much detail about a real life events, and likely the reducers will be improved in the future and process old events with new logic. |
@dtinth @denis-sokolov I agree with you too on that. Btw when I was referencing the redux-saga project I maybe not made it clear that I'm against the idea of making the actionCreators grow and be more and more complex over time. The Redux-saga project is also an attempt to do what you are describing @dtinth but there is a subtile difference with what you both say. It seems you want to say if you write every raw event that happened to the action log then you can easily compute any state from the reducers of this action log. This is absolutly true, and I've taken that path for a while until my app became very hard to maintain because the action log became not explicit, and reducers too complex over time. Maybe you can look at this point of the original discussion that lead to Redux saga discussion: paldepind/functional-frontend-architecture#20 (comment) Usecase to solveImagine you have a Todo app, with the obvious TodoCreated event. Then we ask you to code an app onboarding. Once the user creates a todo, we should congratulate him with a popup. The "impure" way:This is what @bvaughn seems to prefer function createTodo(todo) {
return (dispatch, getState) => {
dispatch({type: "TodoCreated",payload: todo});
if ( getState().isOnboarding ) {
dispatch({type: "ShowOnboardingTodoCreateCongratulation"});
}
}
} I don't like this approach because it makes the action creator highly coupled to the app view's layout. It assumes the actionCreator should know the structure of the UI state tree to take its decision. The "compute everything from raw events" way:This is what @denis-sokolov @dtinth seems to prefer: function onboardingTodoCreateCongratulationReducer(state = defaultState, action) {
var isOnboarding = isOnboardingReducer(state.isOnboarding,action);
switch (action) {
case "TodoCreated":
return {isOnboarding: isOnboarding, isCongratulationDisplayed: isOnboarding}
default:
return {isOnboarding: isOnboarding, isCongratulationDisplayed: false}
}
} Yes you can create a reducer that knows if the congratulation should be displayed. But then you have a popup that will be displayed without even an action saying that the popup has been displayed. In my own experience doing that (and still have legacy code doing that) it is always better to make it very explicit: NEVER display the congratulation popup if no action DISPLAY_CONGRATULATION is fired. Explicit is much easier to maintain than implicit. The simplified saga way.The redux-saga uses generators and may look a bit complicated if you are not used to but basically with a simplified implementation you would write something like: function createTodo(todo) {
return (dispatch, getState) => {
dispatch({type: "TodoCreated",payload: todo});
}
}
function onboardingSaga(state, action, actionCreators) {
switch (action) {
case "OnboardingStarted":
return {onboarding: true, ...state};
case "OnboardingStarted":
return {onboarding: false, ...state};
case "TodoCreated":
if ( state.onboarding ) dispatch({type: "ShowOnboardingTodoCreateCongratulation"});
return state;
default:
return state;
}
} The saga is a stateful actor that receive events and may produce effects. Here it is implemented as an impure reducer to give you an idea of what it is but it actually is not in redux-saga project. Complicating a bit the rules:If you take care of the initial rule it is not very explicit about everything. Can you see how the code would become messy in all 3 implementations over time as the onboarding becomes more and more complicated? The redux-saga wayWith redux-saga and the above onboarding rules, you would write something like function* onboarding() {
while ( true ) {
take(ONBOARDING_STARTED)
take(TODO_CREATED)
put(SHOW_TODO_CREATION_CONGRATULATION)
take(ONBOARDING_ENDED)
}
} I think it solves this usecase in a much simpler way than the above solutions. If I'm wrong please give me your simpler implementation :) You talked about impure code, and in this case there is no impurity in the Redux-saga implementation because the take/put effects are actually data. When take() is called it does not execute, it returns a descriptor of the effect to execute, and at some point an interpreter kicks in, so you don't need any mock to test the sagas. If you are a functional dev doing Haskell think Free / IO monads. In this case it permits to:
It can also provide an interpretation layer, permitting to translate raw events into more meaningful/high-level events (a bit like ELM does by wrapping events as they bubble up). Examples:
If you want to achieve a modular app layout with ducks, it can permit to avoid coupling ducks together. The saga becomes the coupling point. The ducks just have to know of their raw events, and the saga interpret these raw events. This is far better than having duck1 dispatching directly actions of duck2 because it makes duck1 project more easy to reuse in another context. One could however argue that the coupling point could also be in actionCreators and this is what most people are doing today. |
@slorber This is an excellent example! Thanks for taking the time to explain the benefits and drawbacks of each approach clearly. (I even think that should go in the docs.) I used to explore a similar idea (which I named “worker components”). Basically, it’s a React component that doesn’t render anything ( |
Lots of discussion here while I was sleeping. @winstonewert, you raise a good point about time-travel and replaying of buggy code. I think certain types of bugs/changes won't work with time-travel either way, but I think overall you're right. @dtinth, I'm sorry, but I'm not following along with most of your comment. Some part of your action-creator/reducer "ducks" code has to be impure, in that some part of it has to fetch data. Beyond that you lost me. One of the primary purposes of my initial post was just one of pragmatism.
I'm pretty sure we're thinking about different things. When I said "transient invalid state" I was referring to a use-case like the following. For example, myself and a colleague recently released redux-search. This search middleware listens for changes to collections of searchable things and then (re-)indexes them for search. If the user supplies filter text, redux-search returns the list of resource uids that match the user's text. So consider the following: Imagine your application store contains a few searchable objects:
I don't understand what you mean by the approach coupling the action-creator "to the app view's layout". The state tree drives (or informs) the UI. That's one of the major purposes of Flux. And your action-creators and reducers are, by definition, coupled with that state (but not with the UI). For what it's worth the example code you wrote as something I prefer is not the sort of thing I had in mind. Maybe I did a poor job of explaining myself. I think a difficulty in discussing something like this is that it typically doesn't manifest in simple or common examples. (For example standard the TODO MVC app is not complex enough for nuanced discussions like this.) Edited for clarity on the last point. |
Btw @slorber here is an example of what I had in mind. It's a bit contrived. Let's say your state has many nodes. One of those nodes stores shared resources. (By "shared" I mean resources that cached locally and accessed by multiple pages within your application.) These shared resources have their own action-creators and reducers ("ducks"). Another node stores information for a particular application page. Your page also has its own duck. Let's say your page needed to load the latest and greatest Thing and then allow a user to edit it. Here's an example action-creator approach that I might use for such a situation: import { fetchThing, thingSelector } from 'resources/thing/duck'
import { showError } from 'messages/duck'
export function fetchAndProcessThing ({ params }): Object {
const { id } = params
return async ({ dispatch, getState }) => {
try {
await dispatch(fetchThing({ id }))
const thing = thingSelector(getState())
dispatch({ type: 'PROCESS_THING', thing })
} catch (err) {
dispatch(showError(`Invalid thing id="${id}".`))
}
}
} |
No. I wouldn't write an action creator that dispatches two actions. I'd define a single action that did two things. The OP seems to prefer action creators that dispatch smaller actions which allows the transient invalid states I dislike.
This is actually the kind of case that bothers me. To my mind, the index would ideally be something handled entirely by the reducer or a selector. Having to dispatch extra actions to keep the search up-to-date seems a less pure use of redux. |
Not exactly. I'd favor single actions when in regard to your action-creator's node of the state-tree. But if a single, conceptual user "action" affects multiple nodes of the state-tree then you'll need to dispatch multiple actions. You can separately invoke each action (which I think is bad) or you could have a single action-creator dispatch the actions (the redux-thunk way, which I think is better because it hides that information from your view layer).
You're not dispatching extra actions. Search is a middleware. It's automatic. But there does exist a transient state when the two nodes of your tree do not agree. |
@bvaughn Oh, sorry for being such a purist! Well, impure code has to do with data fetching and other side-effects/IO, whereas pure code can not trigger any side effect. See this table for comparison between pure and impure code. Flux best practices says that an action should “describe a user’s action, are not setters.” Flux docs also hinted further where these actions are supposed to come from:
Basically, actions are facts/data that describes “what happened,” not what should happen. Stores can only react to these actions synchronously, predictably, and without any other side effect. All other side effects should be handled in action creators (or sagas 😉). ExampleI’m not saying this is the best way or better than any other way, or even a good way. But this is what I currently consider as best practice. For example, let’s say the user wants to view the scoreboard which requires connection to a remote server. Here’s what should happen:
Assuming actions can only reach the store as a result of user’s action or server response, we can create 5 actions.
These 5 actions are sufficient to handle all the requirements above. You can see that the first 4 actions have nothing to do with any "duck". Every action only describes what happened in the outside world (user wants to do this, server said that) and can be consumed by any reducer. We also don’t have
They are glued together with these action creators: function viewScoreboard () {
return async function (dispatch) {
dispatch({ type: 'SCOREBOARD_VIEW' })
try {
const result = fetchScoreboardFromServer()
dispatch({ type: 'SCOREBOARD_FETCH_SUCCESS', result })
} catch (e) {
dispatch({ type: 'SCOREBOARD_FETCH_FAILURE', error: String(e) })
}
}
}
function closeScoreboard () {
return { type: 'SCOREBOARD_CLOSE' }
} Then each part of the store (governed by reducers) can then react to these actions:
As you can see, a single action can affect many parts of the store. Stores are only given high-level description of an action (what happened?) rather than a command (what to do?). As a result:
|
No worries. :) I appreciate the further clarification. It actually sounds like we're agreeing here. Looking at your example action-creator,
While I agree with this, I think that it often makes more pragmatic sense to test them together. It's likely that either your action or or your reducer (maybe both) are super simple and so the return-on-effort value of testing the simple one in isolation is kind of low. That's why I proposed testing the action-creator, reducer, and related selectors together (as a "duck"). |
That's precisely where I think what you are doing differs from what is considered best practices for redux. I think the standard way is to have one action which multiple nodes of the state tree respond to. |
Ah, interesting observation @winstonewert. We've been following a pattern of using unique type-constants for each "ducks" bundle and so by extension, a reducer only response to actions dispatched by its sibling action-creators. I'm honestly not sure how I feel, initially, about arbitrary reducers responding to an action. It feels like a little like bad encapsulation. |
@bvaughn can you point to any decent example of testing action, reducer, selector in the same test as you describe here?
|
Hi @morgs32 😄 This issue is a bit old and I haven't used Redux in a while. There is a section on the Redux site about Writing Tests that you may want to check it out. Basically I was just pointing out that- rather than writing tests for actions and reducers in isolation- it can be more efficient to write them together, like so: import configureMockStore from 'redux-mock-store'
import { actions, selectors, reducer } from 'your-redux-module';
it('should support adding new todo items', () => {
const mockStore = configureMockStore()
const store = mockStore({
todos: []
})
// Start with a known state
expect(selectors.todos(store.getState())).toEqual([])
// Dispatch an action to modify the state
store.dispatch(actions.addTodo('foo'))
// Verify the action & reducer worked successfully using your selector
// This has the added benefit of testing the selector also
expect(selectors.todos(store.getState())).toEqual(['foo'])
}) This was just my own observation after using Redux for a few months on a project. It's not an official recommendation. YMMV. 👍 |
"Do more in action-creators and less in reducers" What if application is server&client and server must contain business logic and validators? |
I have some different opinions. My choice is My action creators just dispatch actions(async, sync, serial-async, parallel-async, parallel-async in for-loop) based on some promise middleware . My reducers split into many small state slices to handle the business logic. use Yes, there are some cross-state cases in which the meanings About Do more in action-creators and less in reducers.
About Unit testing
To test |
I agree with you @mrdulin that's now the way I've gone too. |
I'm really not liking the "rails"-style directory structure. Now that I'm beginning to have d14rs I'm beginning to need some of the business logic/game-ified stuff and I'm having trouble finding the right place to plug in the impure parts, that will keep the task of ranking from feeling like a chore. I've read a lot of posts about how to best go about writing a redux app that's slightly more complicated than can be delivered with the example docs. As part of that I've learned that the shape of the state tree should be pretty well abstracted from the rest of the application. Since you use reducers to turn actions into state mutations, you should also use selectors so no individual part of the UI (or you coding it) has to care much about where your state is stored, so long as it is, and you can deterministically get it back. The docs suggested method of keeping your "containers" and your "components" separate makes sense from a code standard. It's useful to not tightly couple how you get your data to how you render the data in a UI, so you can write tests around your storage of the data, and write tests around rendering your UI. But, whether a node knows how to map state, or dumbly works on props, is an implementation for THAT component. No one up the tree, or down the tree, should have to care. But in working on this commit, I've been bitten every time I changed a component to be wrapped in a container, because watch updates, and I refresh in the browser and nothing changes in the render. Going and changing the include path for anywhere that renders a component will end up more painful than the reminder to separate concerns is worth. So, I think I'm going to migrate to module/domain/feature style organization. But each component will use default export to declare itself, and inside the file will determine whether that export is a regular react component, or a connected one. I'm going to try to export a destructurable { Component } so that I can write UI tests on just the component so I don't have to find some clever way to intercept the state mapping step, and just pass props in instead. From reading the conversation in this pull request: reduxjs/redux#1171 and this blog post: http://randycoulman.com/blog/2016/09/27/modular-reducers-and-selectors/ I'm frustrated that somehow module organization means that you disregard the recommendation to not pair your reducers and action creators. Since the purpose of using reducers and selectors is to abstract away the shape of the state tree as an implementation detail, it sounds to me like it's appropriate to have a "state"/"reducer" module that handles building and accessing your state. This may just be hubris, and I may come back admitting my faults and wrongs.
@mrdulin Yeah. It looks like middleware is the right place to place your impure logic. Much simpler choice is just call some pure functions/class methods from the middleware: middleware = (...) => {
// if(action.type == 'HIGH_LEVEL')
handlers[action.name]({ dispatch, params: action.payload })
}
const handlers = {
async highLevelAction({ dispatch, params }) {
dispatch({ loading: true });
const data = await api.getData(params.someId);
const processed = myPureLogic(data);
dispatch({ loading: false, data: processed });
}
} |
My case against using selectors everywhere, even for trivial pieces of state that don't require memoization or any sort of data transformation:
Obviously selectors are necessary, but I'd like to hear some other arguments for making them a mandatory API. |
From a practical point of view, one good reason in my experience is that libraries such as reselect or redux-saga leverage selectors to access pieces of state. This is enough for me to stick with selectors. Philosophically speaking, I always see selectors as the "getter methods" of the functional world. And for the same reason why I would never access public attributes of a Java object, I would never access substates directly in a Redux app. |
@IceOnFire There's nothing to leverage if the computation isn't expensive or data transformation isn't required. Getter methods might be common practice in Java but so is accessing POJOs directly in JS. |
Selectors are a reducer's query (read) public API, actions are a reducer's command (write) public API. Reducer's structure is its implementation detail. Selectors and actions are used in the UI layer, and in the saga layer (if you use redux-saga), not in the reducer itself. |
@sompylasar Not sure I follow your point of view here. There is no alternative to actions, I must use them to interact with redux. I don't have to use selectors however, I can just pick something directly from the state when it is exposed, which it is by design. You're describing a way to think about selectors as a reducer's "read" API, but my question was what justifies making selectors a mandatory API in the first place (mandatory as in enforcing that as a best practice in a project, not by library design). |
@timotgl Yes, selectors are not mandatory. But they make a good practice to prepare for future changes in the app code, making possible to refactor them separately:
When you're about to change the store structure, without selectors you'll have to find and refactor all the places where the affected pieces of state are accessed, and this could potentially be a non-trivial task, not simply find-and-replace, especially if you pass around the fragments of state obtained directly from the store, not via a selector. |
@sompylasar Thanks for your input.
That's a valid concern, it just seems like an expensive tradeoff to me. I guess I haven't come across a state refactoring that caused such problems. I have come across "selector spaghetti" however, where nested selectors for every trivial sub-piece of state caused quite the confusion. This counter-measure itself has to be maintained as well after all. But I understand the reason behind this better now. |
@timotgl A simple example that I can share publicly: export const PROMISE_REDUCER_STATE_IDLE = 'idle';
export const PROMISE_REDUCER_STATE_PENDING = 'pending';
export const PROMISE_REDUCER_STATE_SUCCESS = 'success';
export const PROMISE_REDUCER_STATE_ERROR = 'error';
export const PROMISE_REDUCER_STATES = [
PROMISE_REDUCER_STATE_IDLE,
PROMISE_REDUCER_STATE_PENDING,
PROMISE_REDUCER_STATE_SUCCESS,
PROMISE_REDUCER_STATE_ERROR,
];
export const PROMISE_REDUCER_ACTION_START = 'start';
export const PROMISE_REDUCER_ACTION_RESOLVE = 'resolve';
export const PROMISE_REDUCER_ACTION_REJECT = 'reject';
export const PROMISE_REDUCER_ACTION_RESET = 'reset';
const promiseInitialState = { state: PROMISE_REDUCER_STATE_IDLE, valueOrError: null };
export function promiseReducer(state = promiseInitialState, actionType, valueOrError) {
switch (actionType) {
case PROMISE_REDUCER_ACTION_START:
return { state: PROMISE_REDUCER_STATE_PENDING, valueOrError: null };
case PROMISE_REDUCER_ACTION_RESOLVE:
return { state: PROMISE_REDUCER_STATE_SUCCESS, valueOrError: valueOrError };
case PROMISE_REDUCER_ACTION_REJECT:
return { state: PROMISE_REDUCER_STATE_ERROR, valueOrError: valueOrError };
case PROMISE_REDUCER_ACTION_RESET:
return { ...promiseInitialState };
default:
return state;
}
}
export function extractPromiseStateEnum(promiseState = promiseInitialState) {
return promiseState.state;
}
export function extractPromiseStarted(promiseState = promiseInitialState) {
return (promiseState.state === PROMISE_REDUCER_STATE_PENDING);
}
export function extractPromiseSuccess(promiseState = promiseInitialState) {
return (promiseState.state === PROMISE_REDUCER_STATE_SUCCESS);
}
export function extractPromiseSuccessValue(promiseState = promiseInitialState) {
return (promiseState.state === PROMISE_REDUCER_STATE_SUCCESS ? promiseState.valueOrError || null : null);
}
export function extractPromiseError(promiseState = promiseInitialState) {
return (promiseState.state === PROMISE_REDUCER_STATE_ERROR ? promiseState.valueOrError || true : null);
} It's not this reducer user's concern whether what's being stored is
If this nesting was caused by mirroring the reducer nesting (composition), that's not what I'd recommend, that's that reducer's implementation detail. Using the above example, the reducers that use promiseReducer for some parts of their state export their own selectors named according to these parts. Also not every function that looks like a selector has to be exported and be part of the reducer API. function extractTransactionLogPromiseById(globalState, transactionId) {
return extractState(globalState).transactionLogPromisesById[transactionId] || undefined;
}
export function extractTransactionLogPromiseStateEnumByTransactionId(globalState, transactionId) {
return extractPromiseStateEnum(extractTransactionLogPromiseById(globalState, transactionId));
}
export function extractTransactionLogPromiseErrorTransactionId(globalState, transactionId) {
return extractPromiseError(extractTransactionLogPromiseById(globalState, transactionId));
}
export function extractTransactionLogByTransactionId(globalState, transactionId) {
return extractPromiseSuccessValue(extractTransactionLogPromiseById(globalState, transactionId));
} |
Oh one more thing that I almost forgot. Function names and exports/imports can be minified well and safely. Nested object keys — not so much, you need proper data flow tracer in the minifier to not screw up the code. |
@timotgl : a lot of our encouraged best practices with Redux are about trying to encapsulate Redux-related logic and behavior. For example, you can dispatch actions directly from a connected component, by doing You can also hand-write action types everywhere, but we encourage defining constant variables so that they can be imported, traced, and decrease the chance of typos. Similarly, there's nothing preventing you from accessing So no, you don't have to use selectors, but they're a good architectural practice. You might want to read through my post Idiomatic Redux: Using Reselect Selectors for Encapsulation and Performance. |
@markerikson I'm not arguing against selectors in general, or selectors for expensive computations. And I never intended to pass dispatch itself to a component :) My point was that I disagree with this belief:
About your example with On other hand, if you follow this guideline to the letter, you're also doing Under certain conditions I could see why you would want to err on the side of safety and conventions though. Thanks for your time and engagement, I'm overall very happy we have redux. |
Sure. FWIW, there are other selector libraries, and also wrappers around Reselect as well. For example, https://github.com/planttheidea/selectorator lets you define dot-notation key paths, and it does the intermediate selectors for you internally. |
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)
overstate.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:
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:
updateMetadata
action. This approach is terrible for (hopefully) obvious reasons.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.
The text was updated successfully, but these errors were encountered: