Skip to content

Commit e18e93a

Browse files
fix(android): delete enableAccumulatedUpdatesInRawPropsAndroid and make accumulated rawProps the default
Supersedes the earlier Props.cpp patch approach. The flag's expectedReleaseValue was already true and the non-accumulated code path has a latent correctness bug on view-flattening un-flatten (shadow node's rawProps gets overwritten with the latest JS patch only, so the CREATE mount item ships a partial prop set and newly- created native Views lose props like borderRadius). See facebook#52415. This removes the flag entirely and promotes the accumulated behavior to be the only behavior: - Props::initializeDynamicProps unconditionally merges previous rawProps with the incoming patch. - FabricMountingManager::getProps: non-Props-2.0 path now always emits full rawProps for CREATE and diffDynamicProps() for UPDATE. - FabricMountingManager executeMount INSERT: always emits UpdatePropsMountItem (dropped the shouldCreateView gate that only ran under the old flag-off branch). - FabricUIManagerBinding: schedulerDidFinishTransaction is a no-op (transactions are pulled only in schedulerShouldRenderTransactions); the pendingTransactions_ queue and its mutex are deleted with it. - ConcreteComponentDescriptor: enableExclusivePropsUpdateAndroid no longer pivots on the accumulated flag. - Regenerated all feature-flag accessor files (Kotlin + C++) via `yarn featureflags --update` to drop the API surface.
1 parent f88a214 commit e18e93a

25 files changed

Lines changed: 121 additions & 316 deletions

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlags.kt

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* This source code is licensed under the MIT license found in the
55
* LICENSE file in the root directory of this source tree.
66
*
7-
* @generated SignedSource<<82da59cbfb06937bba284ff1df2c64d6>>
7+
* @generated SignedSource<<497133038b4ac4b29ab8d172a82285fc>>
88
*/
99

1010
/**
@@ -96,12 +96,6 @@ public object ReactNativeFeatureFlags {
9696
@JvmStatic
9797
public fun enableAccessibilityOrder(): Boolean = accessor.enableAccessibilityOrder()
9898

99-
/**
100-
* When enabled, Android will accumulate updates in rawProps to reduce the number of mounting instructions for cascading re-renders.
101-
*/
102-
@JvmStatic
103-
public fun enableAccumulatedUpdatesInRawPropsAndroid(): Boolean = accessor.enableAccumulatedUpdatesInRawPropsAndroid()
104-
10599
/**
106100
* Enables various optimizations throughout the path of measuring text on Android.
107101
*/

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsCxxAccessor.kt

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* This source code is licensed under the MIT license found in the
55
* LICENSE file in the root directory of this source tree.
66
*
7-
* @generated SignedSource<<b6d6b8aae3449760d857e5dd52588b03>>
7+
* @generated SignedSource<<50a1a0eeaef6bb9f8a7f7491bad8f24d>>
88
*/
99

1010
/**
@@ -31,7 +31,6 @@ internal class ReactNativeFeatureFlagsCxxAccessor : ReactNativeFeatureFlagsAcces
3131
private var disableTextLayoutManagerCacheAndroidCache: Boolean? = null
3232
private var disableViewPreallocationAndroidCache: Boolean? = null
3333
private var enableAccessibilityOrderCache: Boolean? = null
34-
private var enableAccumulatedUpdatesInRawPropsAndroidCache: Boolean? = null
3534
private var enableAndroidTextMeasurementOptimizationsCache: Boolean? = null
3635
private var enableBridgelessArchitectureCache: Boolean? = null
3736
private var enableCppPropsIteratorSetterCache: Boolean? = null
@@ -212,15 +211,6 @@ internal class ReactNativeFeatureFlagsCxxAccessor : ReactNativeFeatureFlagsAcces
212211
return cached
213212
}
214213

215-
override fun enableAccumulatedUpdatesInRawPropsAndroid(): Boolean {
216-
var cached = enableAccumulatedUpdatesInRawPropsAndroidCache
217-
if (cached == null) {
218-
cached = ReactNativeFeatureFlagsCxxInterop.enableAccumulatedUpdatesInRawPropsAndroid()
219-
enableAccumulatedUpdatesInRawPropsAndroidCache = cached
220-
}
221-
return cached
222-
}
223-
224214
override fun enableAndroidTextMeasurementOptimizations(): Boolean {
225215
var cached = enableAndroidTextMeasurementOptimizationsCache
226216
if (cached == null) {

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsCxxInterop.kt

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* This source code is licensed under the MIT license found in the
55
* LICENSE file in the root directory of this source tree.
66
*
7-
* @generated SignedSource<<7ca56a7b519e6e1619931447dcc1672a>>
7+
* @generated SignedSource<<f7d31a1306d76e08aa21774c29e6e53d>>
88
*/
99

1010
/**
@@ -50,8 +50,6 @@ public object ReactNativeFeatureFlagsCxxInterop {
5050

5151
@DoNotStrip @JvmStatic public external fun enableAccessibilityOrder(): Boolean
5252

53-
@DoNotStrip @JvmStatic public external fun enableAccumulatedUpdatesInRawPropsAndroid(): Boolean
54-
5553
@DoNotStrip @JvmStatic public external fun enableAndroidTextMeasurementOptimizations(): Boolean
5654

5755
@DoNotStrip @JvmStatic public external fun enableBridgelessArchitecture(): Boolean

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsDefaults.kt

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* This source code is licensed under the MIT license found in the
55
* LICENSE file in the root directory of this source tree.
66
*
7-
* @generated SignedSource<<fe424de3f03b5f2ee20b83896eef01b1>>
7+
* @generated SignedSource<<d4a3d675fe5516c39670052b2de41175>>
88
*/
99

1010
/**
@@ -45,8 +45,6 @@ public open class ReactNativeFeatureFlagsDefaults : ReactNativeFeatureFlagsProvi
4545

4646
override fun enableAccessibilityOrder(): Boolean = false
4747

48-
override fun enableAccumulatedUpdatesInRawPropsAndroid(): Boolean = false
49-
5048
override fun enableAndroidTextMeasurementOptimizations(): Boolean = false
5149

5250
override fun enableBridgelessArchitecture(): Boolean = false

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsLocalAccessor.kt

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* This source code is licensed under the MIT license found in the
55
* LICENSE file in the root directory of this source tree.
66
*
7-
* @generated SignedSource<<0de759cf10c9d77c9938f9dac257aada>>
7+
* @generated SignedSource<<5585df6f3056f8f15ed00e58f521865a>>
88
*/
99

1010
/**
@@ -35,7 +35,6 @@ internal class ReactNativeFeatureFlagsLocalAccessor : ReactNativeFeatureFlagsAcc
3535
private var disableTextLayoutManagerCacheAndroidCache: Boolean? = null
3636
private var disableViewPreallocationAndroidCache: Boolean? = null
3737
private var enableAccessibilityOrderCache: Boolean? = null
38-
private var enableAccumulatedUpdatesInRawPropsAndroidCache: Boolean? = null
3938
private var enableAndroidTextMeasurementOptimizationsCache: Boolean? = null
4039
private var enableBridgelessArchitectureCache: Boolean? = null
4140
private var enableCppPropsIteratorSetterCache: Boolean? = null
@@ -227,16 +226,6 @@ internal class ReactNativeFeatureFlagsLocalAccessor : ReactNativeFeatureFlagsAcc
227226
return cached
228227
}
229228

230-
override fun enableAccumulatedUpdatesInRawPropsAndroid(): Boolean {
231-
var cached = enableAccumulatedUpdatesInRawPropsAndroidCache
232-
if (cached == null) {
233-
cached = currentProvider.enableAccumulatedUpdatesInRawPropsAndroid()
234-
accessedFeatureFlags.add("enableAccumulatedUpdatesInRawPropsAndroid")
235-
enableAccumulatedUpdatesInRawPropsAndroidCache = cached
236-
}
237-
return cached
238-
}
239-
240229
override fun enableAndroidTextMeasurementOptimizations(): Boolean {
241230
var cached = enableAndroidTextMeasurementOptimizationsCache
242231
if (cached == null) {

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsProvider.kt

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* This source code is licensed under the MIT license found in the
55
* LICENSE file in the root directory of this source tree.
66
*
7-
* @generated SignedSource<<dd7ae1c5f86fa273402e5b64a8636528>>
7+
* @generated SignedSource<<d91e9cbbbb0ab6f2e88b21db315dfa04>>
88
*/
99

