create applyOptimistic store enhancer

[jedwards1211]

Having to wrap getState calls in ensureState everywhere irks me for several reasons:

• You may have to make changes to numerous places to add it to an existing app
• It gets kind of awkward if there are other reducer enhancers doing the same thing to unwrap all of that state

So I created an optional applyOptimistic store enhancer that applies the optimistic reducer enhancer and calls ensureState internally, meaning the store could be used without doing any ensureState in mapStateToProps in userland.

I'm not 100% satisfied with this though given that middleware still has to use ensureState (Redux docs explain that applyMiddleware should come before other store enhancers because they can dispatch actions asynchronously that need to be seen by the other enhancers.

What do you think? Do you think it would be better to use middleware than a reducer enhancer?

[coveralls]

Coverage increased (+8.2%) to 92.857% when pulling bf37323cccc64f6ba49936f0573ed2eff54b8636 on jedwards1211:store-enhancer into 5d24fb10fc29fafaea56ee500b2d76ffec3ec7bc on mattkrick:master.

[jedwards1211]

In any case I'm not convinced that higher order reducers like this and redux-undo are the cleanest possible pattern for accomplishing these goals.

For instance, the optimistic history could be tacked onto the state via a Symbol prop. It would be a bit hacky, but would not have the unwrapping problem...

[mattkrick]

Duplicate of #8 I do like this idea, but I also get what Dan was saying when he talked about making it completely clear what was going on.

From personal experience, I recently switched from redux-persist (store enhancer) to redux-storage (reducer enhancer) because it gave me the flexibility I needed for async routes (after hours of debugging a really hard to find error). That taught me a painful lesson about not using an API lower than what you need. For this package, I could see how someone would use the store enhancer here & then hope to detect if there is an outstanding fetch, only to realize that they'd have to revert back to using the reducer enhancer.

[jedwards1211]

Hmmm. It frustrates me because there are only so many reducer enhancers one could use before it gets impossibly unwieldy. What about the idea of tacking the enhanced state on with a Symbol prop instead of wrapping the user's state?

[jedwards1211]

I say symbol because one could tack on the optimistic history via POJO or Immutable prop, but that doesn't necessarily work if someone's using some other type of state...

[mattkrick]

To date, I think this & redux-undo are the only 2 reducerEnhancers that wrap your state in another state. There are plenty of reducerEnhancers, but not many give you a wrapped state (redux-storage comes to mind). If you write your own proprietary one, just write your own custom ensureState. It's just naive duck typing, so it's pretty dern easy, nothing special there.

What you're suggesting is (what I call) a reducerExtender, which is what redux-optimist does, and was the impetus for me to write this package. By meddling in application-level reducers, as you alluded to, the package would have to know what kind of object it is in (POJO, immutable, ???) and need a handler for each. That gets gross. Enhancer vs. Extender can be compared to higher order function vs. mixin; HOFs won.

Taking a step back from the immediate problem, if you're handling a lot of async data from the same source, using redux-optimistic-ui is as much fun as paying for your groceries with pennies. You'll have to write an almost identical duck for each data type along with a schema type (you have to know that a specific parent type deletes all of it's children, etc). Instead, I'd suggest moving to a client cache like cashay that handles everything all of this for you. It even works for 3rd party GraphQL backends that aren't relay-enabled. If you often talk to non-graphql backends, write a wrapper for it, like what graphql hub has done. redux-optimistic-ui still has a place for one-off server calls, but it should be the exception, not the norm.

[jedwards1211]

Here's an idea!

Why not just pass getOptimisticState and setOptimisticState into the options for the reducer enhancer, and have it use them? For instance, for immutables:

function getOptimisticState(myState) { return myState.get('redux-optimistic-ui') }
function setOptimisticState(myState, optimisticState) { return myState.set('redux-optimistic-ui', optimisticState) }

This way, no matter what type of state you're using, you can make it work with redux-optimistic-ui, without having to do any unwrapping

[mattkrick]

You could, although you'd need a driver that is larger than 2 functions. You'd need a concat function, a destructuring function, and a few others I think. That'd be a good PR for redux-optimist, but that's still extending the state, not enhancing it. I like code that's expressive & easy to reason through at the cost of brevity. It's that same preference that makes for large redux boilerplates & big, declarative GraphQL queries. If you're coming from the meteor world, you know the heartbreak that "short & magical" can cause.

[jedwards1211]

I am coming from the meteor world and I know the heartbreak that its version of "short and magical" causes. But here is why: meteor maintains control of nearly everything, including the server, package system, compliation process, etc. The kind of "short and magical" I'm advocating here is completely different: instead of maintaining control (wrapping the user's state), it cedes control (by allowing them to define where redux-optimistic-ui's state gets put). What I'm advocating is exactly like redux-form's getFormState function that meatier uses to make redux-optimistic-ui compatible with it; without getFormState, how would you get these two to play nicely together? (With what I'm proposing, meatier wouldn't need to use getFormState at all, and redux-optimistic-ui could be dropped into an existing library without wide-ranging code changes).

My motivation for bringing up this issue was not just brevity, but also an intuitive belief that having to change your view containers to work with this library is an unnecessary form of spaghetti code.

The state wrapping can still remain the default behavior in the form of a default driver, and using a different driver can merely be an opt-in feature for added convenience.

The current design may seem easy to reason about now. But imagine what you'd have to do in every mSTP if you were using 3 reducer enhancers:

import {ensureState as ensureOptimisticState} from 'redux-optimistic-ui'
import {ensureState as ensureUndoState} from 'hypothetical-undo-library'
import {ensureState as ensureTransactionState} from 'some-other-kind-of-transaction-library'

function mapStateToProps(wrappedState) {
  const state = ensureOptimisticState(ensureUndoState(ensureTransactionState(wrappedState)))
  return {...}
}

Is this really easier to reason about than if you just define in one driver for each enhancer that determines where to store its state within your user state? And is it easier to debug? For instance any time something's wrong in your user state, you would have to expand 3 wrappers in the debugger just to get to your own state. From this I conclude that state wrapping is not a scalable pattern for using lots of reducer enhancers together (and I am sure there will be more as time goes on). It may be the simplest pattern, but I argue the costs of that simplicity outweigh the benefits.

I'm also pretty certain that you would not need a driver larger than 2 functions. I will make sure and then make a PR, and I'm pretty sure you'll be convinced it's the right approach (especially since it would be an optional use case just like getFormState).

[mattkrick]

if you've got 3 ensureState functions in your view model layer, you're messing up. You'll want to write an application-specific ensureState & then call that single function.

Default drivers are no good because that's dead code that cannot be eliminated (unless I wrote in compiler-specific rules & allowed you to build without it).

If you've got 3 lifted states from using 3 reducer enhancers simultaneously on the same route & same reducer, your app is bad and you should feel bad. Making it appear less complicated is like spraying perfume on dog dookie. Again, I can't even think of a 3rd, (optimistic, undo, and ???).

That said, could you even think of a use case where you'd be required to use both undo & optimistic-ui? Either you make their use route specific (ie replaceReducer) or you wouldn't apply both to your rootReducer. But let's say you wanna write really bad code & both packages were extenders:

• optimistic-ui copies your entire state, including undo.
• undo copies your entire state, including optimistic-ui, which oh by the way included undo.
• ...turtles all the way down until your browser crashes.

[jedwards1211]

Sigh. That's a good point about why one would not typically need a bunch of different reducer enhancers, and how the two copying each others' state would be a problem.

Do you at least understand why I think having to pepper ensureState all over my containers is gross? Do you at least wish there was a cleaner way to do it, even if it's truly impossible? I'm frustrated that you don't even seem to get my point that not having to make any change to the containers would be a big win if it were possible. I mean, redux-form is the exact opposite of expressive and easy to reason about at the cost of brevity. It's intensely brief and short and magical.

[jedwards1211]

As a side note, given that Dan Abramov made an internal state and connect pattern in react-dnd that parallels redux, I wonder why he's so against local state for use cases like this.