-
Notifications
You must be signed in to change notification settings - Fork 11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Component not aware of state change? #20
Comments
Just give an example to fight with! (ok, there is a sandboxes) |
Working on Chrome, not working on Safari or FF |
In short - this is due to |
@theKashey latest Chrome? On mine its not working on at most second/third click. |
So - by the time second update hits
|
@dai-shi - useProxyfied should use |
In short - a new store update got triggered while an old update is not fully "commited".
|
Oh... I thought about that before, but yeah. (I've got to leave now... 🚗 ) |
Yeah, concurrent mode. Actually, we have got tearing here, when different pieces are using states from different times.
|
Unfortunately this doesnt resolve the issue 😔 |
Good to know. I will make a useLayoutEffect version. I’m not sure if I understand the second bullet, yet. |
Internal state was not yet updated, as a result the second comparison with an old state and a quick state update is a false positive, and might took more time than a comparison with state2. So - once you compare |
Got it. It’s false positives. I wonder if that would cause other false negatives in some edge cases? |
Should not. You already triggered |
OK, let me make a fix. |
Since you pushed to master, we can use it on Codesandbox by using:
in Its based on your example from Additionaly you can use my example repo on localhost with this:
|
Oh... It wasn't about useLayoutEffect nor updating state. Interestingly, if you add one more dispatch, it works. @mrGibi const hc = () => {
dispatch({ type: "increment" });
dispatch({ type: "increment" });
dispatch({ type: "increment" });
}; If you have another to make them four, it won't. |
I dont get it... I had two examples showing the issue:
I thought that both examples are pretty much identical, but the second one was just simplified version of the first one. Its all about making two (or more) fast changes to the state and component not being aware of those changes, right? Well it turns out that they are not as similar as I thought, becouse after your latest commit to master, the first example (now using latest And the second example with:
now using latest |
Please try the latest master. |
Ok, both examples are working now. Thanks! |
So, the question remained to me is whether we really need
So, your approach totally depends on |
Yes. As long as you might have multiple renders before single |
If we both revert If both are correct, oh and if we solve the concurrent mode issue in the future, we don't need |
@theKashey I also want your comment on this. |
🤷♂️ I am not sure why it works. Give me some time to play with it. |
This "if we solve the concurrent mode issue" is kind of strong condition more than I thought on the previous day. |
I've been fighting this all day today.
Im new to Java/TypeScript world, React world and Redux world and I was pretty sure that I messed something up... until I tried doing the same with React-Redux (7.0.0.beta-1) and it turned out it works there!
Im writing a authorization flow with redux-saga - so this example that Im going to show you is just massively simplified code similar to mine. CodeSandboxes below.
Description: TOGGLE button dispatches an action that is caught by redux-saga. The Saga dispatches an action to toggle a value to
true
, wait 10ms, and then dispatches another action to toggle value back tofalse
. Meanwhile the button is disabled ifvalue === true
, and there is avalue ? 'ON' : 'OFF'
text. Interestingly if you increasedelay()
value to (for example) 50ms it works!How does it look like:
using react-redux 7.0.0-beta.1 (good!)
using reactive-react-redux 1.8.0 (bad!)
This was shot on MacOs 10.14.4 and Safari 12.1.
I have tested this also on:
Check out these codesandboxes:
I dropped on this issue by accident. In my original code the
delay
function is just a Api call. This Api call had a wrong port number set so it gotConnection refused
and returned immediately.TURN_ON
andTURN_OFF
was just aisLoading
indicator that disabled a Login button.@dai-shi Im totally blown away by Your work on
reactive-react-redux
and @theKashey 'sproxyequal
. I follow every discussion, every benchmark test and commits to this repo and wish I would understand enough of it to be able to help...The text was updated successfully, but these errors were encountered: