Relax the constraint on actions

[gcanti]

First of all thanks to all the contributors, in particular to @gaearon and @acdlite for your work.

The problem

Currently actions must be plain objects due to this invariant in the dispatch function:

// createStore.js

invariant(
  isPlainObject(action),
  'Actions must be plain objects. Use custom middleware for async actions.'
);

This prevents to use constructors to build instances of actions.
Scanning the issues I found this comment by @gaearon in #355 (comment)

record/replay and associated features won't work with an action class. We don't want to encourage this so people don't lock themselves out of great features

But I can't figure out why.

Relaxing the constraint seems to bring some benefits:

• define actions as models (domain driven design)
• possibly simplify the reducer and get rid of constants
• type check the actions (for example defining actions with tcomb, TypeScript or Flow)

Example (from the counter app example)

Define actions as models

// CounterAction.js

class IncrementEvent {
  patch(state) {
    return state + 1;
  }
}

class DecrementEvent {
  patch(state) {
    return state - 1;
  }
}

export function increment() {
  return new IncrementEvent();
}

export function decrement() {
  return new DecrementEvent();
}

Simplify the reducer and get rid of constants

// counter.js

export default function counter(state = 0, action) {
  if (typeof action.patch === 'function') { 
    return action.patch(state); // get rid of switch and constants
  }
  return state;
}

The implementation above promotes a patch function (instead of a reduce function) as a possible primitive:

patch :: (state) -> state

Type check the action construction (here with the help of tcomb)

// CounterAction.js

import t from 'tcomb';

//
// use the constructors as (type checked) action creators 
// Instances are also frozen
//

export var IncrementEvent = t.struct({});
IncrementEvent.prototype.patch = function (state) {
  return state + 1;
};

export var DecrementEvent = t.struct({});
DecrementEvent.prototype.patch = function (state) {
  return state - 1;
};

[volkanunsal]

👍

This would be a great addition.

[gaearon]

This is very much opposite to how Redux works. How would you serialize them and then replay these actions from a deserialized object?

[gaearon]

This page should cover why we don't ever want to do this:

http://gaearon.github.io/redux/docs/recipes/ReducingBoilerplate.html

It also explains how to avoid switch statements.

state, action => state is literally all Redux is. If you want state => state and lose serialization and record-replay with hot reloading reducers, it's up to you, but then there is no need to use Redux at all.

[gaearon]

type check the actions (for example defining actions with tcomb, TypeScript or Flow)

There is work on getting actions to type check. See #290, #356. It doesn't mean they have to be something other than plain objects. Discriminated unions FTW!

You can also easily write middleware to do similar runtime action checking against a schema.

[gcanti]

Hi @gaearon ! thanks for your comments.

This is very much opposite to how Redux works

I'm not proposing a change to how Redux works, the reducer function (and its signature) is the "heart" of Redux, I'm absolutely fine with that. I'm just proposing to relax a constraint on actions in order to allow a different implementation (using dynamic dispatch on a polymorphic operation instead of a switch) but there will still be concrete and serializable actions. Take this action (from the todomvc example):

class AddTodoEvent {

  constructor(text) {
    this.text = text;
  }

  patch(state) {
    return [{
      id: (state.length === 0) ? 0 : state[0].id + 1,
      marked: false,
      text: this.text
    }, ...state]
  }

}

The additional benefit here is that the action payload and the effect on the state are close, in the same file, in the same definition.

The AddTodoEvent event can be easily recorded in an in memory store and replayed. Instead if you want to store that action as a string, you can solve the problem as you'd do with the serialization / deserialization of a JSON API payload into an object model, e.g. with something like a toJSON() / fromJSON() pair.

For example, in order to be "serialization-compliant" a user must provide:

• the list of actions
• a toJSON(action) -> json function
• a fromJSON(json) -> action function

It's a problem a user must solve in any case in order to use, for example, a Date instance in "plain old" actions:

export function increment() {
  return {
    type: INCREMENT_COUNTER,
    dateOfIncrement: new Date()
  };
}

otherwise you'll end up with a very constrained action system (basically just JSONs).

See #290, #356

Flow is interesting indeed! I spent several weeks on it, but at the moment is incompatible with all the features provided by babel (no const, let, decorators ecc...) and it's not so easy to add in a frontend workflow.
Honestly, given the benefits of babel and runtime type checking, I'm not yet ready to embrace Flow and I'd like to have several choices.

[gaearon]

Let's keep it open for discussion. I'll come back eventually and write a more thorough response. Busy with getting 1.0 out :-)

[gaearon]

For example, in order to be "serialization-compliant" a user must provide:

That's the point of Redux. You get awesome features (hot reloading, time travel, etc) without asking user to provide stuff like toJSON/fromJSON. It's a maintenance burden to implement these correctly. If it doesn't “just work” out of the box, most people won't use it.

[gaearon]

Honestly, given the benefits of babel and runtime type checking, I'm not yet ready to embrace Flow and I'd like to have several choices.

There's nothing inherently Flow-ish there. All I'm saying is that checking plain objects is just as expressive as attaching runtime check semantics to classes. What's the difference? It's like PropTypes.oneOf vs PropTypes.instanceOf.

[gaearon]

The additional benefit here is that the action payload and the effect on the state are close, in the same file, in the same definition.

I don't see this as a benefit at all. In large codebases, you'll want to update absolutely different parts of state that are implementing different features, in response to the same action. Flux seems “complicated” because it forces your actions to be descriptions, not implementations, but that's what it makes it powerful. Two people can work on different parts of app and react to the same action without ever having to see each other's model update logic. This is the decoupling that Redux provides (just like Flux).

“Update functions” that operate on the whole state are cool but it's a different architecture than Redux that (in my experience) scales worse for bigger projects. And, as I described above, it loses some of its benefits.

Why not split your reducer instead of making actions “smart”? In Redux, stuff like AddToEvent is used from inside the reducer. You can split addTo(state) => state and then use it from any reducer in response to any action type. You can write createAddToReducer(actionTypes) and voila, you have a reducer that you can mount anywhere in your reducer tree, and that knows how to add stuff in response to any actions.

Don't underestimate the power of functions!

[gcanti]

If it doesn't “just work” out of the box, most people won't use it.

I see, it seems sensible to me, but then there's still a problem with the invariant: this time it might be too loose.

Let's define the property of being serializable in a "just work" way:

Definition. An object x is just-serializable if the property

deepEqual( x, JSON.parse(JSON.stringify(x)) ) === true

holds.

Note. Being just-serializable is a special case of being "serialization-compliant" where toJSON === JSON.stringify and fromJSON === JSON.parse.

There are 2 cases:

1. Redux must enforce the just-serializable property on actions
2. Redux should be agnostic on the nature of actions

I'd prefer 2, but let's follow the 1 route. The invariant should then become something like:

invariant(
  isJustSerializable(action), // isPlainObject is too weak
  'Actions must be "just serializable"'
);

and actions created with:

export function increment() {
  return {
    type: INCREMENT_COUNTER,
    dateOfIncrement: new Date() // a Date instance is not just-serializable
  };
}

// or... 

// snippet from "Tutorial: Handcrafting an Isomorphic Redux Application (With Love)" 
// https://medium.com/@bananaoomarang/handcrafting-an-isomorphic-redux-application-with-love-40ada4468af4
export function createTodo(text) {
  return {
    type: 'CREATE_TODO',
    promise: request.post(BACKEND_URL, { text }) // a promise is not just-serializable
  };
}

will be invalid.

TL;DR the current invariant should be relaxed or tightened?

[goatslacker]

Checking if an action is serializable on each dispatch is probably not feasible due to performance concerns. Checking a plain object seems like a worth it trade off. It's cheap and covers most of the cases.

You can currently get what you want by attaching a symbol to a POJO and then that'll eliminate switch statements.

[gaearon]

Yes, I would prefer to see it tightened in development. We might do this in later versions or as part of optional developer tools.

[gaearon]

Actually this is wrong:

// or... 

// snippet from "Tutorial: Handcrafting an Isomorphic Redux Application (With Love)" 
// https://medium.com/@bananaoomarang/handcrafting-an-isomorphic-redux-application-with-love-40ada4468af4
export function createTodo(text) {
  return {
    type: 'CREATE_TODO',
    promise: request.post(BACKEND_URL, { text }) // a promise is not just-serializable
  };
}

It is meant to be processed by middleware. By the time Promise middleware processes the action, whatever it dispatches, will be serializable. See Async Actions guide.

For now, I'm closing because I can't see anything actionable here. If we can check actions before they're dispatched, I'd like to see that prototyped in the userland first anyway.

[calvinmetcalf]

coming back on this from a slightly different perspective, chrome/v8 tends to optimize classes much better then it optimizes pojos so something like

function Action (type, payload) {
  this.type = type;
  this.payload = payload;
}
export function increment() {
  return new Action(INCREMENT_COUNTER,  new Date());
}

is going to be optimizable by v8 much better then

export function increment() {
  return {
    type: INCREMENT_COUNTER,
    payload: new Date()
  };
}

[gaearon]

Dispatching thousands of actions per second (which is the only use case where this would matter) is not something you'd do in real Redux apps. They are meant to be perceived by a user, after all. And it's impossible to update the UI that fast anyway.

[calvinmetcalf]

it could also help the central dispatch function get optimized as currently I don't think any of the actions will share the same hidden type so it would be compiled.

Just a gocha I ran into when my node.js instincts of 'making the same object literal a bunch, probably want to turn it into a simple class' kicked in.

[iotch]

I'm sorry for bumping old issue, but will be really great to have the ability to check object types in middlewares to intercept the actions of specific types. For example, we could simple check if(action instaceof APIAction) instead of searching in each action type string or defining an action prop {isApi:true}, or something similar.

[gcanti]

will be really great to have the ability to check object types in middlewares

@iotch relevant discussion here gcanti/redux-tcomb#9 if you want to chime in