Description
Intro
This is an issue to share my research & learnings around the various patches that live in the android-patches folder.
The goal is to upstream back to RN core as much code as we can, in order to deviate as little as possible. To each patch I've assigned a state, based on my understanding, of how upstreamable it is:
- 🔴 = not upstreamable. This is for those cases in which the changes are super custom / might deviate from the direction of the code in core.
- 🟡 = some parts upstreamable. This patch can probably be split into parts that are upstreamable, and some that are not. Ideally, we should upstream as much as we can and just keep the 🔴 part.
- 🟢 = entirely upstreamable. This indicates that we should open PRs on core to upstream the changes. This means that after that PR is merged and this fork has caught up, we can then remove the local patch. (ex. fix(android-patches): remove ImageColor patch #1177)
Patches folders
Sadly, for most of these patches there is little to zero documentation (that I could find) clarifying what they are trying to achieve; so this section is to help create some (at least basic) context for each one. Take what's written below with a grain of salt as it's mostly based on me digging through code and making assumtpions.
Build 🟡
It's a 3-in-1 situation:
- there's some V8 specific changes
- some are related to Folly build errors
- some are to bump Boost to 1.68
The Folly part I'm not sure if it can be upstreamed, but I've already taken care of the boost part: facebook#33565
Once main branch here reaches 0.69, that part of the patch can be removed.
Focus 🟢
It's a 3-in-1 situation:
- Prevent focus leaks due to removal of a focused View
- change of enums → this can be removed because it’s an old change been made redundant by the refactoring with text input
- topOnFocusChange stuff
This should be upstreamable as 3 separate PRs; the main problem is that test needs to be done in keyboard mode (real scenario is a tablet with comments). Some early investigation of upstreaming can be found here facebook#33578 but it needs to be re-done from scratch as 3 separate PRs.
JniUtils 🔴
By looking at this #901 it sounds like it’s just a small optimization that could be done because of no Fabric. So for 0.68 we might need to remove because we've been introducing Fabric support?
Just an headsup, this patch will require some refactoring after this facebook@b8f415e lands in main branch.
MAC 🔴
It’s a small adjustment to this allow to disable DevMenu using new method isSecondaryClickToShowDevMenuEnabled by Simek · Pull Request #528 · microsoft/react-native-macos (github.com)
OfficeRNHost 🟡
Creates a native module registry and we can control when the instance is created. Since these are mostly C++ changes, it should be potentially upstreamable, but needs more investigation/context.
RootViewAttach 🟡
This patch reverts facebook@2c896d3 - and was done here 763bb08.
Upstream, they have actually reintroduced it here add experiment for running the JS on view creation · facebook/react-native@2dd33b1 (github.com) via a flag, only available via RN0.68. So the patch might be removed and just the flag might be turned on.
V8 🔴
As the name implies, these are V8 specific changes - so they can't be upstreamed.