react-native
react-native copied to clipboard
[iOS] Fixes the exported synchronous method not being called on the method queue when it's the main queue
Summary:
If the module sets the method queue to the main queue, we should call it on the main queue if it contains some UI operations, otherwise it may lead to some undefined behavior.
Changelog:
[IOS] [FIXED] - Fixes the exported synchronous method not being called on the method queue when it's the main queue
Test Plan:
The sync method should be called on the main queue if the module's method queue is main queue.
I don't think this is desirable to do automatically.
Any method marked as sync should be invoked synchronously from the JS thread. If a module requires this to block the UI thread and avoid race conditions, that dispatch_sync should be written explicitly inside the native module method.
I don't think this is desirable to do automatically.
Any method marked as sync should be invoked synchronously from the JS thread. If a module requires this to block the UI thread and avoid race conditions, that dispatch_sync should be written explicitly inside the native module method.
@javache Yes, module can call dispatch_sync in its own module, but actually, we said we would run all module's methods on the specific queue. And in some modules like Appearance, it just thinks it should be called on the main queue and access some UIKit methods, maybe user's custom modules also have this potential issue.
https://github.com/facebook/react-native/blob/b37101486bd4dc26f6ec4e646e12d1484ec9479f/packages/react-native/React/Base/RCTBridgeModule.h#L160-L163
@javache Hi, as you said, we need to fix the issue in the modules like Appearance right? If so, I can change my PR to fix those modules issues.
@javache Hi, as you said, we need to fix the issue in the modules like Appearance right? If so, I can change my PR to fix those modules issues.
Yes, let's do that. If they have sync methods called on the wrong thread, that's the right place for a fix.
@javache Done. Please review again.
@javache has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
This pull request was successfully merged by @zhongwuzw in 8bfd7e10393e649554c7246df430019c4f78d5e0
When will my fix make it into a release? | How to file a pick request?