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.6k stars 1.19k forks source link

useRecoilSnapshot() not returning all modified atoms #2059

Open luishdz1010 opened 2 years ago

luishdz1010 commented 2 years ago

https://codesandbox.io/s/repro-pvv1nl?file=/pages/index.tsx

Tab between the inputs and look at the console. The state changes on blur are visible to the atom but not included in useRecoilSnapshot().getNodes_UNSTABLE({isModified: true}), if you tab outside the inputs or setTimeout the blur change, the change is included in the snapshot. Since I'm using recoil-sync, this makes the external synced store lose updates and get into an inconsistent state.

Initially reported here: https://github.com/facebookexperimental/Recoil/issues/1935#issuecomment-1269322780

drarmstr commented 2 years ago

It appears from this reproducer that if some state updates are batched then we may not see all modified atoms marked as modified in the snapshot even though the actual state in the snapshot is the latest and correct. This can cause recoil-sync to miss updates.

nahtnam commented 1 year ago

Any update or workarounds on this?

luishdz1010 commented 1 year ago

Using setTimeout in places where the updates can be contiguous is the only workaround I know of. It's not really maintenable, but might unblock you.

nahtnam commented 1 year ago

@luishdz1010 so if we have two updates, we should do one, then setTimeout for like 10ms for the next one?

MartinSeeler commented 1 year ago

I can confirm this bug, even by batching two updates inside useRecoilTransaction_UNSTABLE, only the first one is in the new snapshot.

nahtnam commented 1 year ago

Anyone have any pointers on where this is in the codebase? It’s blocking for my application and I would like to see if I can figure it out

miklschmidt commented 10 months ago

This one caused me quite a headache over the past couple of days. It gets really fun when using selectors btw (delaying updates is not a workaround in that case).

The only workaround i can think of is manually saving atom state to the store in onSet on another atom effect, disable recoil-sync writes for good measure.

miklschmidt commented 10 months ago

I'm not entirely sure if this snapshot behavior is intended (it doesn't seem like it), or if recoil-sync just shouldn't use snapshots to create the diff it uses to sync to the store.