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

fix bug with selectors reading atoms with effects in snapshots from GitHub issue #1107 #1135

Closed

Conversation

csantos42
Copy link
Contributor

Summary:
Description of setup:

  • A selector is read from a snapshot, and the selector depends on an atom that has an atom effect which calls setSelf() synchronously and adds some code to call setSelf() asynchronously later on, and the first time the app is reading the atom is in that snapshot (to reproduce it's important that the app has not read the atom before and therefore the atom has not initialized before, meaning the effects for that atom have not run yet). Then, the selector is read using a hook.

Expected behavior

  • The atom effect runs twice as it is first initialized by the snapshot and then by the root store via a hook. The async calls to setSelf() should lead to re-renders with an updated selector value as its dependent atom has changed in value.

Issue:

  • The atom effect only initializes once, so only the snapshot is aware of the asynchronous setSelf() calls but the root store is not, which results in the async calls to setSelf() being ignored and the UI does not update.

Differential Revision: D29886554

…itHub issue facebookexperimental#1107

Summary:
Description of setup:
- A selector is read from a snapshot, and the selector depends on an atom that has an atom effect which calls `setSelf()` synchronously and adds some code to call `setSelf()` asynchronously later on, and the first time the app is reading the atom is in that snapshot (to reproduce it's important that the app has not read the atom before and therefore the atom has not initialized before, meaning the effects for that atom have not run yet). Then, the selector is read using a hook.

Expected behavior
- The atom effect runs twice as it is first initialized by the snapshot and then by the root store via a hook. The async calls to `setSelf()` should lead to re-renders with an updated selector value as its dependent atom has changed in value.

Issue:
- The atom effect only initializes once, so only the snapshot is aware of the asynchronous `setSelf()` calls but the root store is not, which results in the async calls to `setSelf()` being ignored and the UI does not update.

Differential Revision: D29886554

fbshipit-source-id: b6270fd8a8a3299b37f4a79de4822d7f7c48cdaf
@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 Jul 29, 2021
@facebook-github-bot
Copy link
Contributor

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

@drarmstr drarmstr added the bug Something isn't working label Jul 29, 2021
@drarmstr drarmstr linked an issue Jul 29, 2021 that may be closed by this pull request
@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 2e4a102.

AlexGuz23 pushed a commit to AlexGuz23/Recoil that referenced this pull request Nov 3, 2022
…itHub issue #1107 (#1135)

Summary:
Pull Request resolved: facebookexperimental/Recoil#1135

Description of setup:
- A selector is read from a snapshot, and the selector depends on an atom that has an atom effect which calls `setSelf()` synchronously and adds some code to call `setSelf()` asynchronously later on, and the first time the app is reading the atom is in that snapshot (to reproduce it's important that the app has not read the atom before and therefore the atom has not initialized before, meaning the effects for that atom have not run yet). Then, the selector is read using a hook.

Expected behavior
- The atom effect runs twice as it is first initialized by the snapshot and then by the root store via a hook. The async calls to `setSelf()` should lead to re-renders with an updated selector value as its dependent atom has changed in value.

Issue:
- The atom effect only initializes once, so only the snapshot is aware of the asynchronous `setSelf()` calls but the root store is not, which results in the async calls to `setSelf()` being ignored and the UI does not update.

Reviewed By: drarmstr

Differential Revision: D29886554

fbshipit-source-id: e06cd3643bb7e5f8d4a3e666e90e3edfb36cd140
snipershooter0701 pushed a commit to snipershooter0701/Recoil that referenced this pull request Mar 5, 2023
…itHub issue #1107 (#1135)

Summary:
Pull Request resolved: facebookexperimental/Recoil#1135

Description of setup:
- A selector is read from a snapshot, and the selector depends on an atom that has an atom effect which calls `setSelf()` synchronously and adds some code to call `setSelf()` asynchronously later on, and the first time the app is reading the atom is in that snapshot (to reproduce it's important that the app has not read the atom before and therefore the atom has not initialized before, meaning the effects for that atom have not run yet). Then, the selector is read using a hook.

Expected behavior
- The atom effect runs twice as it is first initialized by the snapshot and then by the root store via a hook. The async calls to `setSelf()` should lead to re-renders with an updated selector value as its dependent atom has changed in value.

Issue:
- The atom effect only initializes once, so only the snapshot is aware of the asynchronous `setSelf()` calls but the root store is not, which results in the async calls to `setSelf()` being ignored and the UI does not update.

Reviewed By: drarmstr

Differential Revision: D29886554

fbshipit-source-id: e06cd3643bb7e5f8d4a3e666e90e3edfb36cd140
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. fb-exported Merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Using promise to setSelf in atom's effect
4 participants