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.

Sign up for GitHub

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

Jump to bottom

RCTPromise now requires every native module method to resolve with an array. #5851

Closed
geof90 opened this issue Feb 10, 2016 · 8 comments
Closed

RCTPromise now requires every native module method to resolve with an array. #5851

geof90 opened this issue Feb 10, 2016 · 8 comments
Labels
Resolution: Locked This issue was locked by the bot.

Comments

@geof90
Copy link
Contributor

geof90 commented Feb 10, 2016

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:

resolve(@"A value");

Now we have to wrap it with an array and do:

resolve(@[@"A value"]);

From what I see, the simple fix is to revert that change. Running our plugin on RN 0.20-rc1 yielded this error:
screen shot 2016-02-09 at 3 50 06 pm

After reverting that change, everything worked great.

@dmmiller @ide
@facebook-github-bot
Copy link
Contributor

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.

  • If you don't know how to do something or something is not working as you expect but not sure it's a bug, please ask on StackOverflow with the tag react-native or for more real time interactions, ask on Discord in the #react-native channel.
  • If this is a feature request or a bug that you would like to be fixed, please report it on Product Pains. It has a ranking feature that lets us focus on the most important issues the community is experiencing.
  • We welcome clear issues and PRs that are ready for in-depth discussion. Please provide screenshots where appropriate and always mention the version of React Native you're using. Thank you for your contributions!

@geof90
Copy link
Contributor Author

geof90 commented Feb 10, 2016

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.

@geof90 geof90 mentioned this issue Feb 10, 2016
Closed
@dmmiller
Copy link

Let me look into this. Someone else reported this internally. I'll look today. Thanks for reporting it.

@geof90
Copy link
Contributor Author

geof90 commented Feb 10, 2016

@dmmiller Thanks for looking into it!! Really appreciate it.

ghost pushed a commit that referenced this issue Feb 10, 2016
Fix promises on iOS to no longer wrap values in Arrays
c9a1956 
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
@skevy
Copy link
Contributor

skevy commented Feb 10, 2016

Resolved in c9a1956

@skevy skevy closed this as completed Feb 10, 2016
@geof90
Copy link
Contributor Author

geof90 commented Feb 10, 2016

Thanks guys!!

christopherdro pushed a commit to christopherdro/react-native that referenced this issue Feb 12, 2016
@christopherdro
Fix promises on iOS to no longer wrap values in Arrays
2a3e86a 
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
bestander pushed a commit that referenced this issue Feb 15, 2016
@bestander
Fix promises on iOS to no longer wrap values in Arrays
2c6875e 
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
@cnsnake11
Copy link

thanks，this fix my problem.

@kanerogers
Copy link

Thanks so much for fixing this guys - this was driving me nuts!

pglotov pushed a commit to pglotov/react-native that referenced this issue Mar 15, 2016
@pglotov
Fix promises on iOS to no longer wrap values in Arrays
4160126 
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
@facebook facebook locked as resolved and limited conversation to collaborators May 24, 2018
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Jul 20, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
Resolution: Locked This issue was locked by the bot.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
7 participants
@skevy @kanerogers @dmmiller @facebook-github-bot @geof90 @cnsnake11 @react-native-bot