Skip to content

Revert QE for ReactFeatureFlags.reduceDeleteCreateMutation #42339

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.

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Closed
ryancat wants to merge 1 commit into from
Closed

Revert QE for ReactFeatureFlags.reduceDeleteCreateMutation #42339

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion packages/react-native/ReactAndroid/api/ReactAndroid.api
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1911,7 +1911,6 @@ public class com/facebook/react/config/ReactFeatureFlags {
public static field enableViewRecycling Z
public static field excludeYogaFromRawProps Z
public static field fixStoppedSurfaceTagSetLeak Z
public static field reduceDeleteCreateMutation Z
public static field reduceDeleteCreateMutationLayoutAnimation Z
public static field rejectTurboModulePromiseOnNativeError Z
public static field traceTurboModulePromiseRejections Z
Expand Down
6 changes: 0 additions & 6 deletions .../react-native/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,12 +112,6 @@ public class ReactFeatureFlags {
*/
public static boolean reduceDeleteCreateMutationLayoutAnimation = true;

/**
* Allow fix to drop delete...create mutations which could cause missing view state in Fabric
* SurfaceMountingManager.
*/
public static boolean reduceDeleteCreateMutation = false;

/** Report mount operations from the host platform to notify mount hooks. */
public static boolean enableMountHooks = false;

Expand Down
38 changes: 1 addition & 37 deletions packages/react-native/ReactAndroid/src/main/jni/react/fabric/FabricMountingManager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,22 +30,10 @@

namespace facebook::react {

constexpr static auto kReactFeatureFlagsJavaDescriptor =
"com/facebook/react/config/ReactFeatureFlags";

static bool getFeatureFlagValue(const char* name) {
static const auto reactFeatureFlagsClass =
jni::findClassStatic(kReactFeatureFlagsJavaDescriptor);
const auto field = reactFeatureFlagsClass->getStaticField<jboolean>(name);
return reactFeatureFlagsClass->getStaticFieldValue(field);
}

FabricMountingManager::FabricMountingManager(
std::shared_ptr<const ReactNativeConfig>& config,
jni::global_ref<JFabricUIManager::javaobject>& javaUIManager)
: javaUIManager_(javaUIManager),
reduceDeleteCreateMutation_(
getFeatureFlagValue("reduceDeleteCreateMutation")) {}
: javaUIManager_(javaUIManager) {}

void FabricMountingManager::onSurfaceStart(SurfaceId surfaceId) {
std::lock_guard lock(allocatedViewsMutex_);
Expand Down Expand Up @@ -288,30 +276,6 @@ void FabricMountingManager::executeMount(
case ShadowViewMutation::Create: {
bool shouldCreateView =
!allocatedViewTags.contains(newChildShadowView.tag);
if (reduceDeleteCreateMutation_) {
// Detect DELETE...CREATE situation on the same node and do NOT push
// back to the mount items. This is an edge case that may happen
// when for example animation runs while commit happened, and we
// want to filter them out here to capture all possible sources of
// such mutations. The re-ordering logic here assumes no
// DELETE...CREATE in the mutations, as we will re-order mutations
// and batch all DELETE instructions in the end.
auto it = std::remove_if(
cppDeleteMountItems.begin(),
cppDeleteMountItems.end(),
[&](auto& deletedMountItem) -> bool {
return deletedMountItem.oldChildShadowView.tag ==
newChildShadowView.tag;
});
bool hasDeletedViewsWithSameTag = it != cppDeleteMountItems.end();
cppDeleteMountItems.erase(it, cppDeleteMountItems.end());

if (hasDeletedViewsWithSameTag) {
shouldCreateView = false;
LOG(ERROR)
<< "XIN: Detect DELETE...CREATE on the same tag from mutations in the same batch. The DELETE and CREATE mutations are removed before sending to the native platforms";
}
}

if (shouldCreateView) {
cppCommonMountItems.push_back(
Expand Down
2 changes: 0 additions & 2 deletions packages/react-native/ReactAndroid/src/main/jni/react/fabric/FabricMountingManager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,6 @@ class FabricMountingManager final {
allocatedViewRegistry_{};
std::recursive_mutex allocatedViewsMutex_;

const bool reduceDeleteCreateMutation_{false};

jni::local_ref<jobject> getProps(
const ShadowView& oldShadowView,
const ShadowView& newShadowView);
Expand Down