reflux / refluxjs

A simple library for uni-directional dataflow application architecture with React extensions inspired by Flux
BSD 3-Clause "New" or "Revised" License
5.36k stars 330 forks source link

Why actions? #294

Closed danielearwicker closed 9 years ago

danielearwicker commented 9 years ago

In original Flux it uses action dispatch instead of just having methods on stores that directly mutate the store.

This is so multiple stores can handle the same actions and so maintain consistent but different views of the same data.

In Reflux you (sensibly!) let stores aggregate data from other stores by just observing their change events.

But this should mostly mean that dependent stores don't need to handle actions. Ideally there are going to be two kinds of store:

  1. "Model" stores that respond to actions and mutate their own content, and
  2. "Derived" stores that just recompute their content every time they hear a change event.

(Type 2 stores resemble react components, which run the render function any time something changes.)

So in that case, instead of actions, the Type 1 stores could just have methods to mutate their contents. No need for actions-as-events.

ivan-kleshnin commented 9 years ago

With simplest cases this may kinda work but how do you propose to log all actions in that case? History, rollback – all that becomes impossible, right? Also two types of Stores brings additional complexity to reasoning and implementation. How do you propose to implement async data load? In stores? Of what type?

maxguzenski commented 9 years ago

You will end up with a complex spaghetti. Who listen who? you will never know. And as you said stores store "different views of the same data", what happen when a "derived" store needs original data and not a modified one?

ivan-kleshnin commented 9 years ago

I want to underline that passive stores are also valid approach: https://github.com/Yomguithereal/baobab You just need to be consistent. It's easier to combine drawbacks than benefits :)

danielearwicker commented 9 years ago

@ivan-kleshnin

With simplest cases this may kinda work but how do you propose to log all actions in that case? History, rollback – all that becomes impossible, right?

Not right!

Separated actions are really nothing more than a hook that lets you log all mutating method calls on your data models. If that's what you want, and you have ordinary stateful models (with methods that mutate them) then this behaviour can be very easily added "on the outside" to any such object. You just wrap it:

var myStore = listenTo(myRealStore, callback);

Then you export myStore. Each call to a function on myStore results in calls to your callback where you can log it, etc. The listenTo function is very easy to write in JS as a totally reusable utility.

Same trivial bit of code can be reused on every stateful object. No need for boilerplate. Then you get a callback every time the object's mutating methods are called.

Second point is that this is probably not enough anyway. For rollback in particular, logging each action just gives you a one-way stream of actions to replay later on a blank object. But in a responsive UI, say you are implementing Undo, that's a poor way to get the previous state or state-before-last (replay all states from the beginning).

Rollback requires a way to get the inverse of an action. i.e. action is SendEmail, what's the inverse? Not all actions have meaningful inverses. And even if action is Like then the inverse is Unlike, which is a different action. Only a subset of actions are their own inverse, e.g. ToggleLike.

The fanciest approaches use immutable data. The previous 50 states can be kept on an undo stack and you can switch back to any of them instantly. They share as much data internally as possible, to reduce duplication.

But doing everything immutable is tricky. Much easier is to take a leaf out of core React's book. At a checkpoint (after a state change event) run a function that gives you a representation of the current state (for model data this would be toJSON, for a React component it is the render function). Compare it with the previous state to find a minimal statement of what changed.

React components do this to make minimal changes to the DOM. A stateful data model would do the same thing (with JSON deltas) to store minimal history of changes. And such deltas have a standard inverse, so that problem goes away too.

There are lots of ways to do this without separate action-events.

Also two types of Stores brings additional complexity to reasoning and implementation.

It ain't my fault that there are two types of store! It's just the way it is.

From the horse's mouth here's UnreadThreadStore from the FluxChat example.

It doesn't even store any data. It's really just a getCount function that computes a count from whatever is currently stored in ThreadStore.

In its dispatcher responder callback, it has to know which action types will cause the other stores to change their state, so it can emit its own change event!

Reflux makes this much simpler: just have this "store" subscribe to the change event of the stores whose data it depends on. (The waitFor hack in original Flux is just a long-winded way to subscribe to the change events of other stores.)

Note also how this store doesn't mutate any data of its own in response to actions. If a store both aggregates other stores data and mutates it it when an action happens, thing would get very messy.

So there are naturally two types:

  1. Actual mutable stores,
  2. Aggregators of other stores (more like observable sources of changing data than actual stores)

From the client's (the code that listens for changes and reads the current state) perspective they are indistinguishable, but for the implementor they are fundamentally different.

Also (to further improve on this example) it would a good idea if an aggregator cached its latest value, so (in the example) getCount would not have to recompute it unnecessarily for every caller when nothing had changed.

How do you propose to implement async data load? In stores? Of what type?

If a store is a stateful model, consider giving it methods toJSON (which is non-mutating of course, like render) and fromJSON (which mutates and fires change event). Then you can load data into it by saying:

$.get('previouslySavedModelData', d => myStore.fromJson(d));

If a store is an aggregator, let it return a promise from its "compute-my-value" function. In the then handler, put the new value in the store's cache and fire the change event. From the outside, users of the store don't care whether its asynchronous or not.

danielearwicker commented 9 years ago

@maxguzenski

You will end up with a complex spaghetti. Who listen who? you will never know.

You know where spaghetti comes from? Globally addressable stuff. Like for example, a global dispatcher, global actions, global stores. Any part of the code may listen to anything else because it's all global.

The solution is to enforce layering. This can be done by never having any globals. Instead having stores that must be constructed. e.g. in ES6 if you like you can make them classes. In the constructor, you pass them the other store(s) they depend on, if any. They cannot obtain other stores by any other means, because stores are not globally addressable. This is just dependency injection.

This way, it is not possible for them to get in a mess. They have to be layered.

And as you said stores store "different views of the same data", what happen when a "derived" store needs original data and not a modified one?

I'm not sure what you're asking, but I'm very interested in your feedback if you are using Flux or Reflux, so I'll explain what I'm talking about more carefully.

From the Flux website intro:

We originally set out to deal correctly with derived data: for example, we wanted to show an unread count for message threads while another view showed a list of threads, with the unread ones highlighted.

Unread is a consistent filtered version of All. In the FluxChat example they have an Unread store that boils down to this function:

var threads = ThreadStore.getAll();
var unreadCount = 0;
for (var id in threads) {
  if (!threads[id].lastMessage.isRead) {
    unreadCount++;
  }
}
return unreadCount;

Or more succinctly:

var all = ThreadStore.getAll();
return Object.keys(all).filter(id => !all[id].lastMessage.isRead).length;

Or if ThreadStore.getAll() just returned an array:

return all.filter(t => t.lastMessage.isRead).length;

All you need is a mechanism that runs that code, caches the result and fires the change event, every time the ThreadStore fires its own change event. Now you have a derived store that is guaranteed to be consistent.

Suppose we want to have All, Unread, Spam, NotSpam. You use Spam to show the spam folder contents. Should Unread include Spam? Depends on your requirements, but you either make it filter from All or NotSpam.

So you can make stores depend on whatever store has the information they need. Is that what you were asking?

ivan-kleshnin commented 9 years ago

@danielearwicker

Not right!

I thoroughly read your long reply but didn't find any counter-argument about action logging :( Again. If we have no actions and mutate Store from Component where will be our your action logging. Just name that Xxx in a few words. Store logs mutations? Component logs actions? Additional element appears? Because when you talk about all possible alternatives at once... it's hard to follow.

maxguzenski commented 9 years ago

Yes, it is hard to follow, but what are you proposing keep been Flux?

For exemple, I have

AjaxStore - that listen any ajax request (when started, when ended) to show "loading spin"

ErrorStore - listen all actions.onfailed

UsersStore - listen all actions to see if data has {users:...} on it

How I use this in your proposal? how ErrorStore knows that a trigger event from a store is a kind of error or not (sometimes a store can deal with a error, but want to sent to errorstore anyway) ?

danielearwicker commented 9 years ago

@ivan-kleshnin

If we have no actions and mutate Store from Component where will be our your action logging.

There were two parts to my answer about that.

  1. If you really want pure logging at the API level, you can wrap it around the outside using an aspect. Was it clear what I meant about that? I can explain further, with an example. See this gist for a really simple function that wraps an object and will call the notify callback every time a method is called on the wrapped object.
  2. If you want rollback in a reactive UI, you likely do not want logging at the API level. You want an efficient way to return to previous states. Logging only gives you an inefficient way.

Then I explained other ways to get rollback.

Look at it this way: I am trying to understand what problem Flux is meant to solve. One way for me to do that is to take part of it away and then ask: what can I no longer do with it? What becomes easier/harder?

EDIT accidentally closed issue when being thrown off a train!

ivan-kleshnin commented 9 years ago

@danielearwicker thanks, now it's much more clear.

If you really want pure logging at the API level, you can wrap it around the outside using an aspect. Was it clear what I meant about that? I can explain further, with an example. See this gist for a really simple function that wraps an object and will call the notify callback every time a method is called on the wrapped object.

Yes, this is possible. I was confused by the name listenTo when there are no emitters just wrappers. Bad name imo, but I've got your point at least. Object wrapping has a lot of drawbacks from my point of view. React objects look different in console.log and this may break some strictly type-dependent stuff to begin with.

If you want rollback in a reactive UI, you likely do not want logging at the API level. You want an efficient way to return to previous states. Logging only gives you an inefficient way.

I agree with that. Making proper history is very hard in general case. Hovewer, I'm pretty sure that recording actions from Action "part" is easier than from proxy objects. Because that's can appear on library level and some flux implementations already provide this. With proxy you're locked in app space. I had practice with proxies in Python and that was... painful.

Look at it this way: I am trying to understand what problem Flux is meant to solve. One way for me to do that is to take part of it away and then ask: what can I no longer do with it? What becomes easier/harder?

This is intelligent approach.

I think first viable point of actions is data conversion. You submit feedback form with comment. Action receives single string "message" and converts it to proper model. Then stores can (and should) be clean and work with only models and ids. Second point – take async stuff out of stores. To make them more simple and make testing easier. Third point – make data flow more consistent. When there is a crazy mix of reactive and interactive paradigms – reasoning about your system is harder. When everything is reactive – that's simpler again. Sadly, React already has this mix at the core (setState(), etc). But that's another story.

danielearwicker commented 9 years ago

Thanks for feedback. The impression I have from looking at usage examples of Flux is that the coder invariably prefers writing pure functions to recreate derived data "from scratch" every time source data changes. This happens in the TodoMvc and Chat examples. The Chat example maintains two stores (one for all messages, one for threads) but then it recomputes a sorted version of the thread list using a pure function, every time the thread-list component asks for it. So this means every part of the example could be implemented with a single stateful store, plus pure functions that transform the data on demand as required. And if you have a single stateful store, you don't need action-dispatching to keep stores in sync, as there is only one such store.

And as I say above, the action dispatching approach does not conveniently deliver rollback.

So I think Facebook's own examples indicate that Flux is overcomplicated.

I wrote up more here - feedback welcome!