Skip to content
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

Middleware all the things #55

Closed
wants to merge 13 commits into from
Closed

Middleware all the things #55

wants to merge 13 commits into from

Conversation

acdlite
Copy link
Collaborator

@acdlite acdlite commented Jun 7, 2015

Composable middleware

Attempts to address #6.

Currently, the dispatcher does not publicly expose its dispatch() method. Instead, it provides a perform() method that serves the same purpose, except if the action is a function it calls the function and passes dispatch as a callback. This is a clever way of supporting async action creators. However, I believe this feature does not belong in the dispatcher, especially not as the sole interface for dispatching actions. Conceptually, this makes more sense implemented as middleware.

I propose that we remove the perform() method and export dispatch() in its place. dispatch() works the same way as before: it passes an action through the store to calculate the next atom, then emits a change.

To continue supporting async actions, and to provide an extensibility point for external plugins and tools, we can provide some common action middleware, a helper for composing middleware, and documentation for how extension authors can easily create their own.

An action middleware is simply a higher-order function. It returns a function with the same interface as dispatch(). For example, here's how you'd implement the perform() method described above:

// `next()` is either the next middleware in the chain or the `dispatch()` method
function callbackMiddleware(next) {
  return action =>
    typeof action === 'function'
      ? action(next)
      : next(action);
}

const perform = callbackMiddleware(dispatch);

Now we can use perform() just like before:

perform(action);

Of course, things really get interesting when you compose multiple middleware together. Here's another middleware that adds basic support for Flummox-like async action creators:

function promiseMiddleware(next) {
  return action =>
    action && typeof action.then === 'function'
      ? action.then(next)
      : next(action)
}

Combine this with the callback middleware we defined above:

const perform = callbackMiddleware(promiseMiddleware(dispatch));

This is very flexible. You can mix and match middleware, and even change middleware on a per-action basis. We should provide an interface to specify global middleware that gets applied to every action.

Like we do with composeStores(), we can also provide a composeMiddleware() helper as a convenience:

function composeMiddleware(...middlewares) {
  return next =>
    middlewares.reduceRight((_next, middleware) => middleware(_next), next);
}

Here's the above example, rewritten to use our "higher-order middleware" (hehe):

const middleware = composeMiddleware(promiseMiddleware, callbackMiddleware);
const perform = middleware(dispatch);

@gaearon has a good example of how this compose function would work with interceptors (which are just functions that return middleware): #6

Yay!

In summary:

  • Remove perform() method and export dispatch() instead.
  • Rewrite bindActions() so the final parameter is dispatch rather than the dispatcher. (Minor nitpick: Also rename it to bindActionCreators for consistency. I made this mistake with Flummox and came to regret it quickly).
  • Add callbackMiddlware() for async functionality previously provided by perform(). There's no atom parameter, because I don't think it makes sense: you can just pass the atom directly to the action creator. I rewrote the Counter action creator incrementIfOdd as an example. Also, I wasn't sure if this should go on the main export or not, so for now I've put it at redux/middleware/callback.
  • Add composeMiddleware() util for easily composing multiple action middlewares.
  • Rewrite examples, including incrementIfOdd and incrementAsync action creators. incrementAsync illustrates how to implement basic promise middleware.
  • Provide an API for global middleware.
  • Update README (if/when this is ready to be merged)?

Thoughts?

@acdlite
Copy link
Collaborator Author

acdlite commented Jun 7, 2015

I'll wait for further discussion before starting on a global middleware API. (Perhaps dispatcher-level is the more accurate descriptor.)

@gaearon

@acdlite
Copy link
Collaborator Author

acdlite commented Jun 7, 2015

I think this would address all the concerns in #6.

@dariocravero
Copy link
Contributor

Wow, this is neat :)! 👍

@acdlite
Copy link
Collaborator Author

acdlite commented Jun 7, 2015

Just in case it isn't clear, this is fundamentally the same concept as the interceptors described by @gaearon here #6. I'm using slightly different terminology so that we can distinguish between stuff like the callback middleware and side-effect-y middleware like the logger interceptor. Comparing to Dan's example:

function logger(label) {
  return sink => payload => {
    console.log(label, payload);
    sink(payload);
  };
}
  • sink === next
  • payload === action

@acdlite
Copy link
Collaborator Author

acdlite commented Jun 7, 2015

Perhaps the api for global middleware/interceptors could be as simple as this:

createDispatcher({
  store,
  dispatch: (dispatch, emitChange) => compose(
    callbackMiddleware,
    promiseMiddleware,
    logger('Dispatching action...'),
    dispatch, // the actual dispatch to the stores
    logger('Finished dispatching action.')
    logger('Emitting new atom...'),
    emitChange,
    logger('Done emitting.')
  )
})

@acdlite
Copy link
Collaborator Author

acdlite commented Jun 7, 2015

Now that I think about it we probably should settle on a single term, because they're the same thing. "Interceptor" sounds awesome but "middleware" is clearer, I think — anyone who has used Express, Rails, etc. already knows what it means.

@acdlite
Copy link
Collaborator Author

acdlite commented Jun 7, 2015

Oh boy, we could even treat the store as middleware! Then we don't even need a store param!

createDispatcher(emitChange => compose(
  callbackMiddleware,
  promiseMiddleware,
  logger('Dispatching action...'),
  createStore(store),
  logger('Finished dispatching action.')
  logger('Emitting new atom...'),
  emitChange,
  logger('Done emitting.')
))

// or maybe...

createDispatcher(emitChange => [
  callbackMiddleware,
  promiseMiddleware,
  logger('Dispatching action...'),
  createStore(store),
  logger('Finished dispatching action.')
  logger('Emitting new atom...'),
  emitChange,
  logger('Done emitting.')
])

Um, I kinda like this. Too crazy?

@acdlite
Copy link
Collaborator Author

acdlite commented Jun 7, 2015

Well damn, now "dispatcher" feels like the totally wrong word for this... It's the thing that holds the state and emits changes... so shouldn't we call it a store?? Then our store function just becomes... a mapping function?!?!

createStore(emitChange => compose(
  callbackMiddleware,
  promiseMiddleware,
  logger('Dispatching action...'),
  map(store), // wtf should we call this?
  logger('Finished dispatching action.')
  logger('Emitting new atom...'),
  emitChange,
  logger('Done emitting.')
))

Does anyone else like this? Have I taken up residence in Crazytown?

@acdlite acdlite changed the title Action middleware Middleware all the things Jun 7, 2015
@acdlite
Copy link
Collaborator Author

acdlite commented Jun 7, 2015

Actually, a map() won't do because we also need to pass in the atom. (Duh Andrew, it's Redux, not Mapux.) Updated:

createStore(({ emitChange, reduceState }) => compose(
  // ...middleware...
  reduceState(reducer), // "store" is now "reducer"
  // ...more middleware...
  emitChange
));

where reduceState() looks like this:

function reduceState(reducer) {
  return next => action => next(reducer(getAtom(), action))
}

getAtom() is a bound reference to store.getAtom()

@vslinko
Copy link
Contributor

vslinko commented Jun 7, 2015

In general it's looks great! It could be a 1.0.0 release.

@vslinko
Copy link
Contributor

vslinko commented Jun 7, 2015

Rewrite examples, including incrementIfOdd and incrementAsync action creators. incrementAsync illustrates how to implement basic promise middleware.

I'm not sure about that. Action should to be able to dispatch several actions. There is my example:

export function submit() {
  return async (perform, state) => {
    perform(setDisabled(true))
    perform(setError(null))

    try {
      await perform(authorize(
        state.signIn.data.username,
        state.signIn.data.password
      ))
    } catch (error) {
      perform(setError(error))
    } finally {
      perform(setDisabled(false))
    }
  }
}

@vslinko
Copy link
Contributor

vslinko commented Jun 7, 2015

I see you create callbackMiddleware to pass next into action.
It works when your action is pretty simple.
But it ins't solves my previous example because perform from master allows to call async actions by chain.
Here is my full example:

export function submit() {
  return async (perform, state) => {
    perform(setDisabled(true))
    perform(setError(null))

    try {
      await perform(authorize(
        state.signIn.data.username,
        state.signIn.data.password
      ))
    } catch (error) {
      perform(setError(error))
    } finally {
      perform(setDisabled(false))
    }
  }
}


export function authorize(username, password) {
  return async perform => {
    await new Promise(resolve => setTimeout(resolve, 1000))

    if (Math.random() > 0.5) {
      throw new Error('Invalid credentials')
    }

    const user = {
      type: 'users',
      id: '1',
      attributes: {
        username,
      },
    }

    const userLink = {
      type: user.type,
      id: user.id,
    }

    perform(mergeResources([user]))
    perform(setCurrentUser(userLink))

    return user
  }
}

All other action creators is sync and just returns an action.

@rpominov
Copy link

rpominov commented Jun 7, 2015

How do we implement "transactions" middleware with it? Such middleware need to be able to:

  • save current state on start of the transaction
  • log actions
  • replay several actions at arbitrary time (with a single update of the state)
  • replace current state directly (without dispatching actions)

@vslinko
Copy link
Contributor

vslinko commented Jun 7, 2015

I could create PR for master branch with example, if you want to fulfil this usage pattern.

@gaearon
Copy link
Contributor

gaearon commented Jun 7, 2015

Interesting! I wonder if this and the transaction thing are two different extension points, or the same one. Not sure yet.

Another idea I've been toying with is "shadow Flux" under the hood. Imagine

@gaearon
Copy link
Contributor

gaearon commented Jun 7, 2015

Oh. GitHub mobile is ridiculous with its comment button. :-(

@gaearon
Copy link
Contributor

gaearon commented Jun 7, 2015

Anyway. I'll tell you about this "shadow flux" later tomorrow :-)

@gaearon gaearon mentioned this pull request Jun 7, 2015
@gaearon
Copy link
Contributor

gaearon commented Jun 7, 2015

#6 (comment)

@acdlite
Copy link
Collaborator Author

acdlite commented Jun 8, 2015

Alright, I got hot reloading of transactions almost working... the UI updates properly, but the buttons stop working until you do a full page refresh. I'm not familiar with the mechanics of how hot reloading works. There are few other remaining issues, but it mostly works. I'll give a look again tomorrow.

Anyway, here's a preview of what I have so far:

@acdlite
Copy link
Collaborator Author

acdlite commented Jun 8, 2015

I will say that the more I mess around with this, the more I like @gaearon's concept of "lifted" or "meta" actions. My preference would be for a slightly more generalized approach — for instance, how would meta actions work with action middleware (the first part of this PR)? Ideally we could find a way to combine both concepts.

@rpominov
Copy link

rpominov commented Jun 8, 2015

how would meta actions work with action middleware (the first part of this PR)?

As shown here we can stack multiple middlewares on top of each other. And we can create a middleware that transform actions from a different format. But we can't support async formats like function or promise.

type Middleware = {
  map(state: MiddlewareState): Atom
  run(initialState: MiddlewareState, actions: Observable<Action>): Observable<MiddlewareState>
}

This should work. It support asynchrony i.e., a middleware can introduce actions or replace state at an arbitrary time, without "meta" actions, but we can still use "meta" actions. And I think we could imagine a way to stack multiple middlewares on top of each other. And we could implement support of async actions (functions and promises) as one of those stacked middlewares.

But if we to implement internal stuff as middlewares based on observables, we'll probably need to bring Rx as dependency.

Also observables can be stateful itself, for instance foo.take(5) has a counter of how many items it already took as a state. All states like that will be reset on a hot reload.

@gaearon Frankly I don't have a strong opinion on what to choose at this point. So many different ideas. But if I had to choose, I'd go with what you propose in #6, it has limitation of not supporting asynchrony naturally, but it will serve as API for transactions, and for many other purposes. So I'd go with it, and if we'll need more from the middleware API in the future we can change the API.

@acdlite
Copy link
Collaborator Author

acdlite commented Jun 8, 2015

Okay, so after reviewing both this PR and #6 again, here's when my thoughts currently stand.

I don't think @gaearon and I are proposing conflicting options. Upon close inspection, they're actually quite complementary, but it's a bit hard to tell due to the lack of clarity around terminology. Let me try to fix that.

Let's think of a Redux dispatch cycle as an asynchronous stream of actions, which is transformed into an asynchronous stream of states. At a high level, when we talk about "middleware," we mean anything that modifies that stream.

action    --[ middleware ]-->   state

So at the beginning of the stream, you have "action middleware," like the promise and callback middlewares I describe above. These middleware operate asynchronously. They take a stream of "raw actions" (where an "raw action" is the raw value that is sent via Dispatcher.dispatch(), which could, per the above examples, possibly include functions and promises) and turn them into "action payloads," which are proper action objects

At the end of the stream, you have "state middleware," which must be synchronous to fit with the Flux/Redux architecture. We can call this middleware the reducer. Expanding on the above diagram:

raw action    --[ action middleware ]-->    action payload    --[ reducer ]-->    state
                       (async)                                     (sync)

As @rpominov noted, it's really only possible to have a single state middleware that does anything powerful. Within that single reducer, however, you have a lot of freedom.

That's where Dan's concept of "lifted actions" and "Flux inside Flux" fit into this equation. Those strategies can be implemented as reducers. The observables API described by Dan? Also can be implemented as a reducer.

As for Observables/streams, I'm a fan, and they are an excellent at modeling the Flux flow. But I don't think they should be the primary API for constructing middleware in Redux (I believe @gaearon agrees with me here). Using observables inside middleware, though, is perfectly fine! You can even use it inside the reducer, so long as it's synchronous.

As Dan said:

I'd rather focus on a single extension point that allows free experimentation outside the core.

What we want is a high-level, minimal API for creating generalized middleware, preferably one that supports "intercepting" both actions and store atoms, since they are fundamentally all part of the same stream. I believe the approach laid out in this PR may qualify, as shown by my transactions example.

Middleware "specification" (lol)

Middleware are higher-order functions. The reducer is one level higher, a function that returns a middleware. I don't know the best way to document these using formal notation.

Could someone help me out? Here's my attempt to describe it in English, which you can cross reference with the code above.

  • Middleware: A higher-order function with the same signature as dispatch(). It accepts a single parameter, action. It returns a function whose input is next, where next is either the next middleware in the chain, or dispatch() if it is at the end of the chain. Middleware is both composable and asynchronous.
  • Reducer: A higher-order function which returns a synchronous middleware. Its parameters are store, state (the current state atom at the time the reducer is invoked), and dispatch (so that reducers can trigger dispatches). The dispatcher is responsible for sending these values at the time the reducer is invoked. I believe this is the minimal interface most reducers will need. Because a reducer returns a middleware function, it can be composed in exactly the same way as other kinds of middleware, where next is the dispatch method.

Blah, I hope this all makes sense.

@acdlite
Copy link
Collaborator Author

acdlite commented Jun 8, 2015

Just cleared up with @gaearon that in his above comment he was using the word "middleware" to refer to a "dispatcher strategy." Clearing that up helped us get on the same page.

Going forward, I propose that we use "middleware" to refer to composable middleware, like the ones in this PR, and "dispatcher strategy" to describe the higher-level object which accepts new actions and keeps track of state (and which, as an implementation detail, may make use of composable middleware).

@emmenko
Copy link
Contributor

emmenko commented Jun 8, 2015

Thanks for clarifying! 👍
I'm still trying to get the all picture though...

Also, quick question: how does this affect the HOC (Provider, Connector) ?
From your changes it looks like they behave like before. Is that correct?

@gaearon
Copy link
Contributor

gaearon commented Jun 9, 2015

I'm drafting a new PR for custom dispatchers. That's what my “dispatch strategy” really is. Then things like this PR will be implementable outside the core.

@gaearon gaearon mentioned this pull request Jun 9, 2015
@gaearon
Copy link
Contributor

gaearon commented Jun 9, 2015

I propose to close this PR and instead merge #60 which allows this PR to be implemented in userland.

@gaearon
Copy link
Contributor

gaearon commented Jun 9, 2015

(#60 wouldn't happen without the discussion here. Huge props to @acdlite!)

@gaearon
Copy link
Contributor

gaearon commented Jun 9, 2015

Redux 0.9.0 is out with the following default dispatcher function:

export default function createDispatcher(store) {
  return function dispatcher(initialState, setState) {
    let state = store(initialState, {});
    setState(state);

    function dispatchSync(action) {
      state = store(state, action);
      setState(state);
      return action;
    }

    function dispatch(action) {
      return typeof action === 'function' ?
        action(dispatch, state) :
        dispatchSync(action);
    }

    return dispatch;
  };
}

It should be possible now to implement this PR as a custom dispatching function.

@gaearon
Copy link
Contributor

gaearon commented Jun 9, 2015

I think the next step could be to have createDispatcher accept optional scheduler and reducer arguments:

function createDispatcher(store, schedule = createDefaultScheduler(), reduce = createDefaultReducer(store)) {
  return function dispatcher(initialState, setState) {
    let state = reduce(initialState, {});
    setState(state);

    function dispatchRaw(action) {
      state = reduce(state, action);
      setState(state);
      return action;
    }

    function dispatch(action) {
      return schedule(dispatchRaw, () => state, action);
    }

    return dispatch;
  };
}

A default scheduler might look like

function createDefaultScheduler() {
  return function schedule(dispatch, getState, action) {
    return typeof action === 'function' ?
      action(schedule, getState()) :
      dispatch(action);
  }
}

A default reducer might look like:

function createDefaultReducer(store) {
  return function reduce(state, action) {
    return store(state, action);
  };
}

Then we can expose an utility for chaining schedulers, like in this PR.
Thoughts?

@acdlite

@gaearon
Copy link
Contributor

gaearon commented Jun 9, 2015

An alternative dispatcher that replays all actions on top of the initialState:

export default function createReplayingDispatcher(store) {
  return function dispatcher(initialState = {}, setState) {
    let actions = initialState.actions || [];
    let state = { initialState, ...initialState, actions };

    function dispatchSync(action) {
      actions.push(action);
      state = { ...state, ...actions.reduce(store, state.initialState) };
      setState(state);
      return action;
    }

    function dispatch(action) {
      return typeof action === 'function' ?
        action(dispatch, state) :
        dispatchSync(action);
    }

    dispatch({});
    return dispatch;
  };
}

You can swap the default dispatcher with this one on hot reload, and it will “just work”.
Would be great if this was expressible with scheduler + reducer thing I wrote above..

@gaearon
Copy link
Contributor

gaearon commented Jun 9, 2015

So I'm playing around with an “uber dispatcher” (no composition for now, just boolean options). What I realized is ES6 Symbols are awesome! The let us hide the middleware state inside the state atom!

function createDefaultReducer(store) {
  return store;
}

const REPLAY_STATE = Symbol('Replay State');

function createReplayingReducer(store) {
  return (state = {}, action) => {
    let {
      [REPLAY_STATE]: { actions = [], initialState = state } = {},
      ...appState
    } = state;

    actions = [...actions, action];
    appState = actions.reduce(store, initialState);

    return {
      [REPLAY_STATE]: { actions, initialState },
      ...appState
    }
  };
}

function createDefaultScheduler(dispatch, getState) {
  function schedule(action) {
    if (typeof action === 'function') {
      return action(schedule, getState());
    } else {
      return dispatch(action);
    }
  }

  return schedule;
}

export default function createDispatcher(store, {
  log = false,
  replay = false
}: options = {}) {
  return function dispatcher(initialState, setState) {
    const reduce = replay ?
      createReplayingReducer(store) :
      createDefaultReducer(store);

    let state = reduce(initialState, {});
    setState(state);

    function dispatch(action) {
      if (log) {
        console.groupCollapsed(action);
        console.log('State before:', state);
      }

      state = reduce(state, action);
      setState(state);

      if (log) {
        console.log('State after:', state);
        console.groupEnd(action);
      }

      return action;
    }

    return createDefaultScheduler(dispatch, () => state);
  };
}

@acdlite
Copy link
Collaborator Author

acdlite commented Jun 9, 2015

The point about using Symbols to hide "meta" state on the state atom is smart. Small caveat, though. This

let {
  [REPLAY_STATE]: { actions = [], initialState = state } = {},
  ...appState
} = state;

assumes that the state atom is a plain object, rather than something like ImmutableJS. Which isn't a huge deal, since in the 99% case that will be true (because most people will use composeStores()) but we'll need to document assumptions like that.

@gaearon
Copy link
Contributor

gaearon commented Jun 9, 2015

Hmm, good point. We should probably just put the app's state in a separate field instead, and expose just it to the app in redux.getState(). This way the underlying “root” object state is never exposed to the user, we can still use Symbols to avoid clashes between middleware, and the user can even use a primitive as the atom.

@gaearon
Copy link
Contributor

gaearon commented Jun 9, 2015

@acdlite I'd be happy to have you playing with #62

@acdlite
Copy link
Collaborator Author

acdlite commented Jun 9, 2015

Closing this per #60. Eventually once the API is more settled I'll probably open a similar PR to add an easy integration point for global action middleware and get rid of perform(). Because action middleware like callbackMiddleware() and promiseMiddleware() merely wrap around a dispatch() method, they have the special property of being compatible with any implementation of the dispatcher, because every dispatcher will have this fundamental API. In other words, wherever you can do this

dispatch(action);

you can also do this

callbackMiddleware(dispatch)(action)

For that same reason, it's also very easy to implement this in userland for the time being.

@acdlite acdlite closed this Jun 9, 2015
@gaearon
Copy link
Contributor

gaearon commented Jun 9, 2015

👍 Thanks a lot for your work here. I feel that us approaching this from two different sides (close to the action vs close to the setState) really helped figure out the common base API.

@acdlite
Copy link
Collaborator Author

acdlite commented Jun 9, 2015

👍 Agreed!

@emmenko
Copy link
Contributor

emmenko commented Jun 9, 2015

Awesome work guys!!! Keep going 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants