search

dphilipson / typescript-fsa-reducers

Fluent syntax for defining typesafe reducers on top of typescript-fsa.
MIT License
220 stars 16 forks source link

#27: add possibility to pass default handler to reducers #28

Closed monsterkrampe closed 5 years ago

monsterkrampe commented 5 years ago

This resolves #27 (maybe also a good solution for #23). If you have better suggestions than adding a default handler, let me know, I am happy to receive feedback on this.

dphilipson commented 5 years ago

Thank you very much for this contribution! The use case is compelling and I think this is a great addition.

From an API perspective, I think I might prefer to have the default case be added to the chain with a method call, something like

const reducer = reducerWithInitialState(INITIAL_STATE)
    .case(setThing, setThingHandler)
    .case(setOtherThing, setOtherThingHandler)
    .default(defaultHandler);

because I think it's easier to understand visually if it matches the structure of a typical switch statement or if-else chain. If so, .default() should probably end the chain the same way .build() does, since additional calls to .default() don't make sense while additional calls to .case() just look weird.

Let me know what you think! I'm also happy to pick up the changes if you don't have time to get to this.

monsterkrampe commented 5 years ago

I totally agree, that adding it to the chain looks much better.

I first decided to pass the function as a parameter because I also thought, that calling .default() multiple times does not really make sense. Ending the chain with it like .build() seems like the best choice to me.

I think that I can get to this in the next days.

monsterkrampe commented 5 years ago

The default case can now be added to the chain using .default(), which ends the chain just like .build() does.

Let me know if you like it.

dphilipson commented 5 years ago

Sorry for the delay in reviewing. This looks good to me- thanks again for all the effort! I'll merge it and cut a release.