Additional arguments for handleAction{,s} reducers

[myme]

A feature request here.

I have a reducer which have cross-cutting concerns, basically an action logger which puts log lines in the store based on different kinds of actions. This logger needs to know about a broader part of the state than the list of log statements its reducing. By sequencing the reducers in the root reducer, the log reducer can get for example the previous and next version of the state and log accordingly.

Having a root reducer similar to the following (using Immutable containers):

export default (state = new Immutable.Map(), action) => (
  state
    .update('someState', s => someState(s, action))
    .update(next => next.update('log', l => log(l, action, state, next)))
);

I'd optimally want to do the following:

const logInitialState = new Immutable.List();
const log = handleActions({
  // Direct logging action
  [logAppend]: (state, action) => state.push(Immutable.fromJS(action.payload)),

  // React to some other non-logging related action, notice the extra args
  [someAction]: (state, action, prev, next) => {
    const value = next.get('someState');
    if (value === prev.get('someState')) return state;
    return state.push(Immutable.fromJS({
      type: 'someAction',
      message: 'someAction happened',
    }));
  },
}, logInitialState);

But since handleActions does not relay the extra arguments, I'm forced to generate the reducers each time and close around the extra arguments like so:

const logInitialState = new Immutable.List();
const log = (state, action, prev, next) => handleActions({
  // Direct logging action
  [logAppend]: () => state.push(Immutable.fromJS(action.payload)),

  // React to some other non-logging related action
  [someAction]: () => {
    const value = next.get('someState');
    if (value === prev.get('someState')) return state;
    return state.push(Immutable.fromJS({
      type: 'someAction',
      message: 'someAction happened',
    }));
  },
}, logInitialState)(state, action);

This is both extra verbose and inefficient. It would be great if both handleAction and handleActions returns reducers that fetch a rest spread and passes it on. I guess reduce-reducers needs to know about this as well.

Something like the following would probably do:

export default function handleActions(handlers, defaultState) {
  // snip
  return (state = defaultState, action, ...args) => reducer(state, action, ...args);
}

[yangmillstheory]

I'm not sure if we should be passing any arguments other state and action to a reducer. I'd like to not deviate from the documented signature as much as possible.

Couple of questions.

1. Can you provide an example which doesn't use immutable (not everyone, including me, is familiar with this library, and using it in your example makes it hard to understand the essence of the problem)?
2. Can you include next and prev in the payload?
3. Can you move the cross-cutting reducer higher up in the reducer tree (so it can reduce more actions, and hand off to subreducers)?

[myme]

I actually followed the advice given by @gaearon in reduxjs/redux#749 (comment). One thing about the arguments splat is that it won't have an effect unless you're actually invoking the reduced reducer from handleActions with extra arguments.

Sorry about the example using Immutable, I get that it may clutter up the use case illustration a bit if you're not familiar with its api. Here's the example from @gaearon's comment reposted:

function a(state, action) { }
function b(state, action, a) { } // depends on a's state

function something(state = {}, action) {
  let a = a(state.a, action);
  let b = b(state.b, action, a); // note: b depends on a for computation
  return { a, b };
}

This is sort of a minimal example not tied to a specific use case. b in this case depends on the a reducer having computed the updated state, so it receives the next state through its third argument. In my case I need to compare against the previous state, so I pass in two extra arguments.

The log reducer is already "higher" up in the reducer chain, although it only receives it's own namespace so it can't fiddle with the global state (using Immutable you can actually enforce this, not just encourage it). It could get passed the entire state, but that doesn't solve the need for knowing both the before and after state once the other reducers have run.

A perhaps better (although contrived) example:

// Standard self-contained reducers
const adders = combineReducers({
  foo: handleAction(fooAdd, (state = 0, action) => state + action.payload),
  bar: handleAction(barAdd, (state = 0, action) => state + action.payload),
});

// "Monitoring" reducer, reacting to other updates to the state
const log = handleActions({
  [clearLog](state, action) {
    return [];
  },
  [fooAdd](state, action, prev, next) {
    const updates = [];

    if (next.foo === prev.foo) {
      updates.push('foo is unchanged');
    } else if (next.foo > next.bar) {
      updates.push('foo is greater than bar');
    }

    return state.concat(updates);
  },
  [barAdd](state, action, prev, next) {
    const updates = [];

    if (next.bar === prev.bar) {
      updates.push('bar is unchanged');
    } else if (next.bar > next.foo) {
      updates.push('bar is greater than foo');
    }

    return state.concat(updates);
  }
}, []);

