-
Notifications
You must be signed in to change notification settings - Fork 24.3k
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
RCTPromise now requires every native module method to resolve with an array. #5851
Comments
Hey geof90, thanks for reporting this issue! React Native, as you've probably heard, is getting really popular and truth is we're getting a bit overwhelmed by the activity surrounding it. There are just too many issues for us to manage properly.
|
Also a note that this change is not documented anywhere, e.g the website docs do not say that the RCTPromiseResolveBlock must always resolve with the value wrapped in an array. |
Let me look into this. Someone else reported this internally. I'll look today. Thanks for reporting it. |
@dmmiller Thanks for looking into it!! Really appreciate it. |
Summary: public In 9baff8f#diff-8d9841e5b53fd6c9cf3a7f431827e319R331, I incorrectly assumed that iOS was wrapping promises in an extra Array. What was really happening is that all the callers were doing this. I removed the wrapping in the callers and the special case handling MessageQueue. Now one can pass whatever object one wants to resolve and it will show properly in the resolve call on the js side. This fixes issue #5851 Reviewed By: nicklockwood Differential Revision: D2921565 fb-gh-sync-id: 9f81e2a87f6a48e9197413b843e452db345a7ff9 shipit-source-id: 9f81e2a87f6a48e9197413b843e452db345a7ff9
Resolved in c9a1956 |
Thanks guys!! |
Summary: public In facebook@9baff8f#diff-8d9841e5b53fd6c9cf3a7f431827e319R331, I incorrectly assumed that iOS was wrapping promises in an extra Array. What was really happening is that all the callers were doing this. I removed the wrapping in the callers and the special case handling MessageQueue. Now one can pass whatever object one wants to resolve and it will show properly in the resolve call on the js side. This fixes issue facebook#5851 Reviewed By: nicklockwood Differential Revision: D2921565 fb-gh-sync-id: 9f81e2a87f6a48e9197413b843e452db345a7ff9 shipit-source-id: 9f81e2a87f6a48e9197413b843e452db345a7ff9
Summary: public In 9baff8f#diff-8d9841e5b53fd6c9cf3a7f431827e319R331, I incorrectly assumed that iOS was wrapping promises in an extra Array. What was really happening is that all the callers were doing this. I removed the wrapping in the callers and the special case handling MessageQueue. Now one can pass whatever object one wants to resolve and it will show properly in the resolve call on the js side. This fixes issue #5851 Reviewed By: nicklockwood Differential Revision: D2921565 fb-gh-sync-id: 9f81e2a87f6a48e9197413b843e452db345a7ff9 shipit-source-id: 9f81e2a87f6a48e9197413b843e452db345a7ff9
thanks,this fix my problem. |
Thanks so much for fixing this guys - this was driving me nuts! |
Summary: public In facebook@9baff8f#diff-8d9841e5b53fd6c9cf3a7f431827e319R331, I incorrectly assumed that iOS was wrapping promises in an extra Array. What was really happening is that all the callers were doing this. I removed the wrapping in the callers and the special case handling MessageQueue. Now one can pass whatever object one wants to resolve and it will show properly in the resolve call on the js side. This fixes issue facebook#5851 Reviewed By: nicklockwood Differential Revision: D2921565 fb-gh-sync-id: 9f81e2a87f6a48e9197413b843e452db345a7ff9 shipit-source-id: 9f81e2a87f6a48e9197413b843e452db345a7ff9
The issue is related to this change:
9baff8f#diff-8d9841e5b53fd6c9cf3a7f431827e319R331
Because of this, right now, every native module has to resolve with an array, which is silly if we only intend to resolve with a single value.
For example, pre RN 0.20-rc1, we could do:
Now we have to wrap it with an array and do:
From what I see, the simple fix is to revert that change. Running our plugin on RN 0.20-rc1 yielded this error:
After reverting that change, everything worked great.
@dmmiller @ide
The text was updated successfully, but these errors were encountered: