fix: don't throw when an optional RiveView prop is cleared back to undefined#326
Open
mfazekas wants to merge 2 commits into
Open
fix: don't throw when an optional RiveView prop is cleared back to undefined#326mfazekas wants to merge 2 commits into
mfazekas wants to merge 2 commits into
Conversation
…defined Fabric's prop diff sends null (not undefined) when a prop is removed, but Nitro's JSIConverter<std::optional<T>> only maps undefined to nullopt, so clearing any optional RiveView prop (artboardName, stateMachineName, autoPlay, alignment, fit, layoutScaleFactor, dataBind) threw e.g. "Exception in HostFunction: RiveView.artboardName: Value is null, expected a String" during commit. Affects both backends and both platforms since the generated prop parser is shared. Post-process the nitrogen output to treat null like undefined for optional props until mrousavy/nitro#1184 is fixed upstream. Adds an Optional Prop Clear reproducer page that sets and clears each optional prop (fails 7/7 unfixed, passes fixed on both Android backends).
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.
Setting an optional RiveView prop back to
undefined(e.g.artboardNamefrom a string to no value) threwException in HostFunction: RiveView.artboardName: Value is null, expected a Stringduring React commit. Fabric's prop diff sendsnull(notundefined) when a prop is removed, and Nitro'sJSIConverter<std::optional<T>>only mapsundefinedtonullopt, so the inner converter throws. This affects all seven optional props (artboardName,stateMachineName,autoPlay,alignment,fit,layoutScaleFactor,dataBind) and both Android backends — the generated prop parser is shared C++, so iOS is affected the same way.The fix extends
scripts/nitrogen-postprocess.tsto patch the generatedHybridRiveViewComponent.cpp, treatingnulllikeundefinedat the optional-prop parse sites. This is a workaround until it's fixed upstream in Nitro (mrousavy/nitro#1184, open fix PR mrousavy/nitro#1189); the postprocess step already runs as part ofyarn nitrogen, so regeneration keeps the fix.Includes a reproducer page (
example/src/reproducers/OptionalPropClear.tsx) that sets each optional prop and then clears it: it failed 7/7 before the fix and passes 7/7 after, verified on the Android emulator with both the new and the legacy (USE_RIVE_LEGACY=true) backend.