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 useRecoilSnapshot() with Fast Refresh #1891

Closed

Conversation

drarmstr
Copy link
Contributor

Summary:
Fix support for useRecoilSnapshot() when Fast Refresh. (Note this is dev-only)

React's "Fast Refresh" will detect changes to source and re-load and re-render components with the new implementation. However, it appears to call the new render, then the previous effects cleanup handler, then the new effects. This seems strange and caused the snapshot to be released before the new effect retained it. Don't expect behviour change from React, so workaround is to defer the release with setTimeout() so the new effect after the fast refresh has a chance to retain it before the old effect cleanup releases it.

Differential Revision: D37475404

Summary:
Fix support for `useRecoilSnapshot()` when Fast Refresh.  (Note this is dev-only)

React's "Fast Refresh" will detect changes to source and re-load and re-render components with the new implementation.  However, it appears to call the new render, then the previous effects cleanup handler, then the new effects.  This seems strange and caused the snapshot to be released before the new effect retained it.  Don't expect behviour change from React, so workaround is to defer the release with `setTimeout()` so the new effect after the fast refresh has a chance to retain it before the old effect cleanup releases it.

Differential Revision: D37475404

fbshipit-source-id: 3797eecc1775d3e883b90897e6e00fe0453126b8
@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 Jun 28, 2022
@facebook-github-bot
Copy link
Contributor

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

drarmstr added a commit to drarmstr/Recoil that referenced this pull request Jun 28, 2022
Summary: Update changelog for facebookexperimental#1891

Differential Revision: D37503573

fbshipit-source-id: 9ef7cbffcddb9f0a36d6da56843ae041aaa1cd10
@drarmstr drarmstr self-assigned this Jun 28, 2022
@drarmstr drarmstr added the bug Something isn't working label Jun 28, 2022
@drarmstr drarmstr deleted the export-D37475404 branch June 28, 2022 22:46
facebook-github-bot pushed a commit that referenced this pull request Jun 30, 2022
Summary:
Pull Request resolved: #1894

Update changelog for #1891

Reviewed By: habond

Differential Revision: D37503573

fbshipit-source-id: 0bc9754edfefeee3f00d766b81cc80d8c9a49e60
snipershooter0701 pushed a commit to snipershooter0701/Recoil that referenced this pull request Mar 5, 2023
Summary:
Pull Request resolved: facebookexperimental/Recoil#1891

Fix support for `useRecoilSnapshot()` when Fast Refresh.  (Note this is dev-only)

React's "Fast Refresh" will detect changes to source and re-load and re-render components with the new implementation.  However, it appears to call the new render, then the previous effects cleanup handler, then the new effects.  This seems strange and caused the snapshot to be released before the new effect retained it.  Don't expect behviour change from React, so workaround is to defer the release with `setTimeout()` so the new effect after the fast refresh has a chance to retain it before the old effect cleanup releases it.

Reviewed By: habond

Differential Revision: D37475404

fbshipit-source-id: a570324864a688b49f0fcfd55459b54e7abdef39
eagle2722 added a commit to eagle2722/Recoil that referenced this pull request Sep 21, 2024
Summary:
Pull Request resolved: facebookexperimental/Recoil#1891

Fix support for `useRecoilSnapshot()` when Fast Refresh.  (Note this is dev-only)

React's "Fast Refresh" will detect changes to source and re-load and re-render components with the new implementation.  However, it appears to call the new render, then the previous effects cleanup handler, then the new effects.  This seems strange and caused the snapshot to be released before the new effect retained it.  Don't expect behviour change from React, so workaround is to defer the release with `setTimeout()` so the new effect after the fast refresh has a chance to retain it before the old effect cleanup releases it.

Reviewed By: habond

Differential Revision: D37475404

fbshipit-source-id: a570324864a688b49f0fcfd55459b54e7abdef39
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants