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.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid RCTUnsafeExecuteOnMainQueueSync from RCTUIManager #1266

Conversation

christophpurrer
Copy link

@christophpurrer christophpurrer commented Jul 21, 2022

Please select one of the following

  • I am removing an existing difference between facebook/react-native and microsoft/react-native-macos 馃憤
  • I am cherry-picking a change from Facebook's react-native into microsoft/react-native-macos 馃憤
  • I am making a fix / change for the macOS implementation of react-native
  • I am making a change required for Microsoft usage of react-native

Summary

These calls are only needed for a temporary Fabric workaround and would result in a deadlock since we also call RCTUnsafeExecuteOnUIManagerQueueSync from the main thread during window resize.

This is needed for now in Fabric only to call the temporary componentViewName_DO_NOT_USE_THIS_IS_BROKEN workaround on views, which we don't have on desktop since we're a way off from using Fabric (and hopefully this workaround will be gone then).

It was originally added here facebook@ffc7ec9

At Meta we have disabled these lines since May 2021 to fix crashes in production.

Changelog

[macOS] [Fixed] - Avoid RCTUnsafeExecuteOnMainQueueSync from RCTUIManager

Test Plan

Ran rn-tester.
Crash no longer observed

These calls are only needed for a temporary Fabric workaround and would result in a deadlock since we also call `RCTUnsafeExecuteOnUIManagerQueueSync` from the main thread during window resize.

This is needed for now in Fabric only to call the temporary `componentViewName_DO_NOT_USE_THIS_IS_BROKEN` workaround on views, which we don't have on desktop since we're a way off from using Fabric (and hopefully this workaround will be gone then).

It was originally added here facebook@ffc7ec9

At Meta we have disabled these lines since May 2021 to fix crashes in production.
@christophpurrer christophpurrer force-pushed the avoidRTCUnsafeExecuteOnMainQueueSync branch from c9599c1 to 371ef59 Compare July 22, 2022 14:33
@pull-bot
Copy link

Messages
馃摉

馃搵 Verify Changelog Format - A changelog entry has the following format: [CATEGORY] [TYPE] - Message.

CATEGORY may be:
  • General
  • macOS
  • iOS
  • Android
  • JavaScript
  • Internal (for changes that do not need to be called out in the release notes)

TYPE may be:

  • Added, for new features.
  • Changed, for changes in existing functionality.
  • Deprecated, for soon-to-be removed features.
  • Removed, for now removed features.
  • Fixed, for any bug fixes.
  • Security, in case of vulnerabilities.

MESSAGE may answer "what and why" on a feature level. Use this to briefly tell React Native users about notable changes.

Generated by 馃毇 dangerJS against 371ef59

@amgleitman amgleitman merged commit 46da582 into microsoft:main Jul 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants