Reconcile reducer/reducers

[ryanlanciaux]

Griddle major version

1.11.0

Changes proposed in this pull request

Fix an apparent typo from #757

Why these changes are made

A couple of the stories were not working (controlled griddle)

Are there tests?

No but there are some fixed stories

[dahlbyk]

Damn. In my defense, we're not consistent between "reducer" (from core/plugins) and "reducers" (everywhere else).

Are there tests?

There actually are! Unfortunately...

[dahlbyk]

In my defense, we're not consistent between "reducer" (from core/plugins) and "reducers" (everywhere else).

I've added a commit to this branch to try and make this more consistent:

• initializer now returns reducer instead of reducers, which makes sense because it's actually a combined reducer function
• Within initializer, dataReducers (retained from previous code) has been renamed defaultReducer to align with defaultStyleConfig

I do believe it would be better if the core/plugin key were reducers instead of reducer, since it expects a reducer-per-key object rather than an actual reducer function. On the other hand, it would seem reasonable to keep it as reducer if we could handle receiving functions or objects (which we'd coalesce to functions). Thoughts?

[ryanlanciaux]

Awesome - I'll close this one and look at that. Thanks a ton

[dahlbyk]

In retrospect, I should have included both of these commits in #784. Or did you specifically not like the reducers 👉 reducer change?

[ryanlanciaux]

Sorry -- for some reason I thought it was included in #784 :D

I do believe it would be better if the core/plugin key were reducers instead of reducer, since it expects a reducer-per-key object rather than an actual reducer function.

I agree with this -- sorry about the inconsistency that was around from the beginning here.

I think the future intent (that isn't documented) is, like you said. It would be nice to be able to take a reducer function or an object as well as the one-off reducer methods. If you have any stronger opinions on this -- I'm totally happy with either direction 😄

[dahlbyk]

I think the future intent (that isn't documented) is, like you said. It would be nice to be able to take a reducer function or an object as well as the one-off reducer methods. If you have any stronger opinions on this -- I'm totally happy with either direction 😄

I poked around at this for a bit, and it's not obvious how a plain reducer function should fit in the existing reducer object composition model:

1. All BEFORE_REDUCE
2. All *_BEFORE for the current type
3. The "winning" (last-applied) reducer for the current type
4. All *_AFTER for the current type
5. All AFTER_REDUCE

If a plugin provides a plain function for reducer, it's not obvious to me at which point that reducer should be called:

1. In place of (3) for all types, unless another reducer object with type is applied later, e.g.

   plugins=[{ reducer: { A: a }, { reducer: r }, { reducer: { B: b } }]
   

   would call r for all actions, except calling b when type is "B".
2. In place of (3) for all types for which a reducer has not been defined at any point, e.g.

   plugins=[{ reducer: { A: a }, { reducer: r }, { reducer: { B: b } }]
   

   would call r for all actions, except calling a or b when type is "A" or "B", respectively.
3. Before or after (3) for all types, e.g.

   plugins=[{ reducer: { A: a }, { reducer: r }, { reducer: { B: b } }]
   

   would call r for all actions, before or after calling a or b when type is "A" or "B", respectively.

Or maybe there's another option? Thoughts?

[dahlbyk]

We can revisit #783 (comment) later, but for now I've merged this for better naming consistency.

[ryanlanciaux]

Thank you - sorry I dropped off the face of the earth on this one. I totally agree with your assessment above.