possibility that the state from the store is inconsistent

[faceyspacey]

regarding:

// run callback in each commit phase in case something has changed.
// [CAUTION] Limitations in subscription in useEffect
// There is a possibility that the state from the store is inconsistent
// across components which may cause problems in edge cases.

Are you referring to the react-redux issue that makes that library so much more complicated where they have to deal with "zombie components" ??

Truthfully, I never fully understood the issue. I always got close, but never spent the time to reproduce it so i could see it for myself.

If we have that issue still, unfortunately we have to solve it to be taken seriously. I recall that did it with nested usage of context in order to track the relationship between parent HOCs and descendant HOCs. So if, a parent is unsubscribed, insure the child also unsubscribes even if it has stale state that results in it it still rendering. Something like that I think was the issue.

[dai-shi]

As far as I can tell, we call callback.current() every commit phase as you see right followed by that comment, stale props/state are resolved in the next rendering for sure. But that also means there's a chance that components can be rendered once with stale props/state.

esamattis/redux-hooks#9 suggested the use of unstable_batchUpdates, which might solve the edge case.

I'm interested in unstable_batchUpdates for performance too.

---

Please let me know if I confuse stale props/state with zombie components.

[dai-shi]

Just merged #12.
This should help reducing the stale props issue. However, I don't think it will cover all edge cases, so I left callback.current() as it was.

In summary, the stale props issue is almost solved. The exception is the rare case when the global state is updated between triggering rendering and actual rendering, which will only be resolved by next rendering right after rendering with stale props.

The bad news is that this behavior is the limitation of hooks + subscription and it can't be helped. (One possible hack might be using useLayoutEffect and throwing a promise if it detects stale props. I wouldn't take that approach...)

The good news is that this library is new and there's no backward compatibility issue like react-redux.

[dai-shi]

By following the thread reduxjs/react-redux#1179, I'm pretty sure I misunderstood something.
The example was helpful.

Zombie children is not just an issue of stale props, but an unrecoverable error caused by the stale props.
This can't be resolved by just calling callback again. It might be recoverable with a good error boundary (maybe?)

I modified the example for react-hooks-easy-redux: modified example
The error is reproduced with react-hooks-easy-redux v1.3.0, but not with v1.4.0.
This means batchedUpdates helps for this example.

[dai-shi]

I'll remove callback.current() as it doesn't help resolve zombie children issue.

[faceyspacey]

love the prevision. wonder what additional examples we need. wonder if batchedUpdates 100% solves the issue of stale props.

[dai-shi]

wonder if batchedUpdates 100% solves the issue of stale props.

That's what I'm not sure either. Any breaking example would be nice.

Since this is not only our problem, we keep eyes on reduxjs/react-redux#1179.

[faceyspacey]

yup, offload work upstream ;)

[esamattis]

Hi, author of @epeli/redux-hooks here.

@dai-shi @faceyspacey

wonder if batchedUpdates 100% solves the issue of stale props.

Unfortunately no. I've just recently discovered that my hooks implementation is still buggy in some cases. Using batchedUpdates lessens the impact a bit but does not solve it completely as shown here:

https://codesandbox.io/s/p77jyzjkv7

Sadly currently it seems that there's no way to implement fully bug free Redux Hooks without a wrapping component. Or any single atom state management with hooks for that matter...

[dai-shi]

@epeli Thanks for coming over!

Yeah, I didn't expect that batchedUpdates solves 100% of the issue.

Thanks for your example.
I modified it with our library. Could you take a moment to review it?

https://codesandbox.io/s/2p4kxnxj1j

The semantics of our library is totally different from that of react-redux.
So, I'm not totally sure if I captured it correctly.

[esamattis]

Looked bit more closely at this. It actually might be possible that batchedUpdates is enough for react-hooks-easy-redux because there's no map state happening before render execution.

If I understand correctly it just forces component rendering when a used part of the state changes and the used parts are detected during the initial render?

That's actually quite clever.

[dai-shi]

Looked bit more closely at this. It actually might be possible that batchedUpdates is enough for react-hooks-easy-redux because there's no map state happening before render execution.

Yeah, and because it doesn't have any local state.
Do you know if batchedUpdates renders components top-down regardless of the order of update triggers in it?

If I understand correctly it just forces component rendering when a used part of the state changes and the used parts are detected during the initial render?

Yes, and not only the initial render but also every render.

That's actually quite clever.

Glad to hear that.

[esamattis]

Do you know if batchedUpdates renders components top-down regardless of the order of update triggers in it?

It's how I understand it. As I mention in my lib's issue there's a Batching chapter in the Dan's React as a UI Runtime -article which explains it.

The issue in other Redux hooks lib is the map state phase: The map state executes immediately when the store updates so if the map state function is inside a render function and references parent props via closure those props can be based on the previous version of the state and can be incompatible with currento one. Your solution neatly dodges that :)

UPDATE: Moved mutation suggestion to own issue #14

[faceyspacey]

@epeli guess you need to change the top line of your readme yet again:

React Hooks implementation for Redux that does not suffer from the tearing / "zombie child component" problems.

UPDATE Nope. This is buggy too. And so is every other Redux Hooks lib.

😇

ps. I have thought of that

[esamattis]

Haha, I hope so! 😄

[faceyspacey]

@theKashey how do you think the points @epeli brings up affects our plan to build selectors into the store, i.e:

the map state executes immediately when the store updates so if the map state function is inside a render function and references parent props via closure those props can be based on the previous version of the state and can be incompatible with current one

@epeli FYI we are working on an improved redux that has something like this:

const selectors = {
   aSelector: (state, foo, bar) => ...,
}

createStore(reducer, selectors, etc)

const MyComponent = (props) => {
   const state = useReduxState();
   return <div>{state.aSelector(props.foo, props.bar)}</div>
}

yea, it's more like MobX and MobX State Tree in that we are modeling our entire "UI Database" upfront, not just reducers

In this system, selector returns are cached (not memoized) for use in potentially multiple places. And of course proxies (proxyequal) are used under the hood to determine updating, similar to reducers in this library.

[faceyspacey]

@theKashey my intuition says, our selectors also dont have this problem, yay! Check:

@epeli 's redux-hooks:

import {useMapState, useActionCreators} from "@epeli/redux-hooks";

const ActionCreators = {
    inc() {
        return {type: "INCREMENT"};
    },
};

function Counter(props) {
    const count = useMapState(state => state.count + props.baseValue); // CLOSURE!!
    const actions = useActionCreators(ActionCreators);

    return <button onClick={actions.inc}>{count}</button>;
}

vs ours:

const MyComponent = (props) => {
   const state = useReduxState();
   return <div>{state.aSelector(props.foo, props.bar)}</div>
}

At the moment I'm not quite sure how the closure breaks things. Because if the problem is just using props in general, we'll still have the problem too. Thoughts??

[dai-shi]

It's how I understand it. As I mention in my lib's issue there's a Batching chapter in the Dan's React as a UI Runtime -article which explains it.

Good. I've read the article and I assumed it is a natural behavior, but I wasn't 100% confident.
Yeah, the example in the article explains that. Thanks. @epeli

Given that, I would like to close this issue, mainly because there's my misunderstanding at the beginning. @faceyspacey

We can safely say that in most cases, we don't have the issue. There might be an edge case that we can't think of now, but that would be a new issue.

[esamattis]

@faceyspacey If the selector runs within the render phase it should be ok.