Skip to content
This repository has been archived by the owner on Jan 1, 2025. It is now read-only.

Debounce callback with version 0.6.0 #1604

Closed
xotahal opened this issue Feb 13, 2022 · 1 comment
Closed

Debounce callback with version 0.6.0 #1604

xotahal opened this issue Feb 13, 2022 · 1 comment
Assignees
Labels
bug Something isn't working

Comments

@xotahal
Copy link
Contributor

xotahal commented Feb 13, 2022

Hey folks! I am not sure if this is an issue or wanted behaviour. But out of curiosity. I created this codesandbox where we have one atom and two buttons. Atom has a default value set to first. When you press any button it will change the value of the atom and then in useEffect it calls useRecoilCallback where it gets the value from the atom and console log it.

  1. Without debounce it sets the value to second. Then in useEffect it correctly prints out the second as a value.
  2. The debounced button sets the value to third. Then in useEffect it prints out the previous value of the atom (either first or second). But not the third. As I would expect.

It works with 0.5 version.

I think it could be related to this change #1501. Any chance someone could explain to me what exactly happened?

Thank you 🙏

@xotahal
Copy link
Contributor Author

xotahal commented Feb 13, 2022

Here is maybe a bit more "real world" example. Where I created AmountControl component. It has debounced onChange. Once it gets to the parent it changes the state.

There are two useEffects, one of them uses debounced useRecoilCallback (that works fine) and the other one calls the useRecoilCallback without debounce. This one prints always previous value of the state.

@drarmstr drarmstr self-assigned this Feb 15, 2022
@drarmstr drarmstr added the bug Something isn't working label Feb 15, 2022
drarmstr added a commit to drarmstr/Recoil that referenced this issue Feb 15, 2022
Summary:
NOTE: This is a breaking change, but the previous approach had a bug.

Previously, `useRecoilCallback()` would attempt to provide a `Snapshot` based on the state that was currently rendered.  However, there were some bugs where this was not the case.  For example, a callback called from an effect handler may or may not have had the latest rendered state based on the timing of the setter.

The new approach `useRecoilCallback()` will always try to get a snapshot with the latest state.  This should also be more intuitive for users.

Addresses facebookexperimental#1604

Differential Revision: D34236485

fbshipit-source-id: 773574feda763a74734503090474fff5c30de73f
drarmstr added a commit to drarmstr/Recoil that referenced this issue Feb 15, 2022
…tal#1610)

Summary:
Pull Request resolved: facebookexperimental#1610

NOTE: This is a breaking change, but the previous approach had a bug.

Previously, `useRecoilCallback()` would attempt to provide a `Snapshot` based on the state that was currently rendered.  However, there were some bugs where this was not the case.  For example, a callback called from an effect handler may or may not have had the latest rendered state based on the timing of the setter.

The new approach `useRecoilCallback()` will always try to get a snapshot with the latest state.  This should also be more intuitive for users.

Addresses facebookexperimental#1604

Reviewed By: habond

Differential Revision: D34236485

fbshipit-source-id: 8a41e15556900dfb41628b2d77ea23409166f50a
@drarmstr drarmstr linked a pull request Feb 15, 2022 that will close this issue
drarmstr added a commit to drarmstr/Recoil that referenced this issue Feb 15, 2022
…tal#1610)

Summary:
Pull Request resolved: facebookexperimental#1610

NOTE: This is a breaking change, but the previous approach had a bug.

Previously, `useRecoilCallback()` would attempt to provide a `Snapshot` based on the state that was currently rendered.  However, there were some bugs where this was not the case.  For example, a callback called from an effect handler may or may not have had the latest rendered state based on the timing of the setter.

The new approach `useRecoilCallback()` will always try to get a snapshot with the latest state.  This should also be more intuitive for users.

Addresses facebookexperimental#1604

Reviewed By: habond

Differential Revision: D34236485

fbshipit-source-id: fc10621f4569bff753c1755b4c2e41601d6b8069
drarmstr added a commit to drarmstr/Recoil that referenced this issue Feb 16, 2022
…tal#1610)

Summary:
Pull Request resolved: facebookexperimental#1610

NOTE: This is a breaking change, but the previous approach had a bug.

Previously, `useRecoilCallback()` would attempt to provide a `Snapshot` based on the state that was currently rendered.  However, there were some bugs where this was not the case.  For example, a callback called from an effect handler may or may not have had the latest rendered state based on the timing of the setter.

The new approach `useRecoilCallback()` will always try to get a snapshot with the latest state.  This should also be more intuitive for users.

Addresses facebookexperimental#1604

Reviewed By: habond

Differential Revision: D34236485

fbshipit-source-id: b8c79540e1e86fe8228bd32b4f003b7254d17f88
facebook-github-bot pushed a commit that referenced this issue Feb 16, 2022
Summary:
Pull Request resolved: #1610

NOTE: This is a breaking change, but the previous approach had a bug.

Previously, `useRecoilCallback()` would attempt to provide a `Snapshot` based on the state that was currently rendered.  However, there were some bugs where this was not the case.  For example, a callback called from an effect handler may or may not have had the latest rendered state based on the timing of the setter.

The new approach `useRecoilCallback()` will always try to get a snapshot with the latest state.  This should also be more intuitive for users.

Addresses #1604

Reviewed By: habond

Differential Revision: D34236485

fbshipit-source-id: 0cb17cec40b50e8debeb57dc193b9f410d170fc5
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants