search

facebookexperimental / Recoil

Recoil is an experimental state management library for React apps. It provides several capabilities that are difficult to achieve with React alone, while being compatible with the newest features of React.
https://recoiljs.org/
MIT License
19.62k stars 1.19k forks source link

Setting atom from an effect concurrently with another update loses the value #774

Open idubrov opened 3 years ago

idubrov commented 3 years ago

Potentially, similar to #728.

Setting atom value inside an effect (via setSelf) while another update is happening loses the state change. Delaying setSelf via window.requestAnimationFrame solves the issue.

Here is the reproduction case I have: https://codesandbox.io/s/xenodochial-firefly-wny9s?file=/src/App.js

  1. Click "Navigate!". Observe that "State A" value is empty and "State B" is "hello".
  2. Now reload page in the embedded browser with ?delay added to the URL.
  3. Click "Navigate!". Observe that "State A" value is now correct ("from effect") and "State B" is "hello".
drarmstr commented 3 years ago

Please note that calling setSelf() from window.requestAnimationFrame() will first initialize the atom to the default value, then asynchronously update the value. Calling setSelf() synchronously in the effect will initialize that atom to that value for the first render, which is important for things like SSR and rendering the initial state without potential flicker.

idubrov commented 3 years ago

Sure. However, if I don't set stateB, it works as expected. Which makes it very confusing behavior. It only shows up if both are true:

  1. We have data ready in our effect (so we call setSelf immediately).
  2. At the same time, we set some other, unrelated state as well.
drarmstr commented 3 years ago

Found the issue, thanks for the great repro @idubrov! I have a fix in the works, though may be delayed through reviews due to holidays..

mdlavin commented 3 years ago

@drarmstr I noticed that even after your fix was released I still had a case of an effect setting an initial value that was having its value get lost. It seemed related to this bug, so I'm mentioning it here. I opened PR #991 to demonstrate the incorrect behavior.

I spent a little time debugging the code to look for a fix based on your previous fix, but this seems different enough that a new fix didn't jump out at me. I hope my testcase can help somebody debug the problem. If you point me at a rough place that might be causing the problem, I'm happy to take another stab at fixing it myself

drarmstr commented 3 years ago

@mdlavin - Thank you very much for the test case! Yes, I suspect this test is unrelated to the original issue here.