Imagining a more functional, classless Flummox API

gistfile1.js

import Flummox from 'flummox';
import { Map } from 'immutable';

// Instead of a constructor, just wrap flux creation inside a function
export default function createFlux() {
  const flux = new Flummox();
  
  // Actions are the same. Just pass an object instead of a class.
  const thingActions = flux.createActions('things', {
    incrementSomething(amount) {
      return amount;
    },
    
    async createThing(newThing) {
      // async/await isn't strictly necsesary here -- we could just return the
      // the promise, but I like the reminder that this is async
      return await WebAPIUtils.createThing(newThing);
    }
  });
  
  const thingActionIds = fooActions.getActionIds();
  
  // Note that everything above is already possible in the current version
  // of Flummox.
  //
  // Here's where things get interesting...
  const thingStore = flux.createStore('things', {
    getInitialState: () => ({
      things: new Map(),
      _pendingThings: new Map(),
      counter: 0
    }),
    
    // Instead of `this.register()` et al, use hooks (like React's lifecycle
    // hooks) to set up action handlers. `register()` should return a hash of
    // action ids mapped to handlers.
    register: () => ({
      // Return new store state, given the previous state and the value sent by
      // the action. Like with `setState()`, the new state is merged with the
      // old state. It's like a transactional state update, or a
      // reduce operation (except it's a merge, not a replace).
      // 
      // All params are passed as one hash, to support possible future interop
      // with RxJS. Yay for destructuring!
      [thingActionIds.incrementSomething]: ({ prevState, value: amount }) => ({
        counter: prevState.counter + amount
      }),
      
      // For async actions, use a sub-hash with success and failure handlers.
      // The naming convention is taken from RxJS observers. Notice that there's
      // no `onBegin`, which you might expect if you're used to
      // Flummox's `registerAsync()`. For that, use the `registerOnStart()` hook
      [thingActionIds.createThing]: {
        onNext: ({ prevState, value: newThing, payload: { dispatchId } }) => ({
          things: prevState.things.merge({
            [newThing.id]: newThing
          }),
          _pendingThings: prevState.delete(dispatchId)
        }),

        onError: ({ payload: { dispatchId } }) => ({
          _pendingThings: prevState.delete(dispatchId)
        })
      }      
    }),
    
    // Specify handlers that fire at the beginning async actions, in order to
    // perform optimistic updates
    registerOnStart: () => ({
      [thingActionIds.createThing]: ({ prevState, payload: { dispatchId, actionArgs: [ newThing ] }}) => ({
        _pendingThings: prevState._pendingThings.merge({
          [dispatchId]: newThing
        })
      })
    })
  });
  
  // We could support both the class-based API and this new functional API
  // without much fuss. I think this is important, since much of the appeal of
  // Flummox comes from the familiarity and predictability of its API. Many
  // people are not used to functional programming concepts, and will want to
  // stick with classes.
}

This API would enable Flummox stores to be completely stateless.

Looks great!

As we already discussed on Slack, some thoughts:

Instead of having a registerOnStart method, I'd propose that we somehow move those handlers into the {onNext, onError} contract, so we can get rid of that method and have all handlers next to each other. How to name the property would still have to be discussed (As we already figured out onStart/onBegin would be bad, since it is too close to RXJS' onComplete, but having these methods separately just does not feel right to me).

Apart from that I only have a few things regarding the register* functions, but since they aren't related to this gist, I'll keep those out of here for now.

@johanneslumpe I had the same thoughts with registerOnStart, rather see all related handlers together.

I agree with the above statements, why is the register registerOnStart different.
Looks nice otherwise!

I'm still unsure about this, but here's an attempt to synthesize my thoughts on why registerOnStart is its own hook:

We can think of each action as a stream of values over time. With the register() hook, we're subscribing to a stream of actions. We'll use the observable contract from RxJS (React 0.14 will use this same contract to implement its observe() hook):

• onNext() is called for every new value in the stream
• onError() is called whenever the stream produces an error

In RxJS, there's also onCompleted(), which is called exactly once, when the stream has finished sending values.

An onStart() property doesn't really fit conceptually into this model. You'd expect onStart() to be the opposite of onCompleted() and only be called once, when the stream first starts sending values. Whereas what we really want is for it to be called not at the beginning of the stream, but at the beginning of every asynchronous operation (promise) that eventually resolves to the next value in the stream. To me, that sequence of notifications represents a different stream entirely, even though they're both triggered by the same action creator.

Again, please feel free to disagree, but as I see it there are two different streams here:

1. The stream of notifications that signal the beginning of a promise.
2. The stream of values that are resolved from each promise.

(For async actions, you can think of stream 2 as being the result of flatMap-ing a stream of promises.)

Another way of thinking about it is that actions are functions with side effects, where the side effect is a dispatch. For a normal sync action, there is a single dispatch per action invocation. For an async action, there are two dispatches per action invocation, and therefore two side effects. The registerOnStart() hook corresponds to the first dispatch, and the register() hook corresponds to the second dispatch.

By the way, the reason I'm talking about this stuff in terms of streams is that I'd like Flummox to be compatible with functional reactive programming concepts, since 1) I've drunk the Kool Aid and I think FRP is awesome, and 2) React is moving in that direction with its upcoming sideways data loading API. In order for this to work, we need each stream of values to have its own hook.

Does any of this make any sense at all, or am I off my rocker? :D

Another option would be to combine the two streams and use onNext() for both:

onNext({ prevState, value: newThing, payload: { dispatchId, actionArgs: [ newThing ] } }) {
  if (async === 'begin') {
    return {
      _pendingThings: prevState._pendingThings.merge({
        [dispatchId]: newThing
      })
    }
  }

  return {
    things: prevState.things.merge({
      [newThing.id]: newThing
    }),
    _pendingThings: prevState.delete(dispatchId)
  };
}

I don't like this, though, as it seems unintuitive. It would mean you need an if-statement for all async actions, even if you weren't doing optimistic updates.

I agree with the combine syntax feeling unintuitive. I see your point with these almost being 2 separate streams but it still feels weird having related logic broken out into different sections.

If we zoom out at a higher level, what advantages does this syntax offer us? does following the observable contract allow for interop with other apis (RxJs) or is it more for a familiar interface?

I think part of the issue is with async actions we have one actionId representing two separate streams (promises, resolved promise results).

I wonder if theres a way we could identify we want the promise stream instead of the resolved values:

{
  register: () => ({
    [thingActionIds.createThing.begin]: ...,
    [begin(thingActionIds.createThing)]: ...,
    [thingActionIds.createThing]: { onNext, onError }
  })
}

I use begin for lack of a better name.

That's a clever idea. Except action ids (constants) are strings, and you can't attach properties to them. But I like the notion of using a special key for the begin stream.

we want the promise stream instead of the resolved values

That's a better description of what I was trying to say above. The begin / start stream is essentially the result of a mapping transformation on the stream of promises, whereas the onNext stream is the result of a flatMap-ing transformation on the stream of promises.

I do understand why you want to separate them out into two sections, but just as for tappleby it feels a bit off for me. His syntax would allow us to at least keep things a bit closer together. How about something like this:

{
  register: () => ({
    [thingActionIds.createThing]: {
      begin() {...},
      stream: {onNext, onError}
    }
  })
}

But now that I've written that out, it feels weird having to nest the stuff like that. I'll leave it here for discussion anyway.