jumpsuit / jumpstate

Jumpstate is a simple and powerful state management utility for Redux.
MIT License
425 stars 22 forks source link

Before & after hooks/effects #30

Open timoxley opened 7 years ago

timoxley commented 7 years ago

Currently it appears both hooks and effects run before the reducer. I'd like to be able to fire a side effect after the reducer has updated the state e.g. to write updated state to in-browser db.

Is this possible using the existing API and if not would you consider a new post effects/hooks API?

tannerlinsley commented 7 years ago

I would accept a PR to change some things up here. Not sure how to go about it though, what do you have in mind?

codinronan commented 7 years ago

@timoxley I like this idea. I will investigate, shouldn't be too hard to do actually.

timoxley commented 7 years ago

FWIW I was hoping I could hack this by "shadowing" a sandboxed action with an effect, but jumpstate complains that they use the same name.

const Data = State('datas', {
  addData (state, payload) {
    // …
  }
})

Effect('addData', (data) => {
  Data.addData(data)
})

I guess I could change the sandboxed action name, e.g. prefix with underscore.

timoxley commented 7 years ago

@tannerlinsley @codinronan In order for state to have been updated it'd need to either run asynchronously or right at the very end of the middleware chain.

codinronan commented 7 years ago

hey @timoxley @tannerlinsley I started looking into this and it seems like we'd need to pass the store back in after store creation. The only way to be certain that you're emitting after the store updates is to use store.subscribe(), which is not available to middleware.

Any other ideas? Would you guys be opposed to an API that worked that way? It does impact the simplicity of Jumpstate a little (obviously it would be opt-in).

codinronan commented 7 years ago

Hey guys ok so I was thinking more about this and I think I have a solution. We don't "necessarily" need to trigger an effect/hook after the store has updated - store updates occur synchronously within an Effect (any Promise or generator), so, as long as the Effect or Hook returns a promise, it can be then'd - and here's the critical part - this then() will execute after any synchronous state changes (or state changes that occur within a promise that gets returned from the Effect). I take advantage of this extensively in my current project - I have some fairly sophisticated Effect chaining, but the chains are explicit and a bit verbose. I'm currently doing something like this, and I didn't even realize I was solving @timoxley 's problem:

function loadValuations() {
  return Actions.getSomeData()
           .catch(() => {})
           .then(Actions.getSomeOtherData)
           .catch(() => {})
           .then(Actions.processOtherData)
           .catch(() => {});
}

Each of these calls are actually Effects and each one returns a Promise of the calls they are wrapping. This means that any code contained within them will complete before the chain proceeds. Nevermind the catches in between, although I'll mention them in a moment.

So my proposal is a new type of Effect with two signatures:

EffectChain('chainName', Effect1, Effect2, ..., EffectN);
OR
EffectChain({name: 'chainName', exceptionHandler: () => {} }, Effect1, Effect2, ..., EffectN);

Ok so to explain. Basically this function would build dynamically the piece of code I have written above, interjecting the catch handler in between each (or allowing the throw if not specified).

It would simply then the chain, in either case. You are thus specifying dependency chains in a declarative way. I realize this is nothing groundbreaking, I simply realized it could be used in this scenario.

What is the advantage of this? Well - If Effect1 produces state changes, they will be flushed by the time Effect2 is called. I have verified this locally (remember, state changes are synchronous. In fact I have run into some nasty bugs that arise because of this - components throw exceptions on react-redux connect() calls, which cause the Effect to crash unexpectedly).

So - you basically get what @timoxley is asking for without having to create a convoluted subscription API.

By the way, this is not exactly the same as Promise.all - which I tried before. The problem with Promise.all is that it short circuits in the case of an exception. We can retain that behavior, but also allow it to be overridden by specifying the exception handler (e.g. if it is absent, the catches are not interjected).

What do you guys think?

timoxley commented 7 years ago

@codinronan great thoughts though I'm not sure if that promise stuff should be the responsibility of this lib… i.e. if users want different promise error handling pattern, perhaps that's better handled with utility specifically for that.

also note your original code sample could perhaps be shorter by using the second arg to .then?

return Promise.resolve()
.then(Actions.getSomeData, () => {})
.then(Actions.getSomeOtherData, () => {})
.then(Actions.processOtherData, () => {})

The pattern you describe is neat, but I'm not clear how it would help with the use-case I originally presented:

after the reducer has updated the state… write updated state to in-browser db

IIUC using the above pattern would require wrapping every action with the "write to db" action, which perhaps is a good idea because it's explicit, but I was thinking something more like a global handler for injecting that behaviour.

codinronan commented 7 years ago

@timoxley Yep it's not a perfect replacement. And the bigger problem is, it is too opinionated. It's just one way for the consumer to solve the problem right now without changing the library.

I would like to think of a way for the library to support this scenario though. It's just, Redux middlewares are explicitly not intended to run after the reducers, so we're crawling uphill with this one.