Uh oh!
There was an error while loading. Please reload this page.
Fix nullable float props crashing ViewManagers on Android - #58038
Closed
dennytosp wants to merge 1 commit into
Closed
Fix nullable float props crashing ViewManagers on Android#58038dennytosp wants to merge 1 commit into
dennytosp wants to merge 1 commit into
Conversation
A codegen spec that declares an optional float with `WithDefault<CodegenTypes.Float, null>` makes codegen emit a `@Nullable Float` setter on the generated ViewManager interface. If the ViewManager implements that setter with `@ReactProp`, the app crashes during startup with: RuntimeException: Unrecognized type: class java.lang.Float for method: MyNativeViewManager#setTestFloatNullable `ViewManagersPropertyCache.createPropSetter` maps `java.lang.Boolean` and `java.lang.Integer` to boxed prop setters, but has no branch for `java.lang.Float`, so it falls through to the `else` and throws. Those three plus `@Nullable Integer` for colors are the only boxed types codegen can emit, which makes Float the single gap. Add the missing `BoxedFloatPropSetter`, mirroring `BoxedIntPropSetter`: unbox the `Double` that every JS number arrives as, and pass `null` through untouched. `DoubleTypeAnnotation` needs no equivalent because codegen always emits it as a primitive `double`. Fixesreact#55350
@javache has imported this pull request. If you are a Meta employee, you can view this in D116928062. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary:
Fixes#55350.
A codegen'd Fabric component that declares an optional float prop:
crashes the app at startup as soon as the
ViewManagerimplements it with a@ReactPropannotation:WithDefault<Float, null>is the only way to express a genuinely nullable float in a codegen spec — a plain optionalCodegenTypes.Floatis emitted as a primitivefloatdefaulting to0f. So codegen generates a@Nullable Floatsetter in the ViewManager interface, and then the runtime refuses to bind it.Root cause
GeneratePropsJavaInterfaceemits exactly four boxed (nullable) prop types:ViewManagersPropertyCacheBooleanTypeAnnotationwithdefault: null@Nullable BooleanBoxedBooleanPropSetterReservedPropTypeAnnotation/ColorPrimitive@Nullable IntegerBoxedColorPropSetterInt32EnumTypeAnnotation@Nullable IntegerBoxedIntPropSetterFloatTypeAnnotationwithdefault: null@Nullable FloatelsecreatePropSettermapsjava.lang.Booleanandjava.lang.Integer, but has no branch forjava.lang.Float, so it hits theelseand throws. This adds the missingBoxedFloatPropSetter, mirroringBoxedIntPropSetter: unboxDouble(all JS numbers arrive asDouble) toFloat, and passnullstraight through so the setter actually receives the absent value.DoubleTypeAnnotationis deliberately not covered — codegen always emits it as a primitivedouble, never boxed, so there is no nullable-double setter to bind.Note this only affects ViewManagers that use
@ReactProp. A codegen'd ViewManager that routes props through its generated delegate never reachesViewManagersPropertyCache, which is why the crash looks intermittent — the annotation is what pulls inFallbackViewManagerSetter.Changelog:
[ANDROID] [FIXED] - Fix
RuntimeException: Unrecognized type: class java.lang.Floatwhen aViewManagerimplements a nullable float prop (WithDefault<Float, null>) with@ReactPropTest Plan:
Added
testBoxedFloatSettertoReactPropAnnotationSetterTest, alongside the existingtestBoxedBooleanSetter/testBoxedIntSetter, plus aboxedFloatPropon the ViewManager under test. It drivesviewManager.updateProperties(...), which is the sameFallbackViewManagerSetter→getNativePropSettersForViewManagerClass→createPropSetterpath as the crash stack trace, and asserts the setter receives3.5f,-7.0fandnull.Reverting only
ViewManagersPropertyCache.ktand keeping the new test reproduces the reported crash verbatim:All 18 fail rather than just the new one, and that is the bug's real shape:
getNativePropSettersForViewManagerClassbuilds the setter map for the entireViewManager class in one pass and caches it, so a single unbindable prop takes
down every other prop on that manager. That is why the reported crash happens at
startup, while
getNativeProps()is collecting view manager constants, ratherthan when the offending prop is first set.
Kotlin formatting:
ViewManagersPropertyCacheis aninternal objectandBoxedFloatPropSetterisa
private class, soReactAndroid.apiis unchanged.