This repository has been archived by the owner on Jan 1, 2025. It is now read-only.
Fix selector issue with early rendering mode #1566
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Summary: Normally when a dependency resolves it notifies downstream components to re-render and this is sufficient. While it is still pending, the selector returns this promise; subsequent renders while pending will re-use the same loadable using `getExecutionInfoOfInProgressExecution()`. However, there is a corner case we have to notify components that an async selector has resolved, even if the selector does not have a "current pending execution": 1) A component renders with this pending loadable. 2) The upstream dependency resolves. 3) While processing some other selector it reads this one, such as while traversing its dependencies. At this point it gets the new resolved value synchronously and clears the current execution ID. The component wasn't getting the value itself, though, so it still has the pending loadable. 4) When wrapper handling the resolution executes the current execution id was cleared, so it will never notify the component of the new value. I think this is only an issue with "early" rendering since the components read their value using the in-progress execution. Though, I'm not sure this is necessary with `recoil_concurrent_support` mode. Differential Revision: D33737833 fbshipit-source-id: 51934954bd59aece29abfe2fa9bc033b7fda805a
facebook-github-bot
added
CLA Signed
|
This pull request was exported from Phabricator. Differential Revision: D33737833 |
drarmstr
added a commit
to drarmstr/Recoil
that referenced
this pull request
Jan 25, 2022
Summary: Pull Request resolved: facebookexperimental#1566 Normally when a dependency resolves it notifies downstream components to re-render and this is sufficient. While it is still pending, the selector returns this promise; subsequent renders while pending will re-use the same loadable using `getExecutionInfoOfInProgressExecution()`. However, there is a corner case we have to notify components that an async selector has resolved, even if the selector does not have a "current pending execution": 1) A component renders with this pending loadable. 2) The upstream dependency resolves. 3) While processing some other selector it reads this one, such as while traversing its dependencies. At this point it gets the new resolved value synchronously and clears the current execution ID. The component wasn't getting the value itself, though, so it still has the pending loadable. 4) When wrapper handling the resolution executes the current execution id was cleared, so it will never notify the component of the new value. I think this is only an issue with "early" rendering since the components read their value using the in-progress execution. Though, I'm not sure this is necessary with `recoil_concurrent_support` mode. Differential Revision: https://www.internalfb.com/diff/D33737833?entry_point=27 fbshipit-source-id: 13998cfe9e3ad31152e10a4a68027f60255316f9
drarmstr
added a commit
to drarmstr/Recoil
that referenced
this pull request
Jan 25, 2022
Summary: Pull Request resolved: facebookexperimental#1566 Normally when a dependency resolves it notifies downstream components to re-render and this is sufficient. While it is still pending, the selector returns this promise; subsequent renders while pending will re-use the same loadable using `getExecutionInfoOfInProgressExecution()`. However, there is a corner case we have to notify components that an async selector has resolved, even if the selector does not have a "current pending execution": 1) A component renders with this pending loadable. 2) The upstream dependency resolves. 3) While processing some other selector it reads this one, such as while traversing its dependencies. At this point it gets the new resolved value synchronously and clears the current execution ID. The component wasn't getting the value itself, though, so it still has the pending loadable. 4) When wrapper handling the resolution executes the current execution id was cleared, so it will never notify the component of the new value. I think this is only an issue with "early" rendering since the components read their value using the in-progress execution. Though, I'm not sure this is necessary with `recoil_concurrent_support` mode. Differential Revision: https://www.internalfb.com/diff/D33737833?entry_point=27 fbshipit-source-id: f5b1c567ce43458517e672aabb4f78786c3600b8
drarmstr
added a commit
to drarmstr/Recoil
that referenced
this pull request
Jan 25, 2022
Summary: Pull Request resolved: facebookexperimental#1566 Normally when a dependency resolves it notifies downstream components to re-render and this is sufficient. While it is still pending, the selector returns this promise; subsequent renders while pending will re-use the same loadable using `getExecutionInfoOfInProgressExecution()`. However, there is a corner case we have to notify components that an async selector has resolved, even if the selector does not have a "current pending execution": 1) A component renders with this pending loadable. 2) The upstream dependency resolves. 3) While processing some other selector it reads this one, such as while traversing its dependencies. At this point it gets the new resolved value synchronously and clears the current execution ID. The component wasn't getting the value itself, though, so it still has the pending loadable. 4) When wrapper handling the resolution executes the current execution id was cleared, so it will never notify the component of the new value. I think this is only an issue with "early" rendering since the components read their value using the in-progress execution. Though, I'm not sure this is necessary with `recoil_concurrent_support` mode. Differential Revision: https://www.internalfb.com/diff/D33737833?entry_point=27 fbshipit-source-id: f2880ca63d50b5a4cfdc360175b3ed1b10f269ef
drarmstr
added a commit
to drarmstr/Recoil
that referenced
this pull request
Jan 26, 2022
Summary: Pull Request resolved: facebookexperimental#1566 Normally when a dependency resolves it notifies downstream components to re-render and this is sufficient. While it is still pending, the selector returns this promise; subsequent renders while pending will re-use the same loadable using `getExecutionInfoOfInProgressExecution()`. However, there is a corner case we have to notify components that an async selector has resolved, even if the selector does not have a "current pending execution": 1) A component renders with this pending loadable. 2) The upstream dependency resolves. 3) While processing some other selector it reads this one, such as while traversing its dependencies. At this point it gets the new resolved value synchronously and clears the current execution ID. The component wasn't getting the value itself, though, so it still has the pending loadable. 4) When wrapper handling the resolution executes the current execution id was cleared, so it will never notify the component of the new value. I think this is only an issue with "early" rendering since the components read their value using the in-progress execution. Though, I'm not sure this is necessary with `recoil_concurrent_support` mode. Differential Revision: https://www.internalfb.com/diff/D33737833?entry_point=27 fbshipit-source-id: c8ba67330f10736de0be687f1108827186041797
AlexGuz23
pushed a commit
to AlexGuz23/Recoil
that referenced
this pull request
Nov 3, 2022
Summary: Pull Request resolved: facebookexperimental/Recoil#1566 Normally when a dependency resolves it notifies downstream components to re-render and this is sufficient. While it is still pending, the selector returns this promise; subsequent renders while pending will re-use the same loadable using `getExecutionInfoOfInProgressExecution()`. However, there is a corner case we have to notify components that an async selector has resolved, even if the selector does not have a "current pending execution": 1) A component renders with this pending loadable. 2) The upstream dependency resolves. 3) While processing some other selector it reads this one, such as while traversing its dependencies. At this point it gets the new resolved value synchronously and clears the current execution ID. The component wasn't getting the value itself, though, so it still has the pending loadable. 4) When wrapper handling the resolution executes the current execution id was cleared, so it will never notify the component of the new value. I think this is only an issue with "early" rendering since the components read their value using the in-progress execution. Though, I'm not sure this is necessary with `recoil_concurrent_support` mode. Reviewed By: davidmccabe Differential Revision: D33737833 fbshipit-source-id: ac71f5ac4ea72543f384421054838f0f4c88a9c3
snipershooter0701
pushed a commit
to snipershooter0701/Recoil
that referenced
this pull request
Mar 5, 2023
Summary: Pull Request resolved: facebookexperimental/Recoil#1566 Normally when a dependency resolves it notifies downstream components to re-render and this is sufficient. While it is still pending, the selector returns this promise; subsequent renders while pending will re-use the same loadable using `getExecutionInfoOfInProgressExecution()`. However, there is a corner case we have to notify components that an async selector has resolved, even if the selector does not have a "current pending execution": 1) A component renders with this pending loadable. 2) The upstream dependency resolves. 3) While processing some other selector it reads this one, such as while traversing its dependencies. At this point it gets the new resolved value synchronously and clears the current execution ID. The component wasn't getting the value itself, though, so it still has the pending loadable. 4) When wrapper handling the resolution executes the current execution id was cleared, so it will never notify the component of the new value. I think this is only an issue with "early" rendering since the components read their value using the in-progress execution. Though, I'm not sure this is necessary with `recoil_concurrent_support` mode. Reviewed By: davidmccabe Differential Revision: D33737833 fbshipit-source-id: ac71f5ac4ea72543f384421054838f0f4c88a9c3
eagle2722
added a commit
to eagle2722/Recoil
that referenced
this pull request
Sep 21, 2024
Summary: Pull Request resolved: facebookexperimental/Recoil#1566 Normally when a dependency resolves it notifies downstream components to re-render and this is sufficient. While it is still pending, the selector returns this promise; subsequent renders while pending will re-use the same loadable using `getExecutionInfoOfInProgressExecution()`. However, there is a corner case we have to notify components that an async selector has resolved, even if the selector does not have a "current pending execution": 1) A component renders with this pending loadable. 2) The upstream dependency resolves. 3) While processing some other selector it reads this one, such as while traversing its dependencies. At this point it gets the new resolved value synchronously and clears the current execution ID. The component wasn't getting the value itself, though, so it still has the pending loadable. 4) When wrapper handling the resolution executes the current execution id was cleared, so it will never notify the component of the new value. I think this is only an issue with "early" rendering since the components read their value using the in-progress execution. Though, I'm not sure this is necessary with `recoil_concurrent_support` mode. Reviewed By: davidmccabe Differential Revision: D33737833 fbshipit-source-id: ac71f5ac4ea72543f384421054838f0f4c88a9c3
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
Normally when a dependency resolves it notifies downstream components to re-render and this is sufficient. While it is still pending, the selector returns this promise; subsequent renders while pending will re-use the same loadable using
getExecutionInfoOfInProgressExecution().However, there is a corner case we have to notify components that an async selector has resolved, even if the selector does not have a "current pending execution":
I think this is only an issue with "early" rendering since the components read their value using the in-progress execution. Though, I'm not sure this is necessary with
recoil_concurrent_supportmode.Differential Revision: D33737833