1010
/**
@@ -45,8 +45,6 @@ public interface ReactNativeFeatureFlagsProvider {
4545

4646
@DoNotStrip public fun enableAccessibilityOrder(): Boolean
4747

48-
@DoNotStrip public fun enableAccumulatedUpdatesInRawPropsAndroid(): Boolean
49-
5048
@DoNotStrip public fun enableAndroidTextMeasurementOptimizations(): Boolean
5149

5250
@DoNotStrip public fun enableBridgelessArchitecture(): Boolean

packages/react-native/ReactAndroid/src/main/jni/react/fabric/FabricMountingManager.cpp

Lines changed: 12 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -339,15 +339,11 @@ jni::local_ref<jobject> getProps(
339339

340340
return ReadableNativeMap::newObjectCxxArgs(std::move(diff));
341341
}
342-
if (ReactNativeFeatureFlags::enableAccumulatedUpdatesInRawPropsAndroid()) {
343-
if (oldProps == nullptr) {
344-
return ReadableNativeMap::newObjectCxxArgs(newProps->rawProps);
345-
} else {
346-
return ReadableNativeMap::newObjectCxxArgs(
347-
diffDynamicProps(oldProps->rawProps, newProps->rawProps));
348-
}
342+
if (oldProps == nullptr) {
343+
return ReadableNativeMap::newObjectCxxArgs(newProps->rawProps);
349344
}
350-
return ReadableNativeMap::newObjectCxxArgs(newProps->rawProps);
345+
return ReadableNativeMap::newObjectCxxArgs(
346+
diffDynamicProps(oldProps->rawProps, newProps->rawProps));
351347
}
352348

353349
struct InstructionBuffer {
@@ -725,28 +721,15 @@ void FabricMountingManager::executeMount(
725721

726722
bool shouldCreateView =
727723
!allocatedViewTags.contains(newChildShadowView.tag);
728-
if (ReactNativeFeatureFlags::
729-
enableAccumulatedUpdatesInRawPropsAndroid()) {
730-
if (shouldCreateView) {
731-
LOG(ERROR) << "Emitting insert for unallocated view "
732-
<< newChildShadowView.tag;
733-
}
734-
(maintainMutationOrder ? cppCommonMountItems
735-
: cppUpdatePropsMountItems)
736-
.push_back(
737-
CppMountItem::UpdatePropsMountItem(
738-
{}, newChildShadowView));
739-
} else {
740-
if (shouldCreateView) {
741-
LOG(ERROR) << "Emitting insert for unallocated view "
742-
<< newChildShadowView.tag;
743-
(maintainMutationOrder ? cppCommonMountItems
744-
: cppUpdatePropsMountItems)
745-
.push_back(
746-
CppMountItem::UpdatePropsMountItem(
747-
{}, newChildShadowView));
748-
}
724+
if (shouldCreateView) {
725+
LOG(ERROR) << "Emitting insert for unallocated view "
726+
<< newChildShadowView.tag;
749727
}
728+
(maintainMutationOrder ? cppCommonMountItems
729+
: cppUpdatePropsMountItems)
730+
.push_back(
731+
CppMountItem::UpdatePropsMountItem(
732+
{}, newChildShadowView));
750733

751734
// State
752735
if (newChildShadowView.state) {

packages/react-native/ReactAndroid/src/main/jni/react/fabric/FabricUIManagerBinding.cpp

Lines changed: 8 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -620,35 +620,9 @@ FabricUIManagerBinding::getMountingManager(const char* locationHint) {
620620
}
621621

622622
void FabricUIManagerBinding::schedulerDidFinishTransaction(
623-
const std::shared_ptr<const MountingCoordinator>& mountingCoordinator) {
624-
if (ReactNativeFeatureFlags::enableAccumulatedUpdatesInRawPropsAndroid()) {
625-
// We don't do anything here. We will pull the transaction in
626-
// `schedulerShouldRenderTransactions`.
627-
} else {
628-
// We shouldn't be pulling the transaction here (which triggers diffing of
629-
// the trees to determine the mutations to run on the host platform),
630-
// but we have to due to current limitations in the Android implementation.
631-
auto mountingTransaction = mountingCoordinator->pullTransaction(
632-
/* willPerformAsynchronously = */ true);
633-
if (!mountingTransaction.has_value()) {
634-
return;
635-
}
636-
637-
std::unique_lock<std::mutex> lock(pendingTransactionsMutex_);
638-
auto pendingTransaction = std::find_if(
639-
pendingTransactions_.begin(),
640-
pendingTransactions_.end(),
641-
[&](const auto& transaction) {
642-
return transaction.getSurfaceId() ==
643-
mountingTransaction->getSurfaceId();
644-
});
645-
646-
if (pendingTransaction != pendingTransactions_.end()) {
647-
pendingTransaction->mergeWith(std::move(*mountingTransaction));
648-
} else {
649-
pendingTransactions_.push_back(std::move(*mountingTransaction));
650-
}
651-
}
623+
const std::shared_ptr<const MountingCoordinator>& /*mountingCoordinator*/) {
624+
// We don't do anything here. We will pull the transaction in
625+
// `schedulerShouldRenderTransactions`.
652626
}
653627

654628
void FabricUIManagerBinding::schedulerShouldRenderTransactions(
@@ -671,33 +645,11 @@ void FabricUIManagerBinding::schedulerShouldRenderTransactions(
671645
}
672646
}
673647

674-
if (ReactNativeFeatureFlags::enableAccumulatedUpdatesInRawPropsAndroid()) {
675-
auto mountingTransaction = mountingCoordinator->pullTransaction(
676-
/* willPerformAsynchronously = */ true);
677-
if (mountingTransaction.has_value()) {
678-
auto transaction = std::move(*mountingTransaction);
679-
mountingManager->executeMount(transaction);
680-
}
681-
} else {
682-
std::vector<MountingTransaction> pendingTransactions;
683-
684-
{
685-
// Retain the lock to access the pending transactions but not to execute
686-
// the mount operations because that method can call into this method
687-
// again.
688-
//
689-
// This can be re-entrant when mounting manager triggers state updates
690-
// synchronously (this can happen when committing from the UI thread).
691-
// This is safe because we're already combining all the transactions for
692-
// the same surface ID in a single transaction in the pending transactions
693-
// list, so operations won't run out of order.
694-
std::unique_lock<std::mutex> lock(pendingTransactionsMutex_);
695-
pendingTransactions_.swap(pendingTransactions);
696-
}
697-
698-
for (auto& transaction : pendingTransactions) {
699-
mountingManager->executeMount(transaction);
700-
}
648+
auto mountingTransaction = mountingCoordinator->pullTransaction(
649+
/* willPerformAsynchronously = */ true);
650+
if (mountingTransaction.has_value()) {
651+
auto transaction = std::move(*mountingTransaction);
652+
mountingManager->executeMount(transaction);
701653
}
702654
}
703655

packages/react-native/ReactAndroid/src/main/jni/react/fabric/FabricUIManagerBinding.h

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -159,10 +159,6 @@ class FabricUIManagerBinding : public jni::HybridClass<FabricUIManagerBinding>,
159159
surfaceHandlerRegistry_{};
160160
std::shared_mutex surfaceHandlerRegistryMutex_; // Protects `surfaceHandlerRegistry_`.
161161

162-
// Track pending transactions, one per surfaceId
163-
std::mutex pendingTransactionsMutex_;
164-
std::vector<MountingTransaction> pendingTransactions_;
165-
166162
float pointScaleFactor_ = 1;
167163

168164
bool enableFabricLogs_{false};

packages/react-native/ReactAndroid/src/main/jni/react/featureflags/JReactNativeFeatureFlagsCxxInterop.cpp

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* This source code is licensed under the MIT license found in the
55
* LICENSE file in the root directory of this source tree.
66
*
7-
* @generated SignedSource<<411aef7b1b977407b52e4f23e95db0a5>>
7+
* @generated SignedSource<<4b634360db1cb90223867041ea754fdc>>
88
*/
99

1010
/**
@@ -105,12 +105,6 @@ class ReactNativeFeatureFlagsJavaProvider
105105
return method(javaProvider_);
106106
}
107107

108-
bool enableAccumulatedUpdatesInRawPropsAndroid() override {
109-
static const auto method =
110-
getReactNativeFeatureFlagsProviderJavaClass()->getMethod<jboolean()>("enableAccumulatedUpdatesInRawPropsAndroid");
111-
return method(javaProvider_);
112-
}
113-
114108
bool enableAndroidTextMeasurementOptimizations() override {
115109
static const auto method =
116110
getReactNativeFeatureFlagsProviderJavaClass()->getMethod<jboolean()>("enableAndroidTextMeasurementOptimizations");
@@ -650,11 +644,6 @@ bool JReactNativeFeatureFlagsCxxInterop::enableAccessibilityOrder(
650644
return ReactNativeFeatureFlags::enableAccessibilityOrder();
651645
}
652646

653-
bool JReactNativeFeatureFlagsCxxInterop::enableAccumulatedUpdatesInRawPropsAndroid(
654-
facebook::jni::alias_ref<JReactNativeFeatureFlagsCxxInterop> /*unused*/) {
655-
return ReactNativeFeatureFlags::enableAccumulatedUpdatesInRawPropsAndroid();
656-
}
657-
658647
bool JReactNativeFeatureFlagsCxxInterop::enableAndroidTextMeasurementOptimizations(
659648
facebook::jni::alias_ref<JReactNativeFeatureFlagsCxxInterop> /*unused*/) {
660649
return ReactNativeFeatureFlags::enableAndroidTextMeasurementOptimizations();
@@ -1119,9 +1108,6 @@ void JReactNativeFeatureFlagsCxxInterop::registerNatives() {
11191108
makeNativeMethod(
11201109
"enableAccessibilityOrder",
11211110
JReactNativeFeatureFlagsCxxInterop::enableAccessibilityOrder),
1122-
makeNativeMethod(
1123-
"enableAccumulatedUpdatesInRawPropsAndroid",
1124-
JReactNativeFeatureFlagsCxxInterop::enableAccumulatedUpdatesInRawPropsAndroid),
11251111
makeNativeMethod(
11261112
"enableAndroidTextMeasurementOptimizations",
11271113
JReactNativeFeatureFlagsCxxInterop::enableAndroidTextMeasurementOptimizations),

0 commit comments

Comments
 (0)