Skip to content

feat!: remove onStop callback in favor of data-binding triggers - #364

Merged
mfazekas merged 5 commits into
mainfrom
feat/databinding-finished-trigger
Aug 21, 2026
Merged

feat!: remove onStop callback in favor of data-binding triggers#364
mfazekas merged 5 commits into
mainfrom
feat/databinding-finished-trigger

Conversation

@mfazekas

@mfazekas mfazekas commented Aug 16, 2026

Copy link
Copy Markdown
Collaborator

Removes the onStop prop introduced in #353 (shipped only in v0.5.0-beta.1). Per the discussion with the Rive team, state-machine settling is intentionally internal — rive-android 11.8.0 already removed the public settledFlow this was built on — so app behavior shouldn't depend on it.

The replacement is a data-binding trigger fired by the state machine when the timeline completes (see this community file for the editor setup). Shipped here as a README section (including the exit-time-100% gotcha), a finished_trigger.riv asset with a "Finished Trigger" demo page in the example app, and a harness test asserting the trigger fires exactly once and only after the timeline actually plays — on all four backends, legacy included.

The Android render loop keeps the internal stop-advancing-when-settled optimization from #353; the iOS settled machinery is removed entirely (the SDK's own view already stops advancing at rest).

@mfazekas
mfazekas requested review from HayesGordon and a lite review from Copilot August 18, 2026 06:32
@mfazekas
mfazekas enabled auto-merge (squash) August 18, 2026 06:33

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR removes the previously introduced onStop callback API from RiveView (and its Nitro/native plumbing) and replaces the recommended “animation finished” workflow with a data-binding trigger emitted by the state machine on timeline completion, backed by an example demo and harness test coverage.

Changes:

  • Remove onStop from the public TS props surface and all generated Nitro bindings across iOS/Android and shared C++.
  • Update README guidance to use a ViewModel trigger + useRiveTrigger instead of onStop.
  • Add a “Finished Trigger” example demo + new harness test, and delete the prior onStop harness.

Reviewed changes

Copilot reviewed 14 out of 30 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/specs/RiveView.nitro.ts Removes onStop from Nitro view prop spec.
src/core/RiveView.tsx Removes onStop prop passthrough/defaulting from the JS component wrapper.
README.md Replaces onStop docs with data-binding trigger + useRiveTrigger guidance.
nitrogen/generated/shared/json/RiveViewConfig.json Removes onStop from generated prop config.
nitrogen/generated/shared/c++/views/HybridRiveViewComponent.hpp Removes cached onStop prop from shared generated component.
nitrogen/generated/shared/c++/views/HybridRiveViewComponent.cpp Removes onStop raw-prop extraction and prop hashing entry.
nitrogen/generated/shared/c++/HybridRiveViewSpec.hpp Removes getOnStop/setOnStop from shared generated spec.
nitrogen/generated/shared/c++/HybridRiveViewSpec.cpp Removes onStop getter/setter registration.
nitrogen/generated/ios/swift/HybridRiveViewSpec.swift Removes onStop from generated Swift protocol.
nitrogen/generated/ios/swift/HybridRiveViewSpec_cxx.swift Removes Swift/C++ bridge plumbing for onStop.
nitrogen/generated/ios/c++/views/HybridRiveViewComponent.mm Stops forwarding onStop updates into Swift part.
nitrogen/generated/ios/c++/HybridRiveViewSpecSwift.hpp Removes onStop forwarding methods from generated Swift wrapper.
nitrogen/generated/android/riveOnLoad.cpp Adjusts includes/registration ordering after removing onStop binding usage.
nitrogen/generated/android/kotlin/com/margelo/nitro/rive/HybridRiveViewSpec.kt Removes onStop from generated Kotlin spec and its C++ bridge property.
nitrogen/generated/android/c++/views/JHybridRiveViewStateUpdater.cpp Stops updating onStop on the Android state updater.
nitrogen/generated/android/c++/JHybridRiveViewSpec.hpp Removes getOnStop/setOnStop declarations from generated JNI spec.
nitrogen/generated/android/c++/JHybridRiveViewSpec.cpp Removes JNI wiring and include related to onStop.
ios/new/RiveReactNativeView.swift Removes settled-stream observation/coalescing logic previously used for onStop.
ios/new/HybridRiveView.swift Removes wiring from native view settling to onStop.
ios/legacy/HybridRiveView.swift Removes legacy no-op onStop prop and its explanatory comment.
example/src/reproducers/DeinitOffMain.tsx Removes usage of raw onStop prop in the reproducer.
example/src/demos/FinishedTriggerDemo.tsx Adds demo showing “finished” trigger pattern as the onStop replacement.
example/assets/rive/finished_trigger.riv.d.ts Adds generated TS type definition for the new demo .riv asset.
example/tests/on-stop.harness.tsx Removes the old onStop harness test.
example/tests/finished-trigger.harness.tsx Adds harness test asserting the trigger fires once and after playback.
docs/runtime-backends.md Removes documentation about onStop being experimental-only.
android/src/new/java/com/rive/RiveReactNativeView.kt Removes onStop callback wiring while keeping settled optimization logic.
android/src/new/java/com/margelo/nitro/rive/HybridRiveView.kt Removes onStop forwarding from Android new-runtime hybrid view.
android/src/legacy/java/com/margelo/nitro/rive/HybridRiveView.kt Removes legacy no-op onStop property and its explanatory comment.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread android/src/new/java/com/rive/RiveReactNativeView.kt
Comment thread README.md Outdated
@mfazekas

