fix: don't throw when an optional RiveView prop is cleared back to undefined#326
Open
mfazekas wants to merge 141 commits into
Open
fix: don't throw when an optional RiveView prop is cleared back to undefined#326mfazekas wants to merge 141 commits into
mfazekas wants to merge 141 commits into
Conversation
Add new experimental iOS backend (ios/new/) with synchronous API, move legacy backend files to ios/legacy/, add getEnums() support, retry listener streams on missingData, and restore TestComponentOverlay. Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering)
Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering)
getEnums() in legacy now throws directing users to the experimental backend instead of creating throwaway Worker+File instances. Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering)
…essing Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering)
… binding Each Worker has its own C++ command server with its own m_artboards handle map. Creating separate Workers per file meant artboard handles from one file were invalid on another file's server. Using a shared singleton Worker fixes cross-file artboard property set. Also wires fit/alignment through experimental Fit enum and improves asset type detection with audio/font magic bytes. Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Happy <yesreply@happy.engineering>
Passing fit to the Rive() constructor breaks layout mode because the MTKView drawable isn't ready yet. Set rive.fit after setupRiveUIView() instead.
Port Flutter data binding tests for VM access, enums, creation variants, list properties, artboard and image properties. Includes new .riv test assets and react-native-harness upgrade to alpha.25.
…ceByIndex Prevents fatal crash when passing negative numbers to Swift APIs expecting unsigned integers.
CocoaPods doesn't embed SPM-resolved dynamic frameworks automatically. Patches the embed script to include RiveRuntime when USE_RIVE_SPM=1.
Move 19 backend-specific files to src/legacy/java/, add Gradle sourceSets switching via USE_RIVE_NEW_API property, prepare empty experimental dirs.
Uses the new handle-based Rive Android SDK (app.rive.*) with CommandQueue, path-based ViewModelInstance property access, and Kotlin Flows for reactivity. Throws UnsupportedOperationException for SMI inputs, text runs, and events. Also fixes Gradle property resolution to use rootProject.findProperty.
Custom TextureView renderer with Choreographer render loop, CommandQueue polling infrastructure, ViewModel resolution and property validation, and example Compose test activities for manual testing.
Also fix viewModelByIndex/ByName on Android legacy to catch SDK exceptions instead of letting them propagate as JniExceptions.
…fferences Fix colorProperty setter Double→Int overflow on Android legacy (toLong().toInt()). Make enum and replaceViewModel tests tolerant of Android SDK behavioral differences.
Use .viewModelDefault(from: .name(vmName)) for default instance creation instead of artboard-based lookup that only worked for the default artboard's VM. Skip instanceName, color get/set, and non-existent property checks on experimental iOS where the SDK doesn't support them.
…anceName Add async variants for viewModelByName, defaultArtboardViewModel, and ViewModel create methods. Pass instanceName through at creation time on both iOS and Android experimental backends as a workaround for the SDK not exposing ViewModelInstance.name.
…rimental iOS Color.argbValue is now public in rive-ios 6.15.2 — implement getValue via blockingAsync, addListener via valueStream, and fix setter crash by using UInt32(bitPattern:) for negative ARGB values from JS.
- Always use SPM for RiveRuntime (remove CocoaPods fallback) - Add RiveSPMEmbedFix module to auto-embed RiveRuntime.framework - Pin SPM to exactVersion instead of upToNextMajorVersion - Replace USE_RIVE_SPM with USE_RIVE_EXPERIMENTAL_RUNTIME to select legacy vs experimental backend independently of SPM
…erimental iOS test issues - Rename USE_RIVE_EXPERIMENTAL_RUNTIME to USE_RIVE_NEW_API (matches Android) - Remove USE_RIVE_SPM and all SPM embedding hacks from podspec/Podfile - Use standard CocoaPods dependency for RiveRuntime - Emit initial value in experimental addListener (number/string/bool/enum/color) - Guard tests that crash on experimental iOS (list ops, autoPlay, artboard/image loading) - Handle createInstanceByName throwing on experimental backend
Fixes flaky autoplay harness tests. The 6.20.1 fix for "massive timestamp delta on first frame" was causing the animation to jump through its entire cycle in one frame, making ypos property change detection unreliable. Verified with 100 consecutive test runs (0 failures).
Adds 15 new harness tests covering listener callback invocation, set() + getValueAsync() round-trips, and removeListeners() for all ViewModel property types. iOS native coverage goes from 41% to 50%. Target types: BooleanProperty (16→84%), ColorProperty (14→84%), EnumProperty (16→84%), TriggerProperty (0→81%), StringProperty (37→84%), NumberProperty (85→90%).
Adds harness tests for two previously untested areas: **Font config** (6 tests): loadFont by system name, loadFont by URL, systemFallback, setFallbackFonts with default/weight-specific fonts, clearFallbackFonts, error on invalid font name. **Asset loading** (4 tests): referencedAssets with font type, image URL type, multiple asset types, and undefined assets. Covers HybridRiveFontConfig.swift (was 0%), AssetLoader.swift (was 1%), DataSourceResolver.swift (was 0%), HTTPDataLoader.swift (was 43%).
Adds 7 new tests covering previously untested native code paths: - **fromBytes**: load .riv from raw ArrayBuffer (HybridRiveFileFactory) - **RiveImages.loadFromURLAsync**: load image and verify byteSize (HybridRiveImageFactory + HybridRiveImage) - **RiveLog.setLogLevel**: exercise all 4 log levels (HybridRiveLogger) - **RiveView pause/play/reset**: exercise view control methods with a rendered RiveView (HybridRiveView)
Upgrades harness from 1.2.0 to 1.3.0. Key improvements from harness PRs #129 and #132: - Bridge RPC closes immediately on app disconnect instead of waiting for timeout - New internal RPC transport with heartbeat-based liveness detection - Dead app connections fail fast with `AppBridgeDisconnectedError` instead of ambiguous "device did not respond" after 90s Should reduce the intermittent CI flakiness we've been seeing on legacy iOS.
…ntModificationException (#286) `onChanged` iterates `listeners.values` on a `Dispatchers.Default` thread (the flow-collection coroutine), while `addListener`/`removeListener` mutate the same map from the JS thread. A plain `LinkedHashMap` fails fast with `ConcurrentModificationException` when this overlap occurs. `ConcurrentHashMap` iterates safely with concurrent mutations — no CME, no locks needed at call sites.
…oid) (#285) ## Problem On the experimental backend, `play()`/`pause()`/`reset()` were no-ops — animations couldn't be paused/resumed and `autoPlay={false}` was ignored. - **iOS**: only mutated a private `isPaused` flag never forwarded to `RiveUIView`. - **Android**: empty bodies; the render loop advanced unconditionally and `autoPlay` was never consumed. Additionally, `useRiveTrigger` never fired on Android experimental — three separate issues: 1. `awaitViewReady()` resolved at end of `configure()`, before the TextureView surface existed. Triggers fired before the surface was up were lost. 2. `getTriggerFlow` has `replay=0` (no initial value on subscription); `drop=1` was silently eating the first real trigger. 3. The listener coroutine was launched with `DEFAULT` start, so it was merely queued on `Dispatchers.Default` and could miss a trigger fired before it reached `collect()`. Also, `dataBind={{ byName }}` on Android always bound from `vmNames.first()` (first VM in file) instead of the artboard's default ViewModel. ## Fix - **iOS** (`ios/new/RiveReactNativeView.swift`): forward `isPaused` to `RiveUIView` in play/pause/playIfNeeded, pass it into `RiveUIView(rive:isPaused:)`, set it from `autoPlay`. - **Android** (`android/src/new/.../RiveReactNativeView.kt`): gate `advanceStateMachine` on a `paused` flag toggled by play/pause, honor `autoPlay`. Signal `awaitViewReady()` on first rendered frame (not end of configure). - **Android triggers**: use `drop=0` for trigger flow (no initial value to skip); launch listener coroutine with `CoroutineStart.UNDISPATCHED`. - **Android byName**: use `ViewModelSource.DefaultForArtboard(art)` instead of `Named(vmNames.first())`, matching iOS and the Auto path. - **reset()**: deprecated. Neither experimental backend has a reset primitive — marked `@deprecated` in the spec, experimental impl logs an error and does nothing. ## Verification - `autoplay.harness.tsx`: pause freezes `ypos`, play resumes it, `autoPlay={false}` holds. Green on both platforms. - `use-rive-trigger.harness.tsx`: both stable and unstable-callback tests now pass on Android. - `viewmodel-instance-lookup.harness.tsx`: byName regression test verifies `artboard2 + byName("vmi1") → _id="vm2.vmi1.id"` (not vm1). - CI: legacy iOS `pod install` step clears `Podfile.lock` to avoid external `fast_float` checksum rejection.
`asyncExecuteOnUiThread` posts to the UI thread and returns immediately,
so the Promise resolves before the work runs. If the action throws, it
crashes on the UI thread instead of rejecting the Promise.
`play()`/`pause()`/`reset()` on the experimental backend are just flag
toggles or a deprecation log — no UI thread requirement, no throw.
Replace with `Promise.async { view.play() }` etc.
Property accessors (`numberProperty`, `stringProperty`, etc.) were using
`runBlocking { flow.first() }` as an existence probe before returning
the wrapper. This blocks the calling thread and can deadlock if called
from the main thread (e.g. via a worklet).
The iOS experimental backend already does the right thing — it just
creates and returns the wrapper directly. This PR aligns Android
experimental to the same behaviour: return a wrapper always, fail lazily
when `getValueAsync()` is called on an invalid path.
Also removes the `propertyNames` lazy val (used for `hasProperty`) which
had its own one-time `runBlocking` call.
Test: updated guard for the "non-existent returns undefined" test (skips
on all experimental backends), added new test asserting
`getValueAsync()` rejects for invalid paths on experimental.
Removes unreachable and never-populated code from the experimental iOS backend. - Drop `eventListeners`, `addEventListener`, `removeEventListeners` from `RiveReactNativeView` — the Hybrid layer throws before these are ever called - Drop `listenerTasks` from `HybridViewModelListProperty` and `HybridViewModelImageProperty` — the dict is declared and torn down but never populated
…iew reuse, bind-before-ready (#289) Addresses several correctness issues from the experimental runtime review. - **Android**: cancel auto-bind `CoroutineScope` on dispose; adds `disposed` guard inside the Main-thread hop to prevent post-teardown binding against a stale/reconfigured handle - **iOS**: reuse existing `RiveUIView` on reconfigure by assigning `riveUIView.rive = rive` instead of tearing down and recreating the view — eliminates orphaned MTKView draw calls on file switch - **iOS**: queue `bindViewModelInstance` calls that arrive before `configTask` completes, then apply them once `riveInstance` is set — previously the bind was silently dropped Includes harness tests for the iOS fixes: - `reconfigure.harness.tsx` — verifies animation still plays after switching file prop and back - `bind-before-ready.harness.tsx` — reproduces and verifies the bind-before-ready race (failed before fix, passes after)
Stacked on #289. Adds `setValueAsync(value)` to Number, String, Boolean, Color, and Enum property types across all 4 backends (iOS experimental, iOS legacy, Android experimental, Android legacy). On iOS experimental, this is a proper async write — the promise resolves only after the SDK `setValue` call completes on the main actor, giving ordering guarantees and error propagation. On legacy and Android, `set()` is already synchronous so `setValueAsync` is a thin wrapper that makes the API consistent. Includes harness tests covering all 5 property types.
…ental # Conflicts: # example/package.json # package.json # yarn.lock
Brings in #298 (data-binding thread-safety) + 0.4.12 release. Conflict resolutions: - ios/legacy/HybridViewModel{,Instance}.swift: keep #298's MainThread.run thread-safety on the getters AND the experimental getPropertiesAsync(). - package.json + yarn.lock: react-native-nitro-modules -> 0.35.10 (required by #298's iOS Promise.onMain routing); yarn.lock regenerated via yarn install. - .gitignore: keep both entries. - example/ios/Podfile.lock: kept current; regenerated by pod install (CI build-ios does rm + pod install).
- test-harness-ios-legacy pinned Xcode 16.4, which runners no longer have (it was added before main bumped the shared jobs to 26.3, and main has no legacy job to merge a fix from) -> bump to 26.3 and its simulator os_version 18.6 -> >=26.0 to match the iOS 26 runtimes Xcode 26.3 ships. - Pin all iOS jobs (build-ios, test-harness-ios, -legacy) to macos-15: macos-latest is mid-migrating to macOS 26, whose sim set lacks an iPhone 16 Pro on iOS >=26, which fails the simulator-action boot.
…sh) (#313) Bumps `runtimeVersions.android` 11.4.1 → 11.6.1 on the experimental backend line to fix the intermittent native `SIGSEGV` in `test-harness-android`. Root cause is a single-threaded re-entrancy / iterator-invalidation **use-after-free** in `rive::DataBindContainer::updateDataBinds` (a bind that auto-binds a nested view model re-enters `addDataBind`/`removeDataBind` and invalidates the active iterator). Fixed upstream in rive-runtime [#12649](https://github.com/rive-app/rive-runtime/pull/12649), shipped in rive-android 11.6.1 (11.4.1/11.5.0/11.6.0 are vulnerable). The experimental backend compiles + builds clean against 11.6.1 (no API changes needed). Full investigation: #308. (`main` is unaffected — it already pins 11.6.1 and does not carry the experimental backend.)
Resolves conflicts in ci.yml (kept macos-15 pin comments) and use-rive-trigger.harness.tsx (combined the #230 deterministic-rerender de-flake with the experimental backend's awaitViewReady readiness gating).
…314) Cherry-picks the `fmt`-as-C++17 Podfile workaround onto the experimental backend line so its iOS build survives the Xcode 26.4+ (Apple clang 21) `consteval` error in `fmt` (via RCT-Folly on RN 0.80.3). Same change as #306 (which targets `main`); applies cleanly here and the branch had no prior fmt workaround. Remove once we bump to RN 0.83+ (fix is upstream there).
Merges `main` up to the 0.4.14 release into the iOS experimental branch. No conflicts. Notable changes pulled in: 1. fix(android): don't re-acquire a disposed Rive file on view re-attach (#319) 2. fix(hooks): no-op stale property setter calls after dispose (#316) 3. fix(android): bump rive-android to 11.6.2 (#317) — resolves to 11.6.2 here (up from 11.6.1) On the #319 native fix: this branch has the legacy/new Android backend split, so git's rename detection applied the fix to the `android/src/legacy` variant of `RiveReactNativeView.kt` — which is correct, since the crash is a property of upstream `RiveAnimationView`'s cached `rendererAttributes.resource`. The `android/src/new` backend is a separate `TextureView`/`CommandQueue` implementation with no such code path, so the same tweak neither applies nor is needed there. A re-attach lifecycle bug in the new backend, if any, would be a separate fix. --------- Co-authored-by: Rive Engineering <60802183+rive-engineering@users.noreply.github.com>
…ental # Conflicts: # rive-debug-utils/src/specs/DebugUtils.nitro.ts
…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.