function root(state = {}, action) {
  // foo and bar do their thing
  const next = adders(state, action);
  // log depend on state updated by firstStage reducers
  return log(next.log, action, state, next);
}

The logging reducer is just one application of this kind of use case. Another example could be something like a "Game over" monitor, which checks that all enemies are dead, the player's health is depleted or time has run out. It could be very clean to model this in the way that regular reducers run their course, then after the tick, the monitor reducer runs and determines if we're at end of game or not.

Middlewares are also a powerful way to implement cross-cutting concerns. They can easily inspect the state before and after core reducers have run. However, I sincerely feel like these kind of operations are in fact reducers. They're deterministic and pure thus fulfilling reducer requirements in every other respect.

[yangmillstheory]

Thanks for the clarification; it's great to learn about this use case.

function root(state = {}, action) {
  // foo and bar do their thing
  const next = adders(state, action);
  // log depend on state updated by firstStage reducers
  return log(next.log, action, state, next);
}

Where does next.log come from?

I've seen the thread reduxjs/redux#749 before and am interested in reading through it in more detail. What do you think about submitting a PR for this?

Also, what would the middleware implementation look like?

[myme]

next.log is the state namespace for the log reducer. Under init, it's undefined and so the handleActions for log would return the empty list default argument []. It then concatenates new log statements (or clears the log) based on the actions as actions trickle in.

I could have a look at a PR, though not right away. As I noted in the first comment, there needs to be an update to reduce-reducers as well in order to propagate the extra arguments down to the individual reducers.

I don't have time to sketch out a middleware example in code right now. But conceptually you'd have to inspect all dispatched actions. If the action is one that might trigger new log output (fooAdd and barAdd in the example), snapshot the state before calling the next middleware (or reducers) and capture the state again afterwards. If the state has changed, the middleware would have to (1) monkey patch the state by adding log statements; or (2) dispatch a new logAppend action which simply invokes a plain reducer which appends the log statement. There might be other solutions too.

However, both of those options are suboptimal though. Option 1 would result in state changes outside of the reducer chain, which becomes very opaque and magic to the user. Whereas option 2 would, amongst other things, break the replay feature to the redux dev tools; because when you replay fooAdd or barAdd a new logAppend would fire and you end up with two equal statements in the log.

[timche]

I have a few concerns with allowing additional arguments:

