Skip to content

Commit d25ff0d

Browse files
committed
Fix Android Fabric sync prop null override
1 parent eeb17ba commit d25ff0d

2 files changed

Lines changed: 65 additions & 5 deletions

File tree

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/SurfaceMountingManager.kt

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1332,7 +1332,10 @@ internal constructor(
13321332
for ((propKey, propValue) in patchMap) {
13331333
if (outputReadableMap.hasKey(propKey)) {
13341334
if (propKey == PROP_TRANSFORM) {
1335-
assert(outputReadableMap.getType(propKey) == ReadableType.Array && propValue is List<*>)
1335+
val outputType = outputReadableMap.getType(propKey)
1336+
assert(
1337+
(outputType == ReadableType.Array || outputType == ReadableType.Null) &&
1338+
propValue is List<*>)
13361339
val array = WritableNativeArray()
13371340
for (item in propValue as List<*>) {
13381341
if (item is Map<*, *>) {
@@ -1350,7 +1353,10 @@ internal constructor(
13501353
}
13511354
outputReadableMap.putArray(propKey, array)
13521355
} else if (propKey == PROP_OPACITY) {
1353-
assert(outputReadableMap.getType(propKey) == ReadableType.Number && propValue is Number)
1356+
val outputType = outputReadableMap.getType(propKey)
1357+
assert(
1358+
(outputType == ReadableType.Number || outputType == ReadableType.Null) &&
1359+
propValue is Number)
13541360
outputReadableMap.putDouble(propKey, (propValue as Number).toDouble())
13551361
}
13561362
}

packages/react-native/ReactAndroid/src/test/java/com/facebook/react/fabric/SurfaceMountingManagerSynchronousMountPropsTest.kt

Lines changed: 57 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,15 +10,17 @@
1010
package com.facebook.react.fabric
1111

1212
import com.facebook.react.ReactRootView
13+
import com.facebook.react.bridge.JavaOnlyArray
1314
import com.facebook.react.bridge.JavaOnlyMap
15+
import com.facebook.react.bridge.ReadableArray
1416
import com.facebook.react.bridge.ReactTestHelper
1517
import com.facebook.react.fabric.mounting.MountingManager
16-
import com.facebook.react.fabric.mounting.MountingManager.MountItemExecutor
1718
import com.facebook.react.fabric.mounting.SurfaceMountingManager
1819
import com.facebook.react.internal.featureflags.ReactNativeFeatureFlagsForTests
1920
import com.facebook.react.uimanager.ThemedReactContext
2021
import com.facebook.react.uimanager.ViewManager
2122
import com.facebook.react.uimanager.ViewManagerRegistry
23+
import com.facebook.react.views.view.ReactViewGroup
2224
import com.facebook.react.views.view.ReactViewManager
2325
import com.facebook.testutils.shadows.ShadowNativeLoader
2426
import com.facebook.testutils.shadows.ShadowNativeMap
@@ -67,8 +69,8 @@ class SurfaceMountingManagerSynchronousMountPropsTest {
6769
themedReactContext = ThemedReactContext(reactContext, reactContext, null, -1)
6870
mountingManager =
6971
MountingManager(
70-
ViewManagerRegistry(listOf<ViewManager<*, *>>(ReactViewManager())),
71-
MountItemExecutor {},
72+
ViewManagerRegistry(listOf<ViewManager<*, *>>(TestReactViewManager())),
73+
{},
7274
)
7375
}
7476

@@ -83,6 +85,25 @@ class SurfaceMountingManagerSynchronousMountPropsTest {
8385
smm.addViewAt(surfaceId, tag, 0)
8486
}
8587

88+
private class TestReactViewManager : ReactViewManager() {
89+
override fun setTransformProperty(
90+
view: ReactViewGroup,
91+
transforms: ReadableArray?,
92+
transformOrigin: ReadableArray?,
93+
) {
94+
view.translationY = 0.0f
95+
if (transforms == null) {
96+
return
97+
}
98+
for (i in 0..<transforms.size()) {
99+
val transform = transforms.getMap(i) ?: continue
100+
if (transform.hasKey("translateY")) {
101+
view.translationY = transform.getDouble("translateY").toFloat()
102+
}
103+
}
104+
}
105+
}
106+
86107
/** Stored synchronous opacity should override a stale Fabric mount update. */
87108
@Test
88109
fun storeSynchronousProps_overridesStaleOpacityInUpdateProps() {
@@ -137,6 +158,39 @@ class SurfaceMountingManagerSynchronousMountPropsTest {
137158
assertThat(smm.getView(tag).alpha).isEqualTo(0.2f)
138159
}
139160

161+
/** A null Fabric prop update should not clear the stored synchronous opacity override. */
162+
@Test
163+
fun updateProps_withNullOpacity_keepsStoredSynchronousProp() {
164+
val smm = startSurface()
165+
val tag = 42
166+
createAndAttachView(smm, tag)
167+
168+
smm.storeSynchronousMountPropsOverride(tag, JavaOnlyMap.of("opacity", 0.3))
169+
smm.updatePropsSynchronously(tag, JavaOnlyMap.of("opacity", 0.3))
170+
assertThat(smm.getView(tag).alpha).isEqualTo(0.3f)
171+
172+
smm.updateProps(tag, JavaOnlyMap.of("opacity", null))
173+
174+
assertThat(smm.getView(tag).alpha).isEqualTo(0.3f)
175+
}
176+
177+
/** A null Fabric prop update should not clear the stored synchronous transform override. */
178+
@Test
179+
fun updateProps_withNullTransform_keepsStoredSynchronousProp() {
180+
val smm = startSurface()
181+
val tag = 42
182+
val transform = JavaOnlyArray.of(JavaOnlyMap.of("translateY", 40.0))
183+
createAndAttachView(smm, tag)
184+
185+
smm.storeSynchronousMountPropsOverride(tag, JavaOnlyMap.of("transform", transform))
186+
smm.updatePropsSynchronously(tag, JavaOnlyMap.of("transform", transform))
187+
assertThat(smm.getView(tag).translationY).isEqualTo(40.0f)
188+
189+
smm.updateProps(tag, JavaOnlyMap.of("transform", null))
190+
191+
assertThat(smm.getView(tag).translationY).isEqualTo(40.0f)
192+
}
193+
140194
/**
141195
* When a view is deleted, stored synchronous props should be cleaned up. A recreated view with
142196
* the same tag should not be affected by the old stored props.

0 commit comments

Comments
 (0)