Skip to content

Commit 17674fa

Browse files
authored
fix(experimental): address review findings — coroutine scope, RiveUIView 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)
1 parent f6427be commit 17674fa

5 files changed

Lines changed: 243 additions & 22 deletions

File tree

android/src/new/java/com/rive/RiveReactNativeView.kt

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ import com.margelo.nitro.rive.RiveLog
2222
import kotlinx.coroutines.CompletableDeferred
2323
import kotlinx.coroutines.CoroutineScope
2424
import kotlinx.coroutines.Dispatchers
25+
import kotlinx.coroutines.SupervisorJob
26+
import kotlinx.coroutines.cancel
2527
import kotlinx.coroutines.launch
2628
import kotlinx.coroutines.withContext
2729
import kotlin.time.Duration
@@ -81,6 +83,8 @@ class RiveReactNativeView(context: ThemedReactContext) : FrameLayout(context) {
8183
@Volatile
8284
private var paused = false
8385

86+
private val viewScope = CoroutineScope(Dispatchers.Default + SupervisorJob())
87+
8488
private val textureView = TextureView(context).apply {
8589
layoutParams = LayoutParams(LayoutParams.MATCH_PARENT, LayoutParams.MATCH_PARENT)
8690
surfaceTextureListener = object : TextureView.SurfaceTextureListener {
@@ -290,16 +294,15 @@ class RiveReactNativeView(context: ThemedReactContext) : FrameLayout(context) {
290294
boundInstance = null
291295
}
292296
is BindData.Auto -> {
293-
CoroutineScope(Dispatchers.Default).launch {
297+
viewScope.launch {
294298
try {
295299
val vmNames = riveFile.getViewModelNames()
296300
if (vmNames.isEmpty()) return@launch
297301
withContext(Dispatchers.Main) {
302+
if (disposed) return@withContext
298303
val art = artboard ?: return@withContext
299304
val source = ViewModelSource.DefaultForArtboard(art).defaultInstance()
300305
val instance = ViewModelInstance.fromFile(riveFile, source)
301-
// A handle of 1L is the C++ null sentinel — the artboard has ViewModels but
302-
// none is set as the default, so binding would fire "instance 0x1 not found".
303306
if (instance.instanceHandle.handle == 1L) {
304307
Log.d(TAG, "Auto-binding skipped: no default ViewModel for artboard")
305308
return@withContext
@@ -387,6 +390,7 @@ class RiveReactNativeView(context: ThemedReactContext) : FrameLayout(context) {
387390

388391
fun dispose() {
389392
disposed = true
393+
viewScope.cancel()
390394
RiveErrorLogger.removeListener(errorListener)
391395
stopRenderLoop()
392396
// Null handles to prevent any further draw calls.
Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
import {
2+
describe,
3+
it,
4+
expect,
5+
render,
6+
waitFor,
7+
cleanup,
8+
} from 'react-native-harness';
9+
import { Platform, View } from 'react-native';
10+
import {
11+
RiveView,
12+
RiveFileFactory,
13+
Fit,
14+
type RiveFile,
15+
type RiveViewRef,
16+
type ViewModelInstance,
17+
} from '@rive-app/react-native';
18+
19+
// bouncing_ball has a 'ypos' ViewModel property driven by the state machine
20+
const BOUNCING_BALL = require('../assets/rive/bouncing_ball.riv');
21+
22+
function expectDefined<T>(value: T): asserts value is NonNullable<T> {
23+
expect(value).toBeDefined();
24+
}
25+
26+
const delay = (ms: number) => new Promise((r) => setTimeout(r, ms));
27+
28+
describe('bindViewModelInstance before view ready', () => {
29+
it('bind called before configure completes is applied', async () => {
30+
// The race only exists on iOS experimental — riveInstance is set async
31+
if (
32+
Platform.OS !== 'ios' ||
33+
RiveFileFactory.getBackend() !== 'experimental'
34+
) {
35+
return;
36+
}
37+
38+
const file = await RiveFileFactory.fromSource(BOUNCING_BALL, undefined);
39+
const vm = file.defaultArtboardViewModel();
40+
expectDefined(vm);
41+
const instance: ViewModelInstance = vm.createDefaultInstance()!;
42+
expectDefined(instance);
43+
44+
let ref: RiveViewRef | null = null;
45+
let error: string | null = null;
46+
let bindCalledBeforeReady = false;
47+
48+
// The hybridRef callback fires synchronously on mount, before the Swift
49+
// configTask async work has had a chance to run — so riveInstance is nil
50+
// at this point, and the bind will be silently dropped without the fix.
51+
function TestView({ riveFile }: { riveFile: RiveFile }) {
52+
return (
53+
<View style={{ width: 200, height: 200 }}>
54+
<RiveView
55+
hybridRef={{
56+
f: (r: RiveViewRef | null) => {
57+
ref = r;
58+
if (r && !bindCalledBeforeReady) {
59+
bindCalledBeforeReady = true;
60+
r.bindViewModelInstance(instance);
61+
}
62+
},
63+
}}
64+
style={{ flex: 1 }}
65+
file={riveFile}
66+
autoPlay
67+
fit={Fit.Contain}
68+
onError={(e) => {
69+
error = e.message;
70+
}}
71+
/>
72+
</View>
73+
);
74+
}
75+
76+
await render(<TestView riveFile={file} />);
77+
await waitFor(() => expect(ref).not.toBeNull(), { timeout: 5000 });
78+
79+
expect(bindCalledBeforeReady).toBe(true);
80+
81+
// Let the animation run
82+
await delay(1200);
83+
84+
// ypos on OUR instance should be changing if the bind was applied.
85+
// If the bind was silently dropped, ypos stays at its initial value
86+
// because the state machine is driving a different (auto-bound) instance.
87+
const ypos = instance.numberProperty('ypos');
88+
expectDefined(ypos);
89+
const v1 = ypos.value;
90+
await delay(600);
91+
expect(ypos.value).not.toBe(v1);
92+
93+
expect(error).toBeNull();
94+
cleanup();
95+
});
96+
});
Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
import {
2+
describe,
3+
it,
4+
expect,
5+
render,
6+
waitFor,
7+
cleanup,
8+
} from 'react-native-harness';
9+
import { useState } from 'react';
10+
import { Platform, View } from 'react-native';
11+
import {
12+
RiveView,
13+
RiveFileFactory,
14+
Fit,
15+
type RiveFile,
16+
type RiveViewRef,
17+
} from '@rive-app/react-native';
18+
19+
const BOUNCING_BALL = require('../assets/rive/bouncing_ball.riv');
20+
const COUNTER = require('../assets/rive/counter.riv');
21+
22+
const delay = (ms: number) => new Promise((r) => setTimeout(r, ms));
23+
24+
type TestContext = { ref: RiveViewRef | null; error: string | null };
25+
26+
function SwitchableRiveView({
27+
file,
28+
context,
29+
}: {
30+
file: RiveFile;
31+
context: TestContext;
32+
}) {
33+
return (
34+
<View style={{ width: 200, height: 200 }}>
35+
<RiveView
36+
hybridRef={{
37+
f: (ref: RiveViewRef | null) => {
38+
context.ref = ref;
39+
},
40+
}}
41+
style={{ flex: 1 }}
42+
file={file}
43+
autoPlay
44+
fit={Fit.Contain}
45+
onError={(e) => {
46+
context.error = e.message;
47+
}}
48+
/>
49+
</View>
50+
);
51+
}
52+
53+
describe('RiveView reconfigure (file switch)', () => {
54+
it('animation still plays after switching file prop and back', async () => {
55+
// Fix only applies to iOS experimental — setupRiveUIView teardown churn
56+
if (
57+
Platform.OS !== 'ios' ||
58+
RiveFileFactory.getBackend() !== 'experimental'
59+
) {
60+
return;
61+
}
62+
63+
const file1 = await RiveFileFactory.fromSource(BOUNCING_BALL, undefined);
64+
const file2 = await RiveFileFactory.fromSource(COUNTER, undefined);
65+
const context: TestContext = { ref: null, error: null };
66+
67+
let setFile!: (f: RiveFile) => void;
68+
function Wrapper() {
69+
const [file, _setFile] = useState<RiveFile>(file1);
70+
setFile = _setFile;
71+
return <SwitchableRiveView file={file} context={context} />;
72+
}
73+
74+
await render(<Wrapper />);
75+
await waitFor(() => expect(context.ref).not.toBeNull(), { timeout: 5000 });
76+
77+
// Confirm bouncing_ball is animating via its ypos ViewModel property
78+
const vmi1 = context.ref!.getViewModelInstance();
79+
expect(vmi1).not.toBeNull();
80+
const ypos1 = vmi1!.numberProperty('ypos');
81+
expect(ypos1).toBeDefined();
82+
const valueBefore = ypos1!.value;
83+
await delay(500);
84+
expect(ypos1!.value).not.toBe(valueBefore);
85+
86+
// Switch to counter.riv — triggers setupRiveUIView reconfigure
87+
setFile(file2);
88+
await delay(600);
89+
expect(context.error).toBeNull();
90+
91+
// Switch back to bouncing_ball — triggers setupRiveUIView again
92+
setFile(file1);
93+
await delay(600);
94+
expect(context.error).toBeNull();
95+
96+
// Animation should still be running on the reconfigured view
97+
const vmi2 = context.ref!.getViewModelInstance();
98+
expect(vmi2).not.toBeNull();
99+
const ypos2 = vmi2!.numberProperty('ypos');
100+
expect(ypos2).toBeDefined();
101+
const valueAfterSwitch = ypos2!.value;
102+
await delay(500);
103+
expect(ypos2!.value).not.toBe(valueAfterSwitch);
104+
105+
expect(context.error).toBeNull();
106+
cleanup();
107+
});
108+
});

ios/new/RiveReactNativeView.swift

Lines changed: 30 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ struct ViewConfiguration {
2222
class RiveReactNativeView: UIView {
2323
private var riveUIView: RiveUIView?
2424
private var riveInstance: RiveRuntime.Rive?
25+
private var pendingBindInstance: ViewModelInstance?
2526
private var viewReadyContinuations: [CheckedContinuation<Void, Never>] = []
2627
private var isViewReady = false
2728
private var configTask: Task<Void, Never>?
@@ -87,6 +88,10 @@ class RiveReactNativeView: UIView {
8788

8889
self.riveInstance = rive
8990
self.isPaused = !config.autoPlay
91+
if let pending = self.pendingBindInstance {
92+
self.pendingBindInstance = nil
93+
rive.stateMachine.bindViewModelInstance(pending)
94+
}
9095
self.setupRiveUIView(with: rive)
9196

9297
if !self.isViewReady {
@@ -106,7 +111,12 @@ class RiveReactNativeView: UIView {
106111
}
107112

108113
func bindViewModelInstance(viewModelInstance: ViewModelInstance) {
109-
riveInstance?.stateMachine.bindViewModelInstance(viewModelInstance)
114+
if let rive = riveInstance {
115+
rive.stateMachine.bindViewModelInstance(viewModelInstance)
116+
} else {
117+
// configTask hasn't finished yet — queue it to apply once riveInstance is set
118+
pendingBindInstance = viewModelInstance
119+
}
110120
}
111121

112122
func getViewModelInstance() -> ViewModelInstance? {
@@ -166,22 +176,24 @@ class RiveReactNativeView: UIView {
166176
// MARK: - Internal
167177

168178
private func setupRiveUIView(with rive: RiveRuntime.Rive) {
169-
// Remove existing view if any
170-
// Note: The old RiveUIView's MTKView may still fire a few draw calls after removal,
171-
// which can cause "state machine not found" errors if the old state machine is deallocated.
172-
// This is a limitation of the experimental API - RiveUIView.rive is not publicly settable.
173-
riveUIView?.removeFromSuperview()
174-
175-
let uiView = RiveUIView(rive: rive, isPaused: isPaused)
176-
uiView.translatesAutoresizingMaskIntoConstraints = false
177-
addSubview(uiView)
178-
NSLayoutConstraint.activate([
179-
uiView.leadingAnchor.constraint(equalTo: leadingAnchor),
180-
uiView.trailingAnchor.constraint(equalTo: trailingAnchor),
181-
uiView.topAnchor.constraint(equalTo: topAnchor),
182-
uiView.bottomAnchor.constraint(equalTo: bottomAnchor),
183-
])
184-
self.riveUIView = uiView
179+
if let existing = riveUIView {
180+
// Reuse the existing view — avoids tearing down the MTKView on every
181+
// reconfigure, which previously caused orphaned draw calls ("state machine
182+
// not found") from the old MTKView after removeFromSuperview.
183+
existing.rive = rive
184+
existing.isPaused = isPaused
185+
} else {
186+
let uiView = RiveUIView(rive: rive, isPaused: isPaused)
187+
uiView.translatesAutoresizingMaskIntoConstraints = false
188+
addSubview(uiView)
189+
NSLayoutConstraint.activate([
190+
uiView.leadingAnchor.constraint(equalTo: leadingAnchor),
191+
uiView.trailingAnchor.constraint(equalTo: trailingAnchor),
192+
uiView.topAnchor.constraint(equalTo: topAnchor),
193+
uiView.bottomAnchor.constraint(equalTo: bottomAnchor),
194+
])
195+
self.riveUIView = uiView
196+
}
185197
}
186198

187199
private func cleanup() {
@@ -191,6 +203,7 @@ class RiveReactNativeView: UIView {
191203
riveUIView?.removeFromSuperview()
192204
riveUIView = nil
193205
riveInstance = nil
206+
pendingBindInstance = nil
194207
}
195208

196209
deinit {

src/hooks/useViewModelInstance.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -174,8 +174,8 @@ function createInstance(
174174
if (instanceName) {
175175
try {
176176
vmi = source.createInstanceByName(instanceName);
177-
} catch {
178-
// experimental backend throws for non-existent names
177+
} catch (e) {
178+
console.warn(`createInstanceByName('${instanceName}') failed:`, e);
179179
}
180180
if (!vmi) {
181181
return {

0 commit comments

Comments
 (0)