We are moving away from the original concept of a reducer ´(state, action) => state` which is super simple and beginner-friendly. Additional arguments can be confusing and rely heavily on the root reducer/other dependencies.

---

The game over example is imo a perfect example of using a (memoized) selector. This is nothing that has to be a reducer. The container component, e.g. GameOver is subscribing to foo and bar from the state, checks whether a condition is trueor false and reacts/renders accordingly.

reducers/foo.js:

export default handleAction(fooAdd, (state = 0, action) => state + action.payload)

reducers/bar.js:

export default handleAction(barAdd, (state = 0, action) => state + action.payload)

containers/GameOver.js:

// ...

function GameOver(props) {
  if (props.gameOver) {
    return // Presentational Component
  }
  return null
}

function mapStateToProps(state, ownProps) {
  const { foo, bar } = state
  return {
      gameOver: foo < bar
    }
}

export default connect(mapStateToProps)(GameOver)

---

We should stick to the principle of unidirectional data flow. The log reducer can be computed by selecting from the state too, it's nothing that has to be an additional reducer imo. The foo and bar reducer just need to be written a bit different, so we stick to the comment from @gaearon: If the data can be computed from a “lesser” state, don't hold it in the state. I also assume that the log reducer will be only used by a single component. Maybe I don't get it and you need to explain me why it's not possible other than having a reducer that sits between foo and bar:

reducers/adders.js:

export default combineReducers({
  // Use an array instead for holding previous and current value.
  foo: handleAction(fooAdd, (state = [0, 0], action) => state.shift().concat(state[1] + action.payload)),
  bar: handleAction(fooAdd, (state = [0, 0], action) => state.shift().concat(state[1] + action.payload))
})

containers/Log.js:

// ...

class Log extends React.Component {
  constructor(props) {
    super(props)
    this.state = {
      log: []
    }
  }

  onComponentWillReceiveProps(nextProps) {
    this.setState({
      log: [...this.state.log, nextProps.result]
    })
  }

  render() {
    // Render `this.state.log`
  }
}

// Selector
function getResult(state) {
  const { foo, bar } = state.adders;
  const result = [];

  if (foo[0] === foo[1]) {
    result.push('foo is unchanged')
  }
  if (bar[0] === bar[1]) {
    result.push('bar is unchanged')
  }

  if (foo[1] > bar[1]) {
    result.push('foo is greater than bar')
  } else if (bar[1] > foo[1]) {
    result.push('bar is greater than foo')
  }

  return result;
}

function mapStateToProps(state, ownProps) {
  return {
      result: getResult(state)
    }
}

export default connect(mapStateToProps)(Log)

[xiaohanzhang]

I am not sure if this will cause any confusing. Additional arguments is optional, beginners doesn't need to know their existence. Personally I found several case will require additional arguments.

1. In large project, a reducer need added a new dependency from outer state.
   Assume we need allow user change and save colour for their input. For example:

const rootState = {
  ui: {inputColor: 'red'}, 
  entities: {
    messages: {}, emails: {}, chats: {},
  }
};
const getAdditionArgs = (state) => {
  return {inputColor: state.ui.inputColor};
}
const rootReducer = (state, action) => {
   return {
     ui: uiReducer(state, action),
     // add extra argument here won't break anything in large project
    //  and only need change several reducers which has actual business logic.
     entities: entitiesReducer(state, action, getAdditionArgs(state)),
   }
};

Another option is add getAdditionArgs in action payload.
But this might require changes in lots components and lots actions. And same action might also been used in many other reducers, which may not like the additional field in payload.

2. 3rd party lib, for example: redux-orm
   Currently handleActions won't work with them, and I think the way redux-orm use reducer is another good use case for additional argument.

class Book extends Model {
    static reducer(state, action, Book) {
    }
}

[myme]

Now that you mention it @timche, I totally agree that the "game over" concept is best modelled with a selector, as you say.

For the log concept, it's possible to do as you suggest, basically keeping track of all actions going through the reducer. What I don't like about that though is the fact that the logic behind the reducer is reduced (no pun intended) to basically a simple unshift/push operation. Which forces consumers to either have to fetch the first/last element themselves or use a selector. Tools like react-devtools won't easily show you the current state of your log and current state of each sub-component of which the log is derived.

I do see the argument for beginner-friendly-ness, yet it wouldn't directly be conflicting with the canonical reducer signature. Additional arguments is purely optional after all. Also, it doesn't have to be verbosely advocated in the documentation, but would definitely deserve a place in an "Advanced use" section. This feature is not something I'm going to keep on insisting you add though. I do sincerely respect your reservations against it.

[timche]

@myme @xiaohanzhang @yangmillstheory I'm sorry for the slow progress here.

If we see that this feature is "largely" requested, I'm convinced to add it. As a library maintainer I think it's important to not add every single feature request. The goal of this library should be to provide a stable and simple API and user-friendly developer experience.

[myme]

I agree @timche. I would say it's a convenient feature (purely subjectively), and wouldn't necessary compromise compatibility. However, that doesn't mean that it's correct to add it simply because you can. Furthermore, I don't think there's any point in leaving this open to clutter up the bug list if there's no incentive to begin working on it.

[lsunsi]

Just wanted to contribute for this issue.

I think this change is good. I realize it violates reducer signature if the users use the additional arguments, but from what I can gather, this is a great way of passing global state as a read only to a slice reducer that can write to it's own slice.

The pattern seems fine and the change seems harmless, since it would change literally anything, it'll just be less opinionated on how you use yours reducers which are, as we know, just functions.

+1 for the change

[MaxMotovilov]

+1 for the change; I am now contemplating forking redux-actions -- which I've been using for years -- just to add that one capability. Applying selectors to the entire input state of the composite reducer in deep slice reducers looks like the cleanest solution for the cross-cutting concern problem.

[tboulis]

+1 for the change