reduxjs / redux

A JS library for predictable global state management
https://redux.js.org
MIT License
60.65k stars 15.28k forks source link

async reducer workflow #307

Closed jquense closed 8 years ago

jquense commented 8 years ago

Right now the composition of stores is strictly a sync composition of functions that return state, and any async stuff needs to be put in an actionCreator (with some sort of async middleware like thunk).

Generally this works out nicely, is straightforward and Easy To Reason About™ and all around a pleasant API to work with. I want to say up front that the suggestion here is not due to an inability to get something done in Redux, but a philosophical one about concerns and the their proper place in the flux cycle. I know I can accomplish the following in action creators, my contention is that it not the right place to do it, but the api forces it.

My issue is that I have a few use cases where the "reducing" work of moving one state to a new one is just asynchronous.

  1. heavy computation/webworkers: Needing reduce a large array doing a bunch of hefty calculations, that needs to be split over a few event loop ticks. Not super common, but it happens often enough in our apps. some real examples: doing a bunch of fuzzy text matching against 15000 contact names. Big D3 graph stuff.
  2. model validation/parsing: this is more controversial but the following example is common to us. I am not talking about the fetching of items from the server, but about taking an item out of state, putting it in a form, and then updating that item later. These items may or may not be saved to the server.

    schema.validateAndCast(formValue)
     .then( castValue => state.collection.push(castValue))
     .catch( err => state.errors.push(err))

    In general I think validation stuff is going to be sitting your action creators as long as you are doing to server syncing there (and I think you should). Often, in addition to that we have wanted helpers we use to update state in reducers to be able to validate and parse the objects they are passed in, since this is part of the reducing process.

to the broader point, there is no reason that "reducing" as an action must or should be synchronous. It can be frustrating to have to split "business" logic for managing state into a bunch of places, when there is a really good place for it already!

On redux's side most of this could be addressed by providing a single hook somewhere around here: https://github.com/gaearon/redux/blob/breaking-changes-1.0/src/createStore.js#L96. If there was some sort of callback hook to set currentState then anyone could create their own async combineReducers however they wish.

heyimalex commented 8 years ago

It seems like actions wouldn't be replayable? You could have alternate versions of history depending on when the async work completed.

Definitely agree that keeping the reducers synchronous makes writing redux-friendly async things (that aren't just dispatch middleware and really should't be higher order stores) more difficult because you need to cleave out the async logic from the sync logic, but I feel like it's the price you pay. Instead of allowing asynchrony in the stores, maybe we should just think about how to write async code in a way that doesn't feel so wrong?

gaearon commented 8 years ago

maybe we should just think about how to write async code in a way that doesn't feel so wrong?

+1. Rx can help a lot here, as Observable is the best primitive for composable streams of values.

gaearon commented 8 years ago

That said I'm also interested in “remote stores” that execute work in web workers. Related: #211, #264.

The nice thing about them is maybe we don't even need them to be in the core, as long as the third-party “remote client store” implements { dispatch, getState, subscribe } interface. I'd be interested to see a proof of concept of this. (Note that “remote server store” would still be a sync reducer, but running in a different environment.)

sergey-lapin commented 8 years ago

@gaearon I made an implementation of this based on thoughts from https://github.com/gaearon/redux/issues/211. Repo is a mess right now but it works! http://lapanoid.github.io/redux-remote/

jquense commented 8 years ago

so async stuff doesn't have to non deterministic in ordering, you can just have it resolve in order same as it does now, you don't have to parallelize it. think connect Middleware. to me that maintains the same level of reasonability as the sync version

wmertens commented 8 years ago

