Repository navigation
Commit 091ac61
Fix DimensionValue props crashing ViewManagers on Android (#58039)
Summary:
A Fabric component whose spec declares a `DimensionValue` prop:
```js
type NativeProps = Readonly<{
...ViewProps,
marginBack?: DimensionValue,
}>;
```
crashes the app while React Native is collecting view manager constants, as soon
as the `ViewManager` implements that prop with `ReactProp`:
```
java.lang.RuntimeException: Unrecognized type: class com.facebook.yoga.YogaValue
for method: MyNativeViewManager#setMarginBack
at ViewManagersPropertyCache.createPropSetter(...)
at ViewManagerPropertyUpdater$FallbackViewManagerSetter.<init>(...)
at ViewManager.getNativeProps(...)
at UIManagerModuleConstantsHelper.internal_createConstantsForViewManager(...)
```
### Root cause
Codegen maps `DimensionValue` to the `DimensionPrimitive` reserved type and emits
a boxed `Nullable YogaValue` setter on the generated ViewManager interface — this
is covered by a committed snapshot:
```java
// GeneratePropsJavaInterface-test.js.snap
public interface DimensionPropNativeComponentManagerInterface<T extends View> extends ViewManagerWithGeneratedInterface {
void setMarginBack(T view, Nullable YogaValue value);
}
```
`ViewManagersPropertyCache.createPropSetter` has no branch for `YogaValue`, so it
falls through to the `else` and throws.
The conversion logic already exists — `DimensionPropConverter` handles exactly the
three shapes a `DimensionValue` arrives in (`null`, a `Double` in points, a `String`
like `"100%"`) and has its own unit tests. But it is only referenced by generated
delegate code (`GeneratePropsJavaDelegate.js`), never by the `ReactProp` path.
Compare `ColorPropConverter`, which is wired into *both* paths — that asymmetry is
the bug.
This adds the missing `DimensionPropSetter`, modelled on `ColorPropSetter`, which
delegates wholesale to the existing converter. It reports `mixed` as its prop type
because a dimension may be a number or a string, matching how colors are reported.
Note the blast radius: `getNativePropSettersForViewManagerClass` builds the setter
map for an entire ViewManager class in one pass, so a single unbindable prop takes
down every other prop on that manager, and `createConstantsForViewManager` reads
`viewManager.nativeProps` unconditionally. That is why this surfaces as a startup
crash rather than a failure when the prop is first set.
This is the same class of gap as #55350 (`Nullable Float`). With both fixed, every
Java type `GeneratePropsJavaInterface` can emit is bindable through `ReactProp`.
## Changelog:
[ANDROID] [FIXED] - Fix `RuntimeException: Unrecognized type: class com.facebook.yoga.YogaValue` when a `ViewManager` implements a `DimensionValue` prop with `ReactProp`
Pull Request resolved: #58039
Test Plan:
Added `testDimensionSetter` and `testFailToUpdateDimensionPropWithArray` to
`ReactPropAnnotationSetterTest`, plus a `dimensionProp` on the ViewManager under
test. They drive `viewManager.updateProperties(...)` — the same
`FallbackViewManagerSetter` -> `getNativePropSettersForViewManagerClass` ->
`createPropSetter` path as the crash — and assert the setter receives
`YogaValue(10.5f, POINT)` for a number, `YogaValue(100f, PERCENT)` for `"100%"`,
and `null` for `null`, and that an unsupported value still surfaces as a
`JSApplicationIllegalArgumentException` rather than escaping raw.
```
$ ./gradlew :packages:react-native:ReactAndroid:testDebugUnitTest \
--tests "com.facebook.react.uimanager.ReactPropAnnotationSetterTest"
BUILD SUCCESSFUL
tests=19 failures=0 errors=0
ok testDimensionSetter <- new
ok testFailToUpdateDimensionPropWithArray <- new
... (19/19)
```
Reverting only `ViewManagersPropertyCache.kt` and keeping the new tests reproduces
the crash:
```
tests=19 failures=19
java.lang.RuntimeException: Unrecognized type: class com.facebook.yoga.YogaValue
for method: ReactPropAnnotationSetterTest$ViewManagerUnderTest#setDimensionProp
```
All 19 fail rather than only the two new ones, for the reason described above: the
setter map is built for the whole ViewManager class at once.
Formatting — `./gradlew ktfmtFormat` leaves both changed files byte-identical.
`ViewManagersPropertyCache` is an `internal object` and `DimensionPropSetter` is a
`private class`, so `ReactAndroid.api` is unchanged.
Reviewed By: christophpurrer
Differential Revision: D116911179
Pulled By: javache
fbshipit-source-id: c9b91cc3086d01ce7594aefca1e3a673cd723af61 parent 8d23b14 commit 091ac61
2 files changed
Lines changed: 42 additions & 0 deletions
File tree
- packages/react-native/ReactAndroid/src
- main/java/com/facebook/react/uimanager
- test/java/com/facebook/react/uimanager
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
11 | 11 | | |
12 | 12 | | |
13 | 13 | | |
| 14 | + | |
14 | 15 | | |
15 | 16 | | |
16 | 17 | | |
17 | 18 | | |
18 | 19 | | |
19 | 20 | | |
20 | 21 | | |
| 22 | + | |
21 | 23 | | |
22 | 24 | | |
23 | 25 | | |
| |||
192 | 194 | | |
193 | 195 | | |
194 | 196 | | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
195 | 206 | | |
196 | 207 | | |
197 | 208 | | |
| |||
412 | 423 | | |
413 | 424 | | |
414 | 425 | | |
| 426 | + | |
415 | 427 | | |
416 | 428 | | |
417 | 429 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
16 | 16 | | |
17 | 17 | | |
18 | 18 | | |
| 19 | + | |
| 20 | + | |
19 | 21 | | |
20 | 22 | | |
21 | 23 | | |
| |||
43 | 45 | | |
44 | 46 | | |
45 | 47 | | |
| 48 | + | |
| 49 | + | |
46 | 50 | | |
47 | 51 | | |
48 | 52 | | |
| |||
135 | 139 | | |
136 | 140 | | |
137 | 141 | | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
138 | 147 | | |
139 | 148 | | |
140 | 149 | | |
| |||
337 | 346 | | |
338 | 347 | | |
339 | 348 | | |
| 349 | + | |
| 350 | + | |
| 351 | + | |
| 352 | + | |
| 353 | + | |
| 354 | + | |
| 355 | + | |
| 356 | + | |
| 357 | + | |
| 358 | + | |
| 359 | + | |
| 360 | + | |
| 361 | + | |
| 362 | + | |
| 363 | + | |
| 364 | + | |
| 365 | + | |
| 366 | + | |
| 367 | + | |
| 368 | + | |
| 369 | + | |
340 | 370 | | |
341 | 371 | | |
342 | 372 | | |
| |||
0 commit comments