Remove State snapshot from StoreSubscriber

[kittinunf]

Based on main redux documentation, listener function signature is subscribe(listener: () => void): Unsubscribe;

https://github.com/reactjs/redux/blob/master/docs/FAQ.md#how-do-i-subscribe-to-only-a-portion-of-the-state-can-i-get-the-dispatched-action-as-part-of-the-subscription

I propose we should match that in other words, we remove the state argument from StoreSubscriber

From

public interface StoreSubscriber<S> {
    void onStateChange(S state);
}

To

public interface StoreSubscriber<S> {
    void onStateChange();
}

Then it is responsibility of the call site to call store.state in order to get current state value.

[beyondeye]

I am afraid this cannot be done. There reason is that Reduks engine is much more asynchronous than the original. multiple reduce/dispatch operation can run in parallel, and notification to subscribers happens independently (if yow want take a look at KovenantStore). This is actually a design choice

[kittinunf]

Wouldn't that make the change even more compelling? Call site could presume that the state that sent along with argument is current one, while the truth is it isn't.

The reason behind is that I update UI by using the argument from the subscriber block. Little did I know that the state has been changed behind my back to something else by dispatching another function. This makes my UI render the out-dated state instead of using the latest version which looks wrong to me.

[beyondeye]

If you are using rxJava or simplestore the behavior is what you expect to be.

[beyondeye]

So don't be afraid. Anyway I will look into it. Perhaps I need to change the design

[kittinunf]

I understand, just want to let you know that I was bitten hard by UI jerking reason is using the state argument from the subscriber block. When I moved to use store.state, everything is fine and working as expected. Perhaps, this is why main redux repo moving away from that version, as well. I remember they were using the (State) => void in the past.

[beyondeye]

Can you tell me more about what was happening in your UI? I would like to try to understand what was happening

[beyondeye]

for reference here is the discussion about it in redux
subscribe API with state as an argument · Issue #303 · reactjs/redux
reduxjs/redux#303

[beyondeye]

In brief: the main reason is to make the methods in Store API, subscribe() and getState() are orthogonal on purpose so if you “override” one in a store enhancer, you don’t need to override the other.

[kittinunf]

But, redux.js still using the () => void, isn't it? I am still convinced that we should follow them. My problem with that as I mentioned to you is the state argument was an old copy version of my state.

[beyondeye]

I think we actually have two issues here:

1. Reduks Store API vs original Redux Store API
2. state changes debouncing
   Apparently by using getState you are effectively getting the debounced state changes. This is what you need in most cases, but even in the original redux this is not the default behaviour. There is a special store enhancer for that (https://github.com/tappleby/redux-batched-subscribe)

[beyondeye]

About the first issue I agree with you. Just I need to think a bit to make it work with KovenantReduks.
About the second issue, need to decide if going Redux way (have a separate store enhancer) or add some built in options to debounce state changes

[beyondeye]

The problem with porting Redux is that the core API is relatively low level. It is not really useful without adding a lot of other stuff. This is the reason I added other things like Selectors to Reduks core. Then it becomes a little bit opinionated. Not to mention that kotlin and javascript are quite different in many aspects, which forces additional choices that can distance Reduks from the original. It is not easy to strike the right balance

[beyondeye]

solved in release 2.0.0b6

[kittinunf]

💛