Skip to content

Commit 136e109

Browse files
mfazekasmfazekas
authored
fix: survive fast refresh without disposing native objects (#212)
## Summary Fixes #205. Fast refresh (HMR) re-runs all effect cleanups then all effects. Hooks that disposed native HybridObjects in effect cleanup would destroy the backing native state, but `useMemo` caches still returned the same (now dead) JS references — causing `NativeState is null` crashes. Introduces `useDisposableMemo` — a `useMemo` replacement for disposable native resources: - Value created synchronously during render (available on first render) - Old value disposed in render phase when deps change - **Prod**: synchronous dispose on unmount (zero overhead) - **Dev**: deferred dispose via `setTimeout(0)`, cancelled if fast refresh / Strict Mode re-mounts the component Migrates `useRiveProperty`, `useRiveList`, and `useViewModelInstance`. The `prevInstanceRef` dance in `useViewModelInstance` (manual render-phase disposal + separate `useEffect` unmount cleanup) is replaced by a single `useDisposableMemo` call that handles both. ## Test plan - [x] Prototype validated with fake native objects (mount/unmount/fast refresh/re-render) - [x] Tested on iOS simulator with Data Binding exerciser — fast refresh survives
1 parent c31b359 commit 136e109

5 files changed

Lines changed: 412 additions & 59 deletions

File tree

‎src/hooks/__tests__/useDisposableMemo.test.ts‎

Lines changed: 242 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,242 @@
1+
import { renderHook, act } from '@testing-library/react-native';
2+
import { useDisposableMemo } from '../useDisposableMemo';
3+
4+
function createDisposable(label: string) {
5+
let alive = true;
6+
return {
7+
label,
8+
get value(): string {
9+
if (!alive) throw new Error(`${label} was disposed`);
10+
return `value-of-${label}`;
11+
},
12+
dispose() {
13+
if (!alive) throw new Error(`${label} double-disposed`);
14+
alive = false;
15+
},
16+
get isAlive() {
17+
return alive;
18+
},
19+
};
20+
}
21+
22+
type Disposable = ReturnType<typeof createDisposable>;
23+
24+
describe('useDisposableMemo', () => {
25+
beforeEach(() => {
26+
jest.useFakeTimers();
27+
});
28+
29+
afterEach(() => {
30+
// Flush any pending deferred disposals from the test
31+
try {
32+
jest.runAllTimers();
33+
} catch {
34+
// Some tests intentionally have throwing cleanups
35+
}
36+
jest.useRealTimers();
37+
});
38+
39+
it('creates value on first render, available immediately', () => {
40+
const { result } = renderHook(() =>
41+
useDisposableMemo(
42+
() => createDisposable('A'),
43+
(d) => d.dispose(),
44+
['dep-a']
45+
)
46+
);
47+
48+
expect(result.current.isAlive).toBe(true);
49+
expect(result.current.value).toBe('value-of-A');
50+
});
51+
52+
it('returns same value on re-render with unchanged deps', () => {
53+
const factory = jest.fn(() => createDisposable('A'));
54+
55+
const { result, rerender } = renderHook(() =>
56+
useDisposableMemo(factory, (d) => d.dispose(), ['stable'])
57+
);
58+
59+
const firstValue = result.current;
60+
rerender({});
61+
rerender({});
62+
63+
expect(result.current).toBe(firstValue);
64+
expect(factory).toHaveBeenCalledTimes(1);
65+
expect(result.current.isAlive).toBe(true);
66+
});
67+
68+
it('disposes old value and creates new one when deps change', () => {
69+
const { result, rerender } = renderHook(
70+
(props: { dep: string }) =>
71+
useDisposableMemo(
72+
() => createDisposable(props.dep),
73+
(d) => d.dispose(),
74+
[props.dep]
75+
),
76+
{ initialProps: { dep: 'A' } }
77+
);
78+
79+
const first = result.current;
80+
expect(first.label).toBe('A');
81+
expect(first.isAlive).toBe(true);
82+
83+
rerender({ dep: 'B' });
84+
85+
expect(first.isAlive).toBe(false);
86+
expect(result.current.label).toBe('B');
87+
expect(result.current.isAlive).toBe(true);
88+
});
89+
90+
it('disposes on unmount (via deferred timeout in dev)', () => {
91+
const { result, unmount } = renderHook(() =>
92+
useDisposableMemo(
93+
() => createDisposable('A'),
94+
(d) => d.dispose(),
95+
['dep']
96+
)
97+
);
98+
99+
const obj = result.current;
100+
expect(obj.isAlive).toBe(true);
101+
102+
unmount();
103+
104+
// Not yet disposed — waiting for setTimeout(0)
105+
expect(obj.isAlive).toBe(true);
106+
107+
act(() => {
108+
jest.runAllTimers();
109+
});
110+
111+
expect(obj.isAlive).toBe(false);
112+
});
113+
114+
it('handles rapid deps cycling A → B → A', () => {
115+
const disposed: string[] = [];
116+
117+
const { result, rerender } = renderHook(
118+
(props: { dep: string }) =>
119+
useDisposableMemo(
120+
() => createDisposable(props.dep),
121+
(d) => {
122+
disposed.push(d.label);
123+
d.dispose();
124+
},
125+
[props.dep]
126+
),
127+
{ initialProps: { dep: 'A' } }
128+
);
129+
130+
const first = result.current;
131+
132+
rerender({ dep: 'B' });
133+
expect(disposed).toEqual(['A']);
134+
const second = result.current;
135+
136+
rerender({ dep: 'A' });
137+
expect(disposed).toEqual(['A', 'B']);
138+
139+
// New 'A' is a fresh instance, not the original
140+
expect(result.current).not.toBe(first);
141+
expect(result.current.label).toBe('A');
142+
expect(result.current.isAlive).toBe(true);
143+
expect(first.isAlive).toBe(false);
144+
expect(second.isAlive).toBe(false);
145+
});
146+
147+
it('handles factory returning undefined', () => {
148+
const cleanup = jest.fn();
149+
150+
const { result, rerender } = renderHook(
151+
(props: { dep: string }) =>
152+
useDisposableMemo(() => undefined as Disposable | undefined, cleanup, [
153+
props.dep,
154+
]),
155+
{ initialProps: { dep: 'A' } }
156+
);
157+
158+
expect(result.current).toBeUndefined();
159+
160+
rerender({ dep: 'B' });
161+
162+
// cleanup called with undefined — should not throw
163+
expect(cleanup).toHaveBeenCalledWith(undefined);
164+
});
165+
166+
it('survives cleanup throwing', () => {
167+
const { result, rerender } = renderHook(
168+
(props: { dep: string }) =>
169+
useDisposableMemo(
170+
() => createDisposable(props.dep),
171+
() => {
172+
throw new Error('cleanup exploded');
173+
},
174+
[props.dep]
175+
),
176+
{ initialProps: { dep: 'A' } }
177+
);
178+
179+
expect(result.current.label).toBe('A');
180+
181+
// Deps change — cleanup throws but new value is still created
182+
rerender({ dep: 'B' });
183+
184+
expect(result.current.label).toBe('B');
185+
expect(result.current.isAlive).toBe(true);
186+
});
187+
188+
it('disposes each intermediate value on sequential deps changes', () => {
189+
const disposed: string[] = [];
190+
191+
const { result, rerender, unmount } = renderHook(
192+
(props: { dep: string }) =>
193+
useDisposableMemo(
194+
() => createDisposable(props.dep),
195+
(d) => {
196+
disposed.push(d.label);
197+
d.dispose();
198+
},
199+
[props.dep]
200+
),
201+
{ initialProps: { dep: 'A' } }
202+
);
203+
204+
rerender({ dep: 'B' });
205+
rerender({ dep: 'C' });
206+
rerender({ dep: 'D' });
207+
208+
expect(disposed).toEqual(['A', 'B', 'C']);
209+
expect(result.current.label).toBe('D');
210+
expect(result.current.isAlive).toBe(true);
211+
212+
unmount();
213+
act(() => {
214+
jest.runAllTimers();
215+
});
216+
217+
expect(disposed).toEqual(['A', 'B', 'C', 'D']);
218+
});
219+
220+
it('compares deps with Object.is semantics', () => {
221+
const factory = jest.fn(() => createDisposable('A'));
222+
223+
const { rerender } = renderHook(
224+
(props: { dep: number }) =>
225+
useDisposableMemo(factory, (d) => d.dispose(), [props.dep]),
226+
{ initialProps: { dep: NaN } }
227+
);
228+
229+
expect(factory).toHaveBeenCalledTimes(1);
230+
231+
// NaN === NaN under Object.is
232+
rerender({ dep: NaN });
233+
expect(factory).toHaveBeenCalledTimes(1);
234+
235+
// 0 !== -0 under Object.is
236+
rerender({ dep: 0 });
237+
expect(factory).toHaveBeenCalledTimes(2);
238+
239+
rerender({ dep: -0 });
240+
expect(factory).toHaveBeenCalledTimes(3);
241+
});
242+
});

‎src/hooks/useDisposableMemo.ts‎

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
import { useRef, useEffect, type DependencyList } from 'react';
2+
3+
const UNINITIALIZED = Symbol('UNINITIALIZED');
4+
5+
function depsEqual(a: DependencyList, b: DependencyList): boolean {
6+
if (a.length !== b.length) return false;
7+
for (let i = 0; i < a.length; i++) {
8+
if (!Object.is(a[i], b[i])) return false;
9+
}
10+
return true;
11+
}
12+
13+
/**
14+
* Like `useMemo`, but with a cleanup callback for disposable native resources.
15+
*
16+
* - Value is created synchronously during render (available on first render).
17+
* - When deps change, the old value is cleaned up during render and a new one
18+
* is created.
19+
* - On unmount in production: cleaned up synchronously in effect cleanup.
20+
* - On unmount in development: cleanup is deferred via `setTimeout(0)` so that
21+
* fast refresh and Strict Mode can cancel it when effects re-run.
22+
*
23+
* Replaces the common `useMemo` + dispose-in-`useEffect`-cleanup pattern that
24+
* breaks on fast refresh (HMR re-runs all effect cleanups, disposing the native
25+
* object, but `useMemo` returns the same dead reference):
26+
*
27+
* ```tsx
28+
* // BEFORE — breaks on fast refresh
29+
* const property = useMemo(() => instance?.getProperty(path), [instance, path]);
30+
* useEffect(() => {
31+
* const unsub = property?.addListener(setValue);
32+
* return () => { unsub?.(); property?.dispose(); };
33+
* }, [property]);
34+
*
35+
* // AFTER
36+
* const property = useDisposableMemo(
37+
* () => instance?.getProperty(path),
38+
* (p) => p?.dispose(),
39+
* [instance, path]
40+
* );
41+
* useEffect(() => {
42+
* const unsub = property?.addListener(setValue);
43+
* return () => unsub?.(); // only unsubscribe, no dispose
44+
* }, [property]);
45+
* ```
46+
*/
47+
export function useDisposableMemo<T>(
48+
factory: () => T,
49+
cleanup: (value: T) => void,
50+
deps: DependencyList
51+
): T {
52+
const ref = useRef<{
53+
value: T;
54+
deps: DependencyList | typeof UNINITIALIZED;
55+
pendingDisposal: ReturnType<typeof setTimeout> | null;
56+
}>({
57+
value: undefined as T,
58+
deps: UNINITIALIZED,
59+
pendingDisposal: null,
60+
});
61+
const cleanupRef = useRef(cleanup);
62+
cleanupRef.current = cleanup;
63+
64+
if (
65+
ref.current.deps === UNINITIALIZED ||
66+
!depsEqual(ref.current.deps, deps)
67+
) {
68+
if (__DEV__ && ref.current.pendingDisposal !== null) {
69+
clearTimeout(ref.current.pendingDisposal);
70+
ref.current.pendingDisposal = null;
71+
}
72+
if (ref.current.deps !== UNINITIALIZED) {
73+
try {
74+
cleanupRef.current(ref.current.value);
75+
} catch {
76+
// Swallow cleanup errors — the old value is being replaced regardless.
77+
}
78+
}
79+
ref.current = { value: factory(), deps, pendingDisposal: null };
80+
}
81+
82+
useEffect(() => {
83+
if (__DEV__) {
84+
if (ref.current.pendingDisposal !== null) {
85+
clearTimeout(ref.current.pendingDisposal);
86+
ref.current.pendingDisposal = null;
87+
}
88+
}
89+
return () => {
90+
if (__DEV__) {
91+
const val = ref.current.value;
92+
ref.current.pendingDisposal = setTimeout(() => {
93+
try {
94+
cleanupRef.current(val);
95+
} catch {
96+
// Swallow — object may already be in a bad state.
97+
}
98+
ref.current.pendingDisposal = null;
99+
}, 0);
100+
} else {
101+
try {
102+
cleanupRef.current(ref.current.value);
103+
} catch {
104+
// Swallow — object may already be in a bad state.
105+
}
106+
}
107+
};
108+
}, []);
109+
110+
return ref.current.value;
111+
}

‎src/hooks/useRiveList.ts‎

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import { useCallback, useEffect, useState, useMemo } from 'react';
44
import type { ViewModelInstance } from '../specs/ViewModel.nitro';
55
import type { UseRiveListResult } from '../types';
6+
import { useDisposableMemo } from './useDisposableMemo';
67

78
/**
89
* Hook for interacting with list ViewModel instance properties.
@@ -22,10 +23,14 @@ export function useRiveList(
2223
setError(null);
2324
}, [path, viewModelInstance]);
2425

25-
const property = useMemo(() => {
26-
if (!viewModelInstance) return undefined;
27-
return viewModelInstance.listProperty(path);
28-
}, [viewModelInstance, path]);
26+
const property = useDisposableMemo(
27+
() => {
28+
if (!viewModelInstance) return undefined;
29+
return viewModelInstance.listProperty(path);
30+
},
31+
(p) => p?.dispose(),
32+
[viewModelInstance, path]
33+
);
2934

3035
useEffect(() => {
3136
if (viewModelInstance && !property) {
@@ -43,9 +48,13 @@ export function useRiveList(
4348
});
4449

4550
return () => {
46-
removeListener();
47-
property.removeListeners();
48-
property.dispose();
51+
try {
52+
removeListener();
53+
property.removeListeners();
54+
} catch {
55+
// Property may already be disposed by useDisposableMemo (deps change).
56+
// Native dispose() handles listener cleanup, so this is safe to ignore.
57+
}
4958
};
5059
}, [property]);
5160

0 commit comments

Comments
 (0)