facebook / react-native

A framework for building native applications using React
https://reactnative.dev
MIT License
116.27k stars 23.99k forks source link

fix: android native rejections should be instanceof Error #44487

Closed huzhanbo1996 closed 5 days ago

huzhanbo1996 commented 1 week ago

Summary:

fix #44050

Changelog:

[ANDROID] [FIXED] - fix: android native rejections should be instanceof Error

Test Plan:

reject returns Error.

huzhanbo1996 commented 1 week ago

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.

analysis-bot commented 1 week ago
Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 19,495,386 -45,446
android hermes armeabi-v7a n/a --
android hermes x86 n/a --
android hermes x86_64 n/a --
android jsc arm64-v8a 22,867,572 -44,417
android jsc armeabi-v7a n/a --
android jsc x86 n/a --
android jsc x86_64 n/a --

Base commit: 54f582f651c68e9fa6bedb5d1546f4e5a4a64704 Branch: main

javache commented 1 week ago

Looks good overall, just a few nits

huzhanbo1996 commented 1 week ago

Looks good overall, just a few nits

Thanks for the advice. Changes updated, PTAL again

facebook-github-bot commented 1 week ago

@javache has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

huzhanbo1996 commented 6 days ago

@javache has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

Hi @javache, is the CI check OK?

facebook-github-bot commented 5 days ago

@javache merged this pull request in facebook/react-native@62cbdbbcc60f9074e47416007aef81e0792d7c95.

github-actions[bot] commented 5 days ago

This pull request was successfully merged by huzhanbo.luc in 62cbdbbcc60f9074e47416007aef81e0792d7c95.

When will my fix make it into a release? | How to file a pick request?