Decide whether we need formal asynchronous state patterns

[aduth]

Related: #594

We'll want to decide if we need to establish patterns around managing asynchronous state data, or if it's enough to pass around dispatch as an argument as included in #594. This is a relatively unsettled problem in Redux, with some consensus around thunks.

http://redux.js.org/docs/advanced/AsyncActions.html#async-action-creators

So far we've made a conscious effort at keeping concepts simple in our state, forgoing some patterns altogether such as constant action types, action creators, middlewares, and selectors. With this in mind we should be cautious about introducing any new patterns unless they can be demonstrably shown to provide benefit outweighing the overhead of their learning curve.

Quoting below a comment I'd shared previously at #594 (comment)

Already noted by @youknowriad that we'll need to consider adopting a pattern for asynchronous data flow. This file is perhaps a bit misleading since at a glance I'd expect it to include a set of action creators, but this isn't a true action creator in that it accepts dispatch and doesn't return an action object.

type ActionCreator = (...args: any) => Action | AsyncAction

http://redux.js.org/docs/Glossary.html#action-creator

I'm fine to flesh this out separately; redux-thunk is a popular option, but I've become less a fan of it over time, since thunks are hard to test, less predictable, and with the default implementation, encourage access to state via the second getState argument (thus breaking the unidirectional data flow).

If I were to suggest an alternative, I think it shouldn't be much trouble to devise a pattern to encourage treating actions with asynchronous effects as expressing intent. Using middleware(s), those intents could be mapped to side effects which themselves may incur additional actions.

Roughly something like: https://gist.github.com/aduth/b36ac70f0d3cd3f61faa0b2b81b98a2a

Not set in stone, specifically:

• Couldn't savePost just return REQUEST ? Maybe we don't need a separate effects mapping. But sometimes the save intent isn't always just about firing a request, but affecting reducer values (e.g. SAVE_POST provides semantic meaning that of just REQUEST which should cause the UI to update to reflect in-progress save)
• Instead of an effect's success property, we could enforce that the request completion itself should be an action expressing intent REQUEST_COMPLETED that has its own effect(s) to cause receivePost to be dispatched
• Maybe we forgo the REQUEST middleware and treat effects as mini-middlewares which have access to dispatch and could just use fetch or wp-api.js in their implementations

Anyways, this has been a bit of a brain dump that'll get lost in pull request history, but jotting down now that it's becoming relevant since this has been on my mind lately.

A few additional resources from side projects where I've been applying these ideas:

• https://github.com/aduth/dones/blob/master/src/state/requests/middleware.js
• https://github.com/aduth/dones/blob/master/src/state/dones/effects.js
• https://github.com/aduth/refx

[BE-Webdesign]

Big +1 for refx, especially if RxJS is not used.

[aduth]

Pulling in some discussion from #769

Do you have any opinions on this? I've actually been leaning toward thunks a bit; even if I personally dislike their data "impurity", it's hard to argue that they're not simple to use, which could be a big win for what's already a complicated setup.

#769 (comment)

@aduth I have a personal preference for generators, redux-saga is a bit complex though, so I use a personal library in general https://github.com/youknowriad/redux-rungen. But this is a lot to handle I think, requires a certain mastery of generators. Thus, I'm also leaning towards thunks to avoid having a steep learning curve.

#769 (comment)

[aduth]

Big +1 for refx, especially if RxJS is not used.

@BE-Webdesign Could you point to some examples on how an RxJS approach (or observables generally) would look? A goal with refx was to treat side effects as almost incidental to the actions occurring in the system, behaving as observers of sorts. I'm curious to what extent this overlaps with a solution using observables proper.

A key worry for me here is colocation. Thunks are at least nice in that actions and side effects occur in the same place. Since a reason many view Redux as daunting is due to its separation between components, actions, and reducers (and selectors, and action types), it's a fair argument to say that thunks improve developer experience by keeping things simpler at smaller scale.

I have a personal preference for generators

@youknowriad Generators intrigue me. Ignoring the fact that they don't (and likely won't) see much usage and are therefore unfamiliar to most, they seem to be a good fit for what we need an asynchronous pattern for; namely, functions (action creators) which can yield many values (actions) over time. What I never understood about sagas is why there need be so many helper utilities. I think I can see purpose for the fork and join utilities in your rungen library, but why couldn't a generator-based action creator simply yield action objects directly?

async function* fetchPost( postId ) {
	yield { type: 'FETCH_POST_START', postId };

	try {
		const response = await fetch( `/wp/posts/${ postId }` );
		const post = await response.json();
		yield { type: 'RECEIVE_POST', post };
		yield { type: 'FETCH_POST_SUCCESS', postId };
	} catch ( error ) {
		yield { type: 'FETCH_POST_ERROR', postId, error };
	}
}

(Requires Stage 3 Async Iterators)

[youknowriad]

but why couldn't a generator-based action creator simply yield action objects directly?

This is a choice from the library to make it explicit and in the same time allow yielding other values.
In your example, you're mixing async/await and yield, I'm not saying this is bad, but async/await is a special case of generators. In your example your could just replace await with yield, drop the async and have your runtime "smart" enough to resolve the promise value if he detect that the yielded value is a promise.

So, to make it explicit if we should dispatch or not the yielded value, we have a put or dispatch helper.

In the same way, other helpers could serve other purposes. For example we could have a helper take( channel ) to get values from a channel (a channel could be a websocket or any channel we could subscribe to, a redux store is a channel for example) etc...

[nylen]

From a quick look, I really like https://github.com/aduth/refx: it seems to preserve most of the simplicity of thunks while improving testability. Increasing separation is a relatively minor concern, IMO far outweighed by improved testability. It also makes sense to me to locate all side-effects together in a similar place.

I think JS generators are nicer in theory than in practice and would prefer that we don't introduce them here. I think Python got generators mostly right and JavaScript got them mostly wrong. They are pretty complicated, and feel half-baked and poorly integrated with the rest of ES6. For a small example, you can't use the ( lambda ) => {} syntax with functions that contain generators.

[youknowriad]

I think JS generators are nicer in theory than in practice and would prefer that we don't introduce them here. I think Python got generators mostly right and JavaScript got them mostly wrong.

I don't agree that JS generators are bad. I feel they are really powerful, but I think generators, in general, have a steep learning curve. I do think it's the nicest approach I've seen to handle side-effects and asynchronous code in Javascript, but in a wide project, I would argue against their use just because of the learning curve. It's not a feature used much by lambda developers.

[BE-Webdesign]

This would be using an extra library which turns the redux store into an observable, and adds the idea of an Epic, which is basically the pull equivalent to a redux Saga.

// action creators; roughly using the same naming and conventions in gutenberg.
const savePost = ( edits, postId = '' ) => {
    const isNew = ! postId;
    const editsToSend = postId ? { id: postId, ...edits } : edits;

    return { type: POST_REQUEST_UPDATE, edits: editsToSend, isNew };
};
const savePostFulfilled = ( post, isNew ) => ( { type: POST_REQUEST_UPDATE_SUCCESS, post, isNew } );
const savePostFailure = ( err, edits, isNew ) => {
    return { type: POST_REQUEST_UPDATE_FAILURE, err, edits, isNew };
};

// epic
const updatePostEpic = action$ =>
  action$.ofType( POST_REQUEST_UPDATE )
    .mergeMap( action => {
        return Observable.fromPromise( new wp.api.models.Post( action.edits ).save().done() )
            .map( post => savePostFulfilled( post, action.isNew ) )
            .catch( err => savePostFailure( err, action.edits, action.isNew ) )
            // If the request failed try it again if online every second, if offline try again when online.
            .retryWhen( err => window.navigator.online ?
                Observable.timer( 1000 ) :
                Observable.fromEvent( window, 'online' )
    } );

// On the publish button click.
dispatch( savePost( state.editor.edits ) );

The advantages of using RxJS or a Saga based middleware are the ability to manage complexity more easily ( notice how trivial it is to add offline handling, you could even slap in local storage and cache handling stuff ). If we needed to throttle network requests, that is trivial as well ( simply chain the throttle call at the end of mergeMap ), not so much for thunks or refx. For right now, RxJS or Sagas are not necessary or adding much value ( I finally relent on my Rx crusade ).

I like refx, because it is light weight ( both in learning, and in code length ) and helps split the side effects from the actual action creators, and in my opinion an easier solution than thunks. One advantage of thunks over refx is that the action flow is more explicit, which could make it easier to track flow as complexity grows. So either thunks or refx are the way to go in my book. If we get into heavy duty async, like collaboration tools, I think that is when Rx or something should be introduced.

Rx should be a definite consideration if/when auto drafting behavior is introduced.

[aduth]

Pulling in remarks from @BE-Webdesign at #769 (comment) :

Do you have any opinions on this? I've actually been leaning toward thunks a bit; even if I personally dislike their data "impurity", it's hard to argue that they're not simple to use, which could be a big win for what's already a complicated setup.

I can see refx being more valuable than thunks. Thunks can pin you into a corner when you want to have dispatched actions trigger other side effects, rather than having whole new action creators added to the codebase. By using a refx-esque approach you can incrementally build complexity on top of the action flow already happening. When auto drafting is implemented, that would be somewhat tricky to do in thunks, but pretty easy to manage with refx. The downside is that it might be more hidden as to what is actually happening when using a library like refx, as the flow of actions is no longer hard coupled to the action creators' functions. Even considering that, I think refx is the right choice, as it will be more flexible and easy to use and maybe the code can document that refx side effects are occurring. If a developer is new to Redux, thunks and refx will both be foreign, so trying to pick a simple to use solution won't provide much benefit to them in my opinion. Even though thunks are more commonplace (probably who knows), I think refx is a better choice and is equally simple to use.

I'd say a good bit of the simplicity in thunks is merely colocation, which is lost when you move effects into a separate file. But I agree that an action observer pattern is one which would set us on a better footing for the future.

To the colocation point, I was recently reintroduced to the Redux Ducks convention which is a rather extreme take on colocation, while flexible in supporting a multitude of side effects patterns.

[BE-Webdesign]

I'd say a good bit of the simplicity in thunks is merely colocation,

With the Ducks pattern, as long as things are self containing, it would be pretty easy to add a combineEffects() helper to the refx library, so the side effects for an action can all be colocated. Then the effects of each action can be combined into a rootEffects when the middleware is created. My two cents still leans towards using refx. You wouldn't even have to go full Ducks, you could just locate the side effects that correspond to an action in the same file as that action creator.