Skip to content

Conversation

@HeyImChris
Copy link

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

This extra check was added over two years ago before this was migrated to GitHub. In that migration we lost the source history but it's fairly easy to piece together why this was added and therefore, why we can remove it.

This check I'm removing was added in a bulk add next to the RCT_DEV macros throughout the code base. It's causing problems being here because calling into DevSettings before we finish initialization is deadlocking in that it tries to create the DevSettings module if it doesn't exist. This gets into a race condition where it tries to create a module before RN is in a state that modules can be created.

Removing it is safe because the check it's around is completely innocuous and won't matter if it wraps the executor in debug or ship builds either way.

This change allows us to enable web debugging scenarios downstream by fixing a crucial deadlock issue.

Changelog

[Apple] [Bug] - Fix devsettings deadlock

Test Plan

Tested downstream and web debugging works

HeyImChris and others added 19 commits February 4, 2021 11:38
* Update RCTCxxBridge.mm

* add nullability checks
@HeyImChris HeyImChris requested a review from harrieshin June 29, 2021 03:01
@HeyImChris HeyImChris self-assigned this Jun 29, 2021
@HeyImChris HeyImChris requested a review from alloy as a code owner June 29, 2021 03:01
@pull-bot
Copy link

Warnings
⚠️

❔ Base Branch - The base branch for this PR is something other than master. Are you sure you want to merge these changes into a stable release? If you are interested in backporting updates to an older release, the suggested approach is to land those changes on master first and then cherry-pick the commits into the branch for that release. The Releases Guide has more information.

Messages
📖

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

DetailsCATEGORY 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 576da0a

@HeyImChris HeyImChris merged commit 7aad4a9 into microsoft:0.63-stable Jun 29, 2021
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.

4 participants