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).
mfazekas
added a commit
that referenced
this pull request
Jul 17, 2026
…nds (#347) Adds an optional `frameRate` view prop (issue #332): a number caps the render loop at that many frames per second, and a `{ minimum, maximum, preferred? }` range maps to `CAFrameRateRange` on iOS; Android honors a range best-effort as a cap at `preferred ?? maximum`. Capping limits frame production, not animation time — playback still advances by the real elapsed time, so wall-clock speed is unchanged. iOS forwards the prop to `RiveUIView.frameRate` (https://rive.app/docs/runtimes/apple/apple#frame-rate). Android's new runtime has no public per-view API (`RiveFramePacer` is internal and only drives the Compose loop), so our Choreographer loop skips vsyncs itself — same math as upstream's pacer — and on API 35+ also applies the cap as the platform `requestedFrameRate` hint. While paused, the Android loop now only draws when content changed (initial frame, resize, rebinding) instead of re-rendering identical frames every vsync. The legacy backends accept the prop and ignore it. Also includes the null-prop postprocess fix from #326 (clearing `frameRate` back to undefined throws without it); dedupes trivially with whichever lands first. Before/after on a looping animation (the Frame Rate Cap example page added here), app process CPU on a Pixel 6 emulator (API 34, % of one core, 12s windows, cold start, idle host): | frameRate | before | after | |---|---|---| | undefined | 37% | 36% | | 30 | no effect | 17% | | 15 | no effect | 9% | | 5 | 37% | 6% | | paused | 36% | 3% | The paused row is the paused-draw fix: before, a paused view kept rendering identical frames at the display refresh rate. Effective frame production was verified independently: `dumpsys gfxinfo` counts 285/151/75/25 frames per 5s for uncapped/30/15/5 and 0 while paused; on the iOS simulator, unique frames in a screen recording measured ~58 fps uncapped, 5 fps at `frameRate={5}` and 15 fps for `{minimum: 10, maximum: 20, preferred: 15}`. Earlier real-device measurements of the same cap logic (OnePlus Dimensity 8350, debug build): uncapped 133% of a core, cap=30 111%, cap=15 68%, paused 14% ≈ the app's cost with Rive unmounted (13%).
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.