How about letting middleware create promises that are stored in the state (via a new action or by augmenting the original) and that have resolution handlers that dispatch more actions as appropriate (via that middleware's scoped dispatch)?

That way all side effect code lives in middleware and all state lives in the Redux store instead of being hidden in invisible async callbacks.

The reducer that stores the promise can chain it on existing promises or can inject the current state.

wmertens commented 8 years ago

Example usage: Getting data about item foo123 so it can be displayed to the user

  1. Dispatch action ENSURE_ITEM, "foo123"
  2. Middleware attaches a function checkPromise(promise){...; return newPromise}
  3. Reducer runs the function with the relevant promise related to item foo123
  4. If the promise is null or deemed stale, checkPromise will request data from the server and returns the promise for its result; otherwise the promise is simply returned
  5. Reducer stores the resulting promise, a state change results in re-rendering
  6. The promise dispatches new actions (FETCHING_ITEM, STORE_ITEM, UNKNOWN_ITEM, ..., REMOVE_PROMISE) that store the itemdata. The promise itself cannot be used because its state is hidden and mutable.

When rendering, the itemdata is either null (unknown state), the actual data, or an absence marker if it doesn't exist.

This means reducers and middleware are stateless, actions creators are simple object packagers and side-effect code is in only one place.

jquense commented 8 years ago

I was also thinking of something as simple as:

function myReducer(state, action, next){
  doAsyncReduction(state, next)
}

where next can only be called ONCE. sort of akin to async's waterfall method

wmertens commented 8 years ago

Hmm but then reducers are no longer pure composeable functions…

On Fri, Jul 24, 2015, 18:39 Jason Quense notifications@github.com wrote:

I was also thinking of something as simple as:

function myReducer(state, action, next){ doAsyncReduction(state, next) }

where next can only be called ONCE.

— Reply to this email directly or view it on GitHub https://github.com/gaearon/redux/issues/307#issuecomment-124575294.

Wout. (typed on mobile, excuse terseness)

jquense commented 8 years ago

@wmertens adding asynchronicity to them sort of precludes that I think, you just can't return something from a function asynchronously right now in JS. The callback example is still pure (assuming next() is called once with the return value, and they are composable since the last argument is the callback. They also execute in order same as normal functions. Promises are another way to do this, assuming you want to say that you can't store a promise in state.

heyimalex commented 8 years ago

@jquense Imagine your async store is processing. Some action comes in and now a decision needs to be made based on the current state of your async store. Here the history can diverge; if the store has resolved already we go one way, and if not we go another. Doesn't that break deterministic action-replay?

jquense commented 8 years ago

@HeyImAlex doesn't the example remain the same if you move the async stuff into actionCreator middleware? I think that if this is a problem, then we already have this problem.

function actionCreator(){
 return (dispatch, getState){
    doAsyncThing()
      .then( result => dosomethingBasedOnStateAndDispatch(getState(), dispatch, result))
    doAnotherAsyncThing()
      .then( result => dosomethingBasedOnStateAndDispatch(getState(), dispatch,  result))
 }
}

Assuming doAsyncThing and doAnotherAsyncThing can finish at different times, or waht if the user calls actionCreator again while the first is still processing?

jquense commented 8 years ago

just a quick context point. There are definately downsides to doing async stuff, Im not arguing that the addition of this hook would have no cost, just that it is needed. I don't think it should be the default, or required, but available for the cases where it is useful, i.e. the benefits out weigh the costs

oops hit closed by accident :P

heyimalex commented 8 years ago

@jquense Yes, when you're dealing with the ui there is asynchrony and non-determinism, but the point is that actions are the point at which that asynchrony is made serial. In your example, if you logged all of the actions dispatched in order you would have a replayable time-travelable log of everything that actually happened.

gaearon commented 8 years ago

What makes it hard to discuss is a lack of specific runnable example that you aim to find a simpler way to rewrite.

vladar commented 8 years ago

Also check out this conversation: https://github.com/gaearon/redux/issues/291#issuecomment-125138124 as it seems to be related.

vladap commented 8 years ago

Just to expand on the discussion where code is serialized.

The thing guaranteed in Redux is that given the same action sequence, when replayed, it results in the same Redux.state.

But there is no order defined in the @jquense example executing 2 async operation independently. Hence Redux.state will be updated differently on each app run. F. e., it is still easy to introduce racing bugs. Developer still has to think carefully what he is doing and if the async code in action creators has to be explicitly serialized, f.e. by Promise.all.

It is important to realize that code is not serialized by dispatching actions because if dispatches are async then different application executions will result in different action sequences. It means that different users can see different application states even they will do the same interactions with a browser.

In most cases it is fine. One usually doesn't care that on the first app run Chart1 loads first and is shown first then Chart2 while on the second app run/refresh it is the other way around. In other cases it can matter though.

Additionally there is no ordering of dispatches of different action creators (AC). It can happen that some other async AC1 dispatches action before AC2, but on another app run AC1 dispatches after AC2. If there is a dependency between them and the effect of this async is not desired it has to be resolved before they dispatch. In some cases, it can require a statefull action creator. F. e. user clicks Click1 and then Click2 and for some reason we need to ensure that async operation of Click1 always finishes before Click2 and we want that both has to be executed async, hence disabling Click2 till Click1 finishes is not an option.

EDIT: I have removed italic from "The thing guaranteed in Redux is that given the same action sequence, when replayed, it results in the same Redux.state" to avoid confusion that it is a quote.

heyimalex commented 8 years ago

@vladap There's a distinction between "action" as seen by the store and "action" as in calling an action creator. The former is synchronous and is where actions/changes to the store are serialized, the latter is just an abstraction that will eventually call the synchronous dispatch.

vladap commented 8 years ago

Fact that action term is used to describe too distinct things - signal/intent from a view and a description what happened, is quite confusing. Fact that action creator term is often shortcut to action makes it even more problematic. It leads to huge confusions in discussions. Where have I done this sin in my comment? Am I somewhere wrong/inaccurate? I have paid attention to use action as a description of what happened, a fact on which reducers execute. And action creator as a factory/producer of actions. I haven't been referring to signals from views. Actually, there are no signals/intents from views in Redux because view is proactively executing an action creator which returns a resulting action which view dispatches but an action creator itself can produce many actions in between and call other action creators.

heyimalex commented 8 years ago

Yeah, pretty confusing. Anyways, I... think I just read your comment wrong :sweat_smile: I thought the italicized paragraph was a quote and what followed was a rebuttal. But I think we're in agreement.

jquense commented 8 years ago

The thing guaranteed in Redux is that given the same action sequence, when replayed, it results in the same Redux.state.

my only point is that there (afiact) still a way to preserve this guarantee and allow async reducers. All that is needed is make redux understand promises (or callbacks), this uses: when/keys.map:

function combineReducers(reducers){

  return function (state, action){
    return when.keys.map(
       reducers, reducer => reducer(state, action))
  }
}

Since stores are pure there is no need to make sure they run in sequence, just that they don't start processing another action before the the current one is finished.

The rest is just performance and optimizations

gaearon commented 8 years ago

Fact that action term is used to describe too distinct things - signal/intent from a view and a description what happened, is quite confusing.

Does this differentiation in the new docs help?

http://gaearon.github.io/redux/docs/Glossary.html#action http://gaearon.github.io/redux/docs/Glossary.html#async-action

heyimalex commented 8 years ago

@jquense Not great because you're going to be blocking while the async work happens, ie you're just making your async work synchronous, but it would let you use async apis inside of reducers where right now you can't at all.

jquense commented 8 years ago

Hmm you aren't blocking at all, how would you even do that in javascript? If by blocking you mean that you can't process another action until the current one finishes, then yeah, definitely, that's what you want. It is taking more time (of course its doing async work), but it is by definition non-blocking.

which is to say that it is asynchronous but not parallelized

heyimalex commented 8 years ago

@jquense We're on the same page, I'm just saying that the waiting is blocking updates to state (and by extension the ui); if your async action takes ten seconds, then the ui is going to be completely unresponsive for ten seconds. But I can totally see how it would be helpful to have.

Honestly I just like to argue :smile: I'm not even convinced replay is that important. Event sourcing uses it to get a picture of state at different points in time, but in javascript our state is relatively small and ephemeral and, if our changes are pure, we can just save the whole thing. I do like that actions give context to state changes. But then that's orthogonal to replay.

Plus, like you were getting at, it's imperfect; if an action creator depends on state to dispatch an action, you can't be sure your immutable-list-of-actions-to-replay are even accurate after any code change. And we don't actually need "immutable" actions (as in event sourcing) because we really don't care about keeping a consistent history. All we want is to make development easier.

So I'm in total agreement about this issue. My biggest problem with Redux is that it makes writing reusable self-contained code that has any asynchrony or any side effects very difficult. Cleaving your code into action creator and reducer feels like it's injecting Redux-isms where you really want framework agnosticism.

Redux's main idea is that keeping a pure, synchronous, immutable core and pushing side effects to the edges make state changes easier to reason about. The paradox is how to do that without completely ruining composability...

vladap commented 8 years ago

I know I can accomplish the following in action creators, my contention is that it not the right place to do it, but the api forces it.

@jquense Could you elaborate some more why you think it doesn't belongs to action creators? Example 1 of a heavy web worker feels to me similar as webapi call, just that one goes over a wire and the other is local. But I could move this computation to the server (in some cases it can be faster for slow devices), give it a webapi and then it would be in an action creator. Or I can support both and decide which to use based on a device, then it makes sense to have it in one place for both, actually some pattern would be used to swap implementation in an action creator.

vladap commented 8 years ago

@jquense Could that be ActionCreator makes you uneasy about it? Originally I thought it should be a pure function, a factory which builds action objects. Now I imagine it as action as a function. I don't have trouble to put business logic into something called an action. And when I click a button I can imagine it as that action is executed - which it is and then this button click reduces (dispatches) resulting action objects into new state.

BTW. do I understand correctly we are not actually dispatching? I would expect it as an operation which is indirectly listened to. But we are basically doing Store.reduce(actions). It would look good to have reduce in my views as it is too much general. Maybe more creative... Store.redux(actions). Then my views would use redux operation, a single functions which would represent the whole Redux lib and its main idea. Just thinking.

vladap commented 8 years ago

http://gaearon.github.io/redux/docs/Glossary.html#action http://gaearon.github.io/redux/docs/Glossary.html#async-action

@gaeron Great docs, really.

I just believe that there will be always confusion till there will be action creators and actions. Typical conversation:

... Where should I put this code? ... Put it into action. ... Do you mean action creator? ... Yeah.

And I believe that action creator term make people uneasy to put code here and call other ACs from AC. But without that it is impossible to do some simple serialization of async code. In vanilla Flux it could be alternatively done in stores as far as new data where dispatched by an action. The docs about it doesn't help with it either as they mostly explain AC as simple factory. The last sentence suggests to create async-action for complex logic but it effectively means that code is defined in AC, f.e. using Promise chain.

Second problem is what action object describes. It seems a bit unclear in docs. It is described both as:

The only way to mutate the state is to emit an action, an object describing what happened. (Three Principles)

An action is a plain object that represents an intention to change the state.

How action.type should be chosen? Should the action.type describe what happened (past tense) or how it is intended to change state (imperative mood). Action object can be seen as an extension hook and its original intention when designed can be changed later by adding new functionality hooked to it. When it describes what happened, there is only 1:1 relation to a piece of code.

Maybe it would help to say that Action objects are equivalents to HTML DOM Event Object.

I don't like to use past tense as it can become tricky in English. I like to name them as nouns. INCREMENT_COUNT changes to COUNT_INCREMENT. It is both simple and neutral and many times similar or even the same as imperative mood. Still I like the distinction because it doesn't forces me to think about an action as what should happen. After all DOM events are nouns, nouns allow names like 'contextmenu', and I don't know how I would put 'mouseclick', 'mousedown', 'mouseup' into past tense.

Async-action. It is fine term, I like it the most from previous Intermediate Action or Intent options. Except, how we call action object which doesn't represent async primitive but is still not ready to be reduced to a state - it will go through a middleware to be somehow transformed. Do we call it action? I don't mind to call it just action.

gaearon commented 8 years ago

We can call it "raw action". That is, it's not ready for consumption.

vladap commented 8 years ago

Raw action is fine. Lately, I was thinking to get more creative and forget all actions, facts, events, messages, records and replacing action creators with actions (as functions), and to call action object Atoms. Atom as a smallest piece of state which can be reduced to an app state. Actions produce atoms which atomically capture what happened. I dunno it overloads another meaning, though.

danmaz74 commented 8 years ago

I'll add my two cents here, as someone who is still considering if we should switch from "vanilla Flux" to Redux. I don't like the name ActionCreator a lot, but adopting a terminology that conflicted with Facebook's examples (ie, call the ActionCreator an Action) would make it really difficult for someone coming from there to understand and adopt Redux.

Second, regarding the replayability, that is extremely important for debugging, and also for automated testing. But the replayability will only work for a sequence of dispatched Actions, not ActionCreator calls - Actions (in the original sense) have deterministic effects, while ActionCreator calls don't, exactly because they can include non-deterministic async calls.

Keeping the deterministic code separated from the non-deterministic one has a very big advantage: it makes testing the deterministic actions very easy. To account for the non-deterministic async part of the code, you "simply" need to test the different sequences of actions and the different results that can come from the async calls: test what happens if AsyncCall1 finishes before AsyncCall2, test the opposite, test what happens if AsyncCall1 returns a timeout error, etc.

Being able to do that is one of the most important things that makes me inclined to seriously consider Redux to solve some of the problems we were finding with vanilla Flux.

gaearon commented 8 years ago

I'm closing this issue because I don't see anything actionable here. If you'd like to see a different pattern of handling async requests in Redux, please make a PR showing the core changes you propose, together with async and real-world examples using the proposed API. Because Redux is tiny, this is totally feasible to do. With 5 or 6 issues discussing the same thing but seemingly with no progress, I'm going to close them until implementation proposals start coming up. :-)