Uh oh!
There was an error while loading. Please reload this page.
fix: android native rejections should be instanceof Error - #44487
fix: android native rejections should be instanceof Error#44487huzhanbo1996 wants to merge 2 commits into
Conversation
Hi, @javache As in #44051, you suggests that a fix from Java to C++. But I found out that same as fix on iOS #41955, we can treat reject on C++. If we want to fix it from Java, we probably need a Java Error type (currently WritableNativeMap is universal argument for callback), and the argument conversion should support Java Error -> Dynamic -> JSI Object to this specific type. It looks this solution needs huge work. If you have any other ideas, please let me know. |
Base commit: 54f582f |
| } | ||
| auto createRejectionError(jsi::Runtime& rt, folly::dynamic args) { | ||
| DCHECK(args.size() == 1) << "promise reject should has only one argument"; |
There was a problem hiding this comment.
Use react_native_assert so we get consistent assert handling
| return; | ||
| } |
| jsError.asObject(rt).setProperty( | ||
| rt, propertyName, valueAsObject.getProperty(rt, propertyName)); |
There was a problem hiding this comment.
Cache jsError.asObject so we don't repeatedly create a new jsi::Value here
| auto propertyNames = valueAsObject.getPropertyNames(rt); | ||
| for (size_t i = 0; i < propertyNames.size(rt); ++i) { | ||
| auto propertyName = propertyNames.getValueAtIndex(rt, i).asString(rt); |
There was a problem hiding this comment.
Wrap in PropNameID.forString() to avoid doing this conversion twice.
javache
commented
May 9, 2024
Looks good overall, just a few nits |
huzhanbo1996
commented
May 10, 2024
Thanks for the advice. Changes updated, PTAL again |
facebook-github-bot
commented
May 10, 2024
@javache has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
huzhanbo1996
commented
May 13, 2024
Hi @javache, is the CI check OK? |
facebook-github-bot
commented
May 14, 2024
This pull request was successfully merged by huzhanbo.luc in 62cbdbb. When will my fix make it into a release? | How to file a pick request? |
Summary: fixreact#44050 ## Changelog: [ANDROID] [FIXED] - fix: android native rejections should be instanceof Error Pull Request resolved: react#44487 Test Plan: reject returns Error. Reviewed By: NickGerleman Differential Revision: D57205131 Pulled By: javache fbshipit-source-id: a5950481d0c4909be4dbea0b430e75222258ae68
Summary: fix#44050 ## Changelog: [ANDROID] [FIXED] - fix: android native rejections should be instanceof Error Pull Request resolved: #44487 Test Plan: reject returns Error. Reviewed By: NickGerleman Differential Revision: D57205131 Pulled By: javache fbshipit-source-id: a5950481d0c4909be4dbea0b430e75222258ae68
Summary: fix#44050 ## Changelog: [ANDROID] [FIXED] - fix: android native rejections should be instanceof Error Pull Request resolved: #44487 Test Plan: reject returns Error. Reviewed By: NickGerleman Differential Revision: D57205131 Pulled By: javache fbshipit-source-id: a5950481d0c4909be4dbea0b430e75222258ae68
Summary:
fix#44050
Changelog:
[ANDROID] [FIXED] - fix: android native rejections should be instanceof Error
Test Plan:
reject returns Error.