-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Add error reporting for attempted async selector set #777
Conversation
Summary: Recoil does not support async selector sets. However, as it does support async gets many users expect async sets. Flow reports a static error if an `async` function is used, TypeScript should as well. Though users may not have or notice this error. This diff throws an error if callbacks in the `set` are attempted to be used asynchronously. However, those errors may not be caughts and be lost. So, we also detect if an `async` function was used by checking if the user's set returns a `Promise` and throws an error for that as well. Differential Revision: D25250686 fbshipit-source-id: 113398d9ca09ff4ff47f681125cc91967750a0ab
This pull request was exported from Phabricator. Differential Revision: D25250686 |
Will async sets on selector will never be implemented? Imagine I have two kind of events that im subscribed on externally, channelAdded, channelRemoved, on channelAdded and channelRemoved i get as argument a channel instance from an 3rd party lib, and need to call some methods that return promises, like getMembers() etc, in order to serialize, Then we endup having a bidirectional selector get/set where the get return alls channels and the set have a switch case on the argument to evaluate value.type 'ADDED' or 'REMOVED' in order to do something with value.payload but just the added need to serialize. Ideally we should need to have the derivate data logic inside the setter and not doing out previously the set call, currently with the current implementation that doesnt support async set, we need to serialise out and send to the set the channel serialize in case of added and the channel without serialize in case of remove. |
This pull request has been merged in 73345f5. |
Summary: Pull Request resolved: facebookexperimental/Recoil#777 Recoil does not support async selector sets. However, as it does support async gets many users expect async sets. Flow reports a static error if an `async` function is used, TypeScript should as well. Though users may not have or notice this error. This diff throws an error if callbacks in the `set` are attempted to be used asynchronously. However, those errors may not be caughts and be lost. So, we also detect if an `async` function was used by checking if the user's set returns a `Promise` and throws an error for that as well. Reviewed By: davidmccabe Differential Revision: D25250686 fbshipit-source-id: 93c529f09734142cdd04ea0eda7097f814b120cb
Summary: Pull Request resolved: facebookexperimental/Recoil#777 Recoil does not support async selector sets. However, as it does support async gets many users expect async sets. Flow reports a static error if an `async` function is used, TypeScript should as well. Though users may not have or notice this error. This diff throws an error if callbacks in the `set` are attempted to be used asynchronously. However, those errors may not be caughts and be lost. So, we also detect if an `async` function was used by checking if the user's set returns a `Promise` and throws an error for that as well. Reviewed By: davidmccabe Differential Revision: D25250686 fbshipit-source-id: 93c529f09734142cdd04ea0eda7097f814b120cb
Summary: Recoil does not support async selector sets. However, as it does support async gets many users expect async sets. Flow reports a static error if an
async
function is used, TypeScript should as well. Though users may not have or notice this error. This diff throws an error if callbacks in theset
are attempted to be used asynchronously. However, those errors may not be caughts and be lost. So, we also detect if anasync
function was used by checking if the user's set returns aPromise
and throws an error for that as well.Differential Revision: D25250686