jvm-redux / jvm-redux-api

MIT License
22 stars 4 forks source link

Middleware API #8

Closed pardom-zz closed 8 years ago

pardom-zz commented 8 years ago

Although Redux can be used without middleware, it's such a core Enhancer that it ships with Redux.js. One of the main concerns that this common API aims to address is the creation of a single Redux ecosystem, which mainly comprises of middleware. For this reason, we need to also provide a middleware API.

There are several ways we may want to provide this API:

These options all vary on a scale of modularity vs simplicity. I propose that we include it in this module, but in an enhancer package.

The API is quite simple, but requires some though on naming. I suggest the following.

package redux.api.enhancer;

interface Middleware<S> {

    Object dispatch(StateProvider<S> stateProvider, Object action, Dispatcher next);

    interface StateProvider<S> {

        S getState();

    }

}

The StateProvider class requires a bit of explanation, and I'll take suggestions on what to actually call this interface to reduce the confusion about it. The dispatch method needs to provide the current state to the middleware. It could be provided by passing in the Store, however, that would expose the opportunity to do unsafe things in the middleware, like replace the reducer or dispatch an action.

The original JS implementation does something similar by passing the store's dispatch function as an argument instead of the store itself.

For reference, here's the original JS middleware implementation: https://github.com/reactjs/redux/blob/master/src/applyMiddleware.js#L26

Thoughts on the placement of the API and a better name for StateProvider?

beyondeye commented 8 years ago

perhaps name StateProvider StoreGetState? StateProvider should be a wrapper class around Store anyway. It is not really a separate concept

beyondeye commented 8 years ago

or define it as a nested class of Store interface, since it is actually an adapter for the store

interface StoreStateGetter<S> {
    S getState();
}
interface Store<S> extends StoreStateGetter<S> {
    S getState();
    /*
...
*/

}
pardom-zz commented 8 years ago

I would really prefer not to nest it inside Store because it has no use in a core implementation. It feels natural and it is general purpose so it's reasonable to assume that it could have use outside of Middleware, however, it has no reason to exist within the core implementation and is extraneous when using a vanilla Store.

As for naming:

There are two hard things in computer science: cache invalidation, naming things, and off-by-one errors.

😛

So here are some suggestions

We could also use the more generic Func0 ala RxJava

package redux.api.enhancer;

interface Middleware<S> {

    Object dispatch(Func0<S> getState, Object action, Dispatcher next);

}

Of course, then we need to decide where to put the Func0 interface. 😛

glung commented 8 years ago

I gave it a few thoughts.

My 2 cents:

A store

Because we already have the Dispatcher interface, we could also have either

Am 03.11.2016 14:54 schrieb "Michael Pardo" notifications@github.com:

I would really prefer not to nest it inside Store because it has no use in a core implementation. It feels natural and it is general purpose so it's reasonable to assume that it could have use outside of Middleware, however, it has no reason to exist within the core implementation and is extraneous when using a vanilla Store.

As for naming:

There are two hard things in computer science: cache invalidation, naming things, and off-by-one errors.

😛

So here are some suggestions

  • StateProvider
  • StateSupplier
  • StateContainer
  • StateGetter
  • StateDelegate
  • StateWrapper

We could also use the more generic Action0 ala RxJava http://reactivex.io/RxJava/javadoc/rx/functions/Action0.html

package redux.api.enhancer; interface Middleware {

Object dispatch(Action0<S> getState, Object action, Dispatcher next);

}

Of course, then we need to decide where to put the Action0 interface. 😛

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/jvm-redux/jvm-redux-api/issues/8#issuecomment-258148525, or mute the thread https://github.com/notifications/unsubscribe-auth/AAhkcdbQvo6FZ7UwodXg1h_kDLVbY05aks5q6eergaJpZM4Kn6Fi .

pardom-zz commented 8 years ago

Hmm, you guys make convincing arguments, and they stand pretty well on their own. I guess at this point, my only hesitation is how far we want to deviate from or stay true to the original JS implementation. The original middleware implementation defines the state function in the Middleware API.

beyondeye commented 8 years ago

Seriously I start to think that we are getting into overengineering land. I would keep things simple and not deviate from the original implementation if there is not some real reason to do that

pardom-zz commented 8 years ago

In that case, I think my original post has the least over-engineered solution and deviates the least from the original.