search

mattkrick / redux-optimistic-ui

a reducer enhancer to enable type-agnostic optimistic updates
MIT License
693 stars 36 forks source link

Provide store enhancer #8

Closed m-lautenbach closed 8 years ago

m-lautenbach commented 8 years ago

This store enhancer replaces manually wrapping state and reducer.

mattkrick commented 8 years ago

i really like this idea & i'm so glad you brought it up. Doing this would allow us to internalize ensureState, which means upgrading your app is as easy as including a store enhancer. Simply awesome.

Conversely, the reason why i haven't done this yet is because a storeEnhancer can't be added/removed throughout the life of the app, whereas a reducerEnhancer can be. In the real world, that mean a bigger landing page payload (much bigger if your landing page doesn't need immutablejs).

Personally, I'd sacrifice the easy-install for payload size & flexibility, but that's just me. Maybe the right option is to give devs the choice like your PR does? Honestly, now that I'm using cashay I don't really have a need for this package, but I understand the utility for situations where a GraphQL-based solution isn't an option.

@gaearon any preference on storeEnhancers vs reducerEnhancers vs both? I love forcing best practices, but maybe this is a good place to give folks the choice?

m-lautenbach commented 8 years ago

Well, we don't plan the dynamic storeEnhancer removal/adding und use ImmutableJS anyway. So having a storeEnhancer included doesn't seem to have drawbacks for us. Especially as writing a storeEnhancer is not the most common thing to do and required some research for me.

gaearon commented 8 years ago

I would rather see it as reducer enhancer so it's clearer what's going on.

Also store enhancers should never mutate the store. If you make it a store enhancer please return a copy.

m-lautenbach commented 8 years ago

Sorry, I don't know how to write a reducer enhancer. Is there a resource about that?

gaearon commented 8 years ago

I think reducer enhancer is what this project already is.

mattkrick commented 8 years ago

I would rather see it as reducer enhancer so it's clearer what's going on.

Following Dan's lead, I like this point, as it follows the standard reducer enhancer pattern. I'll close this for now, but thanks for the suggestion @m-lautenbach

m-lautenbach commented 8 years ago

I'd just like to point out, that for using this library, a reducer enhancer is not enough. The state has to be wrapped as well. That's why I combined these to preconditions into one store enhancer.