Incremental cleanup of selector implementation and fix for multiple stores #1568
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
facebook-github-bot
added
CLA Signed
|
This pull request was exported from Phabricator. Differential Revision: D33761732 |
drarmstr
added a commit
to drarmstr/Recoil
that referenced
this pull request
Jan 25, 2022
…tores (facebookexperimental#1568) Summary: Pull Request resolved: facebookexperimental#1568 As a follow-on to D33737833 do some more incremental cleanup and simplification of the selector implementation. Fix async selectors used from multiple stores which were not notifying other stores if they were no longer the latest evaluation in the first store to evaluate it. Differential Revision: D33761732 fbshipit-source-id: 995d3c64757ef9069956170074dddbf671f848f3
drarmstr
force-pushed
the
export-D33761732
branch
from
79a6c8d
to
bef4c63
Compare
|
This pull request was exported from Phabricator. Differential Revision: D33761732 |
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
Summary: Cleanup unit tests by organizing them a bit more. Also, a couple files were getting rather large and sometimes overruning the 10s threshold with all of the GK and React environment combinations. This splits out the common hook tests from the selector-specific ones and splits the selector tests between tests of just selectors with mock store and selectors via hooks. Differential Revision: D33735180 fbshipit-source-id: 11f27c2f7cebbe2504baa9d11a05c1fcfd4c90c6
…tores (facebookexperimental#1568) Summary: Pull Request resolved: facebookexperimental#1568 As a follow-on to D33737833 do some more incremental cleanup and simplification of the selector implementation. Fix async selectors used from multiple stores which were not notifying other stores if they were no longer the latest evaluation in the first store to evaluate it. Reviewed By: davidmccabe Differential Revision: D33761732 fbshipit-source-id: a2facd8d72ccca1dbf5c60096282c08ba9da20b8
|
This pull request was exported from Phabricator. Differential Revision: D33761732 |
drarmstr
force-pushed
the
export-D33761732
branch
from
bef4c63
to
8d8a64d
Compare
drarmstr
added a commit
to drarmstr/Recoil
that referenced
this pull request
Jan 26, 2022
drarmstr
added a commit
to drarmstr/Recoil
that referenced
this pull request
Jan 28, 2022
drarmstr
added a commit
that referenced
this pull request
Jan 29, 2022
* [docs] Recoil 0.6 Documentation * [docs] Add some more examples and document onSet() fix * [docs] Update 0.6 release notes * [docs] Update 0.6 release notes * [docs] Update Atom effects to remove unstable and upgrade Docusaurus version * [docs] Update 0.6 release notes * [docs] Update 0.6 docs for Snapshot retention. * [docs] Improve async snapshot clarification * [docs] Update node type in RecoilValueInfo<T> * [docs] Further refine retain docs for 0.6 * [docs] 0.6 Release notes updated for #1568 * [docs] Update 0.6 Release Notes * [docs] Update Recoil 0.6 release notes * [docs] Document transition support for Recoil 0.6
AlexGuz23
pushed a commit
to AlexGuz23/Recoil
that referenced
this pull request
Nov 3, 2022
…tores (#1568) Summary: Pull Request resolved: facebookexperimental/Recoil#1568 As a follow-on to D33737833 do some more incremental cleanup and simplification of the selector implementation. Fix async selectors used from multiple stores which were not notifying other stores if they were no longer the latest evaluation in the first store to evaluate it. Reviewed By: davidmccabe Differential Revision: D33761732 fbshipit-source-id: 0872f5e243141af04624cb9028ebf7b07e62a226
snipershooter0701
pushed a commit
to snipershooter0701/Recoil
that referenced
this pull request
Mar 5, 2023
…tores (#1568) Summary: Pull Request resolved: facebookexperimental/Recoil#1568 As a follow-on to D33737833 do some more incremental cleanup and simplification of the selector implementation. Fix async selectors used from multiple stores which were not notifying other stores if they were no longer the latest evaluation in the first store to evaluate it. Reviewed By: davidmccabe Differential Revision: D33761732 fbshipit-source-id: 0872f5e243141af04624cb9028ebf7b07e62a226
eagle2722
added a commit
to eagle2722/Recoil
that referenced
this pull request
Sep 21, 2024
…tores (#1568) Summary: Pull Request resolved: facebookexperimental/Recoil#1568 As a follow-on to D33737833 do some more incremental cleanup and simplification of the selector implementation. Fix async selectors used from multiple stores which were not notifying other stores if they were no longer the latest evaluation in the first store to evaluate it. Reviewed By: davidmccabe Differential Revision: D33761732 fbshipit-source-id: 0872f5e243141af04624cb9028ebf7b07e62a226
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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: As a follow-on to D33737833 do some more incremental cleanup and simplification of the selector implementation. Fix async selectors used from multiple stores which were not notifying other stores if they were no longer the latest evaluation in the first store to evaluate it.
Differential Revision: D33761732