Repository navigation
fix(views): accept null for a removed function prop - #1671
Open
lagudafuadtosin wants to merge 2 commits into
Open
lagudafuadtosin wants to merge 2 commits into
lagudafuadtosin wants to merge 2 commits into
Conversation
ReactProp::fromRawValue unwrapped function props with asObject() before
anything checked for null. When a view had a function prop on one render and
the next render omits it, React Native hands the prop over as null, so the
unwrap threw 'Value is null, expected an Object' from cloneNodeWithNewProps and
the commit failed. The optional converter that would have treated the value as
absent was never reached, and it only knew undefined anyway.
Only unwrap { f } when the value is an object, and let
JSIConverter<std::optional<T>> treat null like undefined in fromJSI and
canConvert. The generated setters already pass nullptr to the platform side
for an empty optional.
Seen through VisionCamera: <Camera onPreviewStarted={fn} /> followed by a
rerender without that prop fails the harness test 'reconfigures when the
Camera device position prop changes' on a Galaxy A14 (SM-A145F, Android 15).
With this change the test passes.
TestView gains an optional someOptionalCallback prop and a hasSomeOptionalCallback() method so the harness can check what the native view received. Kotlin and Swift test views implement both. Nitrogen output regenerated. The new test renders TestView with the callback, rerenders without it and expects hasSomeOptionalCallback() to be false. On a Galaxy A14 (SM-A145F, Android 15) it fails without the ReactProp and optional converter changes and passes with them, views harness 9/9.
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
This branch was previously deployed
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
ReactProp<T>::fromRawValueunwraps function props withasObject()before anything checks for null. When a view had a function prop on one render and the next render omits it, React Native hands the prop over asnull, soasObject()throwsValue is null, expected an ObjectfromcloneNodeWithNewPropsand the commit fails. The optional converter that would treat it as absent is never reached, and it only knewundefinedanyway.Fix, two places:
ReactProp::fromRawValueonly unwraps{ f }when the value is an object, andJSIConverter<std::optional<T>>treatsnulllikeundefinedinfromJSIandcanConvert. The generated setters already passnullptrto the platform side for an empty optional. The first change alone is not enough: the null then reaches the optional converter and the same error comes back fromJSIConverter<std::function>.Test:
accepts a function prop being removed on a later renderinapps/example/__tests__/nitro.views.harness.tsx. It needed an optional function prop onTestView, so the spec gainssomeOptionalCallback?: () => voidandhasSomeOptionalCallback(): boolean, implemented in the Kotlin and Swift test views, nitrogen output regenerated. On a Galaxy A14 (SM-A145F, Android 15) the views harness is 9/9 with the fix. Without it the new test fails: the rerender that drops the prop never reaches the native view, sohasSomeOptionalCallback()still reports true (expected true to be false).Seen first through VisionCamera:
<Camera onPreviewStarted={fn} />then a rerender without that prop fails its harness testreconfigures when the Camera device position prop changeswith the same error. With both changes it passes.