Use synchronous native module calls when New Architecture is enabled #2152
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
yousif-bugsnag
requested review from
lemnik and
gingerbenw
and removed request for
lemnik
yousif-bugsnag
force-pushed
the
PLAT-12175/sync-tm-methods
branch
2 times, most recently
from
2584263 to
72f6971
Compare
gingerbenw
approved these changes
Jun 5, 2024
Comment on lines
+153
to
+195
| # Tests that the given payload value is correct for the target arch and RN version combination. | ||
| # This step will assume the expected and payload values are strings. | ||
| # | ||
| # The DataTable used for this step should have the headers `arch` `version` and `value` | ||
| # The `arch` column must contain either `old` or `new` | ||
| # The `version` column should contain the version of the app to test the value against, or `default` for unspecified versions | ||
| # The `value` column should contain the expected value for the given arch and version combination | ||
| # | ||
| # | arch | version | value | | ||
| # | new | 0.74 | Error | | ||
| # | new | default | java.lang.RuntimeException | | ||
| # | old | default | java.lang.RuntimeException | | ||
| # | ||
| # If the expected value is set to "@null", the check will be for null | ||
| # If the expected value is set to "@not_null", the check will be for a non-null value | ||
| Then('the event {string} equals the version-dependent string:') do |field_path, table| | ||
| payload = Maze::Server.errors.current[:body] | ||
| payload_value = Maze::Helper.read_key_path(payload, "events.0.#{field_path}") | ||
| expected_values = table.hashes | ||
|
|
||
|
|
||
| arch = ENV['RCT_NEW_ARCH_ENABLED'] == 'true' ? 'new' : 'old' | ||
| arch_values = expected_values.select do |hash| | ||
| hash['arch'] == arch | ||
| end | ||
|
|
||
| raise("There is no expected value for the current arch \"#{arch}\"") if arch_values.empty? | ||
|
|
||
| current_version = ENV['RN_VERSION'] | ||
| version_values = arch_values.select do |hash| | ||
| hash['version'] == current_version | ||
| end | ||
|
|
||
| if version_values.empty? | ||
| version_values = arch_values.select { |hash| hash['version'] == 'default' } | ||
| end | ||
|
|
||
| raise("There is no expected value for the current version \"#{current_version}\"") if version_values.empty? | ||
| raise("Multiple expected values found for arch \"#{arch}\" and version \"#{current_version}\"") if version_values.length() > 1 | ||
|
|
||
| expected_value = version_values[0]['value'] | ||
| assert_equal_with_nullability(expected_value, payload_value) | ||
| end |
yousif-bugsnag
force-pushed
the
PLAT-12175/sync-tm-methods
branch
from
72f6971 to
51d86ec
Compare
yousif-bugsnag
force-pushed
the
PLAT-12175/sync-tm-methods
branch
from
51d86ec to
5949754
Compare
…ORT_RETURN` and ensure correct return type in header file
yousif-bugsnag
force-pushed
the
PLAT-12175/sync-tm-methods
branch
from
5949754 to
ba8507f
Compare
Merged
This was referenced Sep 24, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Goal
In RN 0.74 New Architecture (bridgeless mode), once an exception is thrown, async promises are no longer being resolved. This means JS exceptions are not being reported, because post-error communication with the native layer to capture native state and send an exception for delivery is not able to run.
This PR fixes the issue by making calls to the native layer synchronous when the New Architecture is enabled.
As part of this, RN 0.74 has been added to the test matrix and the e2e tests have been updated accordingly.
Design
Synchronous native methods
There are 2 explicitly async (i.e. promise-returning) methods involved in post-error communication:
getPayloadInfo(used inplugin-react-native-event-sync)dispatch(used indelivery-react-native)These have been converted to synchronous methods and separate async versions
getPayloadInfoAsyncanddispatchAsyncadded. Both packages have been updated to use the synchronous methods when the New Arch is enabled.For the remaining native module methods, a preprocessor directive is used on iOS to export those methods as synchronous when the new architecture is enabled (on iOS,
voidnative methods that don't return a value in JS are also asynchronous unless exported on the native side withRCT_EXPORT_BLOCKING_SYNCHRONOUS_METHOD)Testing
React Native 0.74 has now been added to the test matrix. A new step definition has been added to support testing payload values for a given architecture (old vs new) and RN version combination.