Skip to content

fix[AppContainer]: mount react devtools overlay only when devtools are attached - #38785

Merged
lunaleaps merged 1 commit into
react:0.72-stablefrom
hoxyq:0.72-stable/fix-mounting-devtools-overlay
Aug 7, 2023
Merged

fix[AppContainer]: mount react devtools overlay only when devtools are attached#38785
lunaleaps merged 1 commit into
react:0.72-stablefrom
hoxyq:0.72-stable/fix-mounting-devtools-overlay

Conversation

@hoxyq

@hoxyqhoxyq commented Aug 4, 2023

Copy link
Copy Markdown
Contributor

Cherry-picking #38727 into 0.72

…e connected (#38727)
Summary:
Pull Request resolved: #38727
## Changelog:
[General][Fix] - Do not render React DevTools overlays unless they are connected
Fixes#38024.
Reproducible example: https://github.com/rasaha91/rn-bottomsheet.
- This is a temporary workaround to resolve described problem for DEV bundles without attached React DevTools.
- Still, such problem will be present for DEV bundles with attached React DevTools, but this should be only for brownfield apps with a shrinked React Native window.
Checking that DevTools hook is present is not enough to determine whether the DevTools are connected. These changes fix it.
Short description of what's going on there:
1. When React Native root window is rendered inside some native view, which takes only portion of the screen (like the native bottom sheet in the reproducible example), DevtoolsOverlay / InspectorOverlay takes up space based on application window dimension, this results into resizing the hosting window on the native side.
https://pxl.cl/357r3
2. Right way to fix this would be removing the usage of application window sizes, so that DevtoolsOverlay / InspectorOverlay will be allowed only to take React Native's window.
3. Unfortunately, just removing setting is not enough, we should also have at least 1 of 2 things:
- `collapsable` prop should be set to `true` => View will be flattened
- Remove [`flex: 1` style on both root and inner Views](https://github.com/facebook/react-native/blob/b28e3c16ed7cbc8b3ed3f26d91c58acb4bb28879/packages/react-native/Libraries/ReactNative/AppContainer.js#L145-L147), but this is breaking how LogBox works now.
| {F1062478964} | {F1062492367}
Reviewed By: NickGerleman
Differential Revision: D47954883
fbshipit-source-id: d45d8cb82daa8dc9de58f54c137815b3a7abd5db
@hoxyq
hoxyq requested a review from fortmarekAugust 4, 2023 10:31
@facebook-github-botfacebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Facebook Partner: Facebook Partner labels Aug 4, 2023
@github-actions

Copy link
Copy Markdown
Fails
🚫

📋 Missing Changelog - Can you add a Changelog? To do so, add a "## Changelog" section to your PR description. A changelog entry has the following format: [CATEGORY] [TYPE] - Message.

DetailsCATEGORY may be:
  • General
  • 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.

Warnings
⚠️📋 Missing Summary - Can you add a Summary? To do so, add a "## Summary" section to your PR description. This is a good place to explain the motivation for making this change.
⚠️📋 Missing Test Plan - Can you add a Test Plan? To do so, add a "## Test Plan" section to your PR description. A Test Plan lets us know how these changes were tested.

Generated by 🚫 dangerJS against ea3efa8

@analysis-bot

Copy link
Copy Markdown
PlatformEngineArchSize (bytes)Diff
androidhermesarm64-v8a8,490,802-404,234
androidhermesarmeabi-v7a7,808,869-134,718
androidhermesx868,960,847-332,075
androidhermesx86_648,821,819-372,622
androidjscarm64-v8a9,150,105-331,919
androidjscarmeabi-v7a8,339,578-83,989
androidjscx869,203,167-262,875
androidjscx86_649,461,590-318,684

Base commit: e22d1a1
Branch: main

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA SignedThis label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.p: FacebookPartner: FacebookPartnerPick Request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@hoxyq@analysis-bot@lunaleaps@facebook-github-bot