Copy link
Copy Markdown
Collaborator Author

Responses to the Copilot review comments (posting here — inline replies aren't available to me):

README snippet imports — fixed in 92b19bf. While adding them I also caught that the snippet called useViewModelInstance(riveFile) with no options, which resolves to the deprecated overload ("creates the instance synchronously via deprecated runtime APIs that block the JS thread"); it now passes { async: true }, matching the demo page.

!settled guard in observeSettled() — leaving as is. The "emits repeatedly while at rest" premise no longer holds after this PR: the loop stops calling advanceStateMachine once settled is true, and the command server only emits on an advance whose advanceAndApply returns false — so emissions stop too, apart from the one or two already in flight before the collector catches up. Guarding a couple of idempotent volatile writes of a constant isn't worth the extra condition, which mostly makes a reader wonder whether the write is non-idempotent.

Worth noting the guard doesn't buy the safety it looks like it does: if an emission queued before a pointer event is delivered after settled was reset to false, the !settled check passes and it sets settled = true anyway, parking the view until the next interaction. That's a real (rare, self-healing) race on the pinned 11.7.2, and it's fixed upstream rather than here — the restored settledFlow is backed by a generational store that rejects callbacks from before the latest unsettled boundary. A follow-up PR bumps rive-android for exactly that.

State-machine settling is intentionally internal in the Rive runtimes
(rive-android 11.8.0 removed the public settledFlow), so the onStop prop
shipped in 0.5.0-beta.1 is removed. Fire a data-binding trigger from the
state machine instead — see the new README section, the finished_trigger
example asset, and the Finished Trigger demo page. The Android render
loop keeps the internal stop-advancing-when-settled optimization.
…gacy

The behavioral trigger test passes on all four backends; only
getPropertiesAsync (used by the introspection test) is unimplemented on
the legacy backend, so that one is gated to the new runtime.
…ated

Adds the imports and passes async: true — without it the call resolves
to the deprecated synchronous overload that blocks the JS thread.
@mfazekas
mfazekas force-pushed the feat/databinding-finished-trigger branch from 92b19bf to 8fff2c9 Compare August 20, 2026 03:59

@HayesGordon HayesGordon left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I'd prefer for us to keep this, even if it's just for dev/testing purposes. But we can revisit. There is other work that we want to do for all the runtimes to move a way from the idea of play/pause and expose finer control to users

@mfazekas
mfazekas merged commit 041e03b into main Aug 21, 2026
15 checks passed
@mfazekas
mfazekas deleted the feat/databinding-finished-trigger branch August 21, 2026 16:35
mfazekas pushed a commit that referenced this pull request Aug 23, 2026
🤖 I have created a release *beep* *boop*
---


##
[0.5.0-beta.3](v0.5.0-beta.2...v0.5.0-beta.3)
(2026-08-21)


### ⚠ BREAKING CHANGES

* remove onStop callback in favor of data-binding triggers
([#364](#364))

### Features

* remove onStop callback in favor of data-binding triggers
([#364](#364))
([041e03b](041e03b))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants