Skip to content
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

useRecoilCallback() snapshot gets the latest state #1610

Closed

Conversation

drarmstr
Copy link
Contributor

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 #1604

Differential Revision: D34236485

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported labels Feb 15, 2022
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D34236485

@gurkerl83
Copy link

gurkerl83 commented Feb 15, 2022

@drarmstr
Hi, I tried to build this pull request without success. The build mentions a circular dependency; the bundles are generated but with unresolved symbols in them. Linking recoil back to the application and running it causes an error.

drarmstr added a commit to drarmstr/Recoil that referenced this pull request 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
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D34236485

@drarmstr drarmstr linked an issue Feb 15, 2022 that may be closed by this pull request
drarmstr added a commit to drarmstr/Recoil that referenced this pull request 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
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D34236485

@drarmstr drarmstr self-assigned this Feb 16, 2022
@drarmstr drarmstr added bug Something isn't working enhancement New feature or request labels Feb 16, 2022
@drarmstr
Copy link
Contributor Author

@drarmstr Hi, I tried to build this pull request without success. The build mentions a circular dependency; the bundles are generated but with unresolved symbols in them. Linking recoil back to the application and running it causes an error.

Yes, it seems that our internal build avoids the circular dependency when using non-dereferencing imports while the open source build does not. I'll have to break the circular dependency another way.

…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
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D34236485

@drarmstr drarmstr deleted the export-D34236485 branch February 23, 2022 03:43
AlexGuz23 pushed a commit to AlexGuz23/Recoil that referenced this pull request Nov 3, 2022
Summary:
Pull Request resolved: facebookexperimental/Recoil#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
snipershooter0701 pushed a commit to snipershooter0701/Recoil that referenced this pull request Mar 5, 2023
Summary:
Pull Request resolved: facebookexperimental/Recoil#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
eagle2722 added a commit to eagle2722/Recoil that referenced this pull request Sep 21, 2024
Summary:
Pull Request resolved: facebookexperimental/Recoil#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 join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. enhancement New feature or request fb-exported
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Debounce callback with version 0.6.0
3 participants