From 9ccf1ae937d26e13e05adef43d34a002d3c0d958 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikl=C3=B3s=20Fazekas?= Date: Tue, 6 Oct 2026 15:42:11 +0200 Subject: [PATCH 1/4] fix: don't call into a disposed list property when useRiveList unmounts useRiveList's listener cleanup called property.removeListeners(), but by then useDisposableMemo may already have disposed the property: on unmount in release builds dispose() runs synchronously before that cleanup, and on a dependency change the old property is disposed during render. Nitro only checks for a disposed object in debug builds, so in release the call dereferenced a null native object and crashed with SIGSEGV. Native dispose() already removes the listeners on both platforms, so the call is dropped. Fixes #407 --- src/hooks/__tests__/useRiveList.test.ts | 77 +++++++++++++++++++++++++ src/hooks/useRiveList.ts | 7 ++- 2 files changed, 81 insertions(+), 3 deletions(-) create mode 100644 src/hooks/__tests__/useRiveList.test.ts diff --git a/src/hooks/__tests__/useRiveList.test.ts b/src/hooks/__tests__/useRiveList.test.ts new file mode 100644 index 00000000..4fa1bbfa --- /dev/null +++ b/src/hooks/__tests__/useRiveList.test.ts @@ -0,0 +1,77 @@ +import { renderHook, act } from '@testing-library/react-native'; +import { useRiveList } from '../useRiveList'; +import type { ViewModelInstance } from '../../specs/ViewModel.nitro'; + +// Calling a method on a disposed Nitro object dereferences a null native +// object in release builds (#407), so the mock records every such call. +function createMockListProperty() { + let disposed = false; + const callsAfterDispose: string[] = []; + const track = + (name: string, impl: () => T) => + () => { + if (disposed) callsAfterDispose.push(name); + return impl(); + }; + return { + callsAfterDispose, + addListener: jest.fn(track('addListener', () => () => {})), + removeListeners: jest.fn(track('removeListeners', () => {})), + getLengthAsync: jest.fn(track('getLengthAsync', () => Promise.resolve(3))), + dispose: jest.fn(() => { + disposed = true; + }), + }; +} + +type MockListProperty = ReturnType; + +function createMockViewModelInstance(property: MockListProperty) { + return { + listProperty: jest.fn(() => property), + } as unknown as ViewModelInstance; +} + +describe('useRiveList', () => { + it('does not call into a list property disposed by a dependency change', async () => { + const first = createMockListProperty(); + const second = createMockListProperty(); + const firstInstance = createMockViewModelInstance(first); + const secondInstance = createMockViewModelInstance(second); + + const { rerender, unmount } = renderHook( + ({ instance }: { instance: ViewModelInstance }) => + useRiveList('team', instance), + { initialProps: { instance: firstInstance } } + ); + await act(async () => {}); + + rerender({ instance: secondInstance }); + await act(async () => {}); + + expect(first.dispose).toHaveBeenCalled(); + expect(first.callsAfterDispose).toEqual([]); + unmount(); + }); + + it('does not call into the list property after it is disposed on unmount', async () => { + const globals = globalThis as { __DEV__?: boolean }; + const previousDev = globals.__DEV__; + // Release builds dispose synchronously on unmount, before the listener + // effect's cleanup runs. + globals.__DEV__ = false; + try { + const property = createMockListProperty(); + const instance = createMockViewModelInstance(property); + + const { unmount } = renderHook(() => useRiveList('team', instance)); + await act(async () => {}); + unmount(); + + expect(property.dispose).toHaveBeenCalled(); + expect(property.callsAfterDispose).toEqual([]); + } finally { + globals.__DEV__ = previousDev; + } + }); +}); diff --git a/src/hooks/useRiveList.ts b/src/hooks/useRiveList.ts index 5acdae3d..bf62b97a 100644 --- a/src/hooks/useRiveList.ts +++ b/src/hooks/useRiveList.ts @@ -50,12 +50,13 @@ export function useRiveList( }); return () => { + // The property may already be disposed here, and calling one of its + // methods then crashes release builds (#407); dispose() removes the + // native listeners itself. try { removeListener(); - property.removeListeners(); } catch { - // Property may already be disposed by useDisposableMemo (deps change). - // Native dispose() handles listener cleanup, so this is safe to ignore. + // Already disposed by useDisposableMemo. } }; }, [property]); From 0de0c2b71c8316047640834d2d2a481cfa694ec2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikl=C3=B3s=20Fazekas?= Date: Tue, 6 Oct 2026 15:42:11 +0200 Subject: [PATCH 2/4] test: add a reproducer page for the useRiveList unmount crash (#407) --- .../src/reproducers/Issue407ListUnmount.tsx | 124 ++++++++++++++++++ 1 file changed, 124 insertions(+) create mode 100644 example/src/reproducers/Issue407ListUnmount.tsx diff --git a/example/src/reproducers/Issue407ListUnmount.tsx b/example/src/reproducers/Issue407ListUnmount.tsx new file mode 100644 index 00000000..c35f7d7f --- /dev/null +++ b/example/src/reproducers/Issue407ListUnmount.tsx @@ -0,0 +1,124 @@ +import { useCallback, useEffect, useRef, useState } from 'react'; +import { View, Text, StyleSheet, Pressable } from 'react-native'; +import { + useRiveFile, + useRiveList, + useViewModelInstance, + type ViewModelInstance, +} from '@rive-app/react-native'; +import { type Metadata } from '../shared/metadata'; + +/** + * Issue #407: useRiveList's cleanup called a method on a list property that + * useDisposableMemo had already disposed. Debug builds throw (and the hook + * swallowed it); release builds segfault on the first unmount. + * + * Start mounts and unmounts a component using useRiveList every CYCLE_MS, for + * CYCLES rounds. Run it in a release build: expected is reaching + * "done" without the app crashing. + */ + +const CYCLES = 20; +const CYCLE_MS = 300; + +function TeamList({ instance }: { instance: ViewModelInstance }) { + const { length, error } = useRiveList('team', instance); + return ( + + team length: {error ? `error: ${error.message}` : String(length)} + + ); +} + +export default function Issue407ListUnmount() { + const { riveFile } = useRiveFile( + require('../../assets/rive/databinding_lists.riv') + ); + const { instance } = useViewModelInstance(riveFile, { + viewModelName: 'DevRel', + async: true, + }); + const [mounted, setMounted] = useState(false); + const [cycle, setCycle] = useState(0); + const [status, setStatus] = useState('idle'); + const timer = useRef | undefined>(undefined); + + const stop = useCallback(() => { + if (timer.current) clearInterval(timer.current); + timer.current = undefined; + }, []); + + const start = useCallback(() => { + stop(); + setCycle(0); + setStatus('running'); + let n = 0; + timer.current = setInterval(() => { + n += 1; + setMounted((m) => !m); + setCycle(Math.ceil(n / 2)); + if (n >= CYCLES * 2) { + stop(); + setStatus('done'); + } + }, CYCLE_MS); + }, [stop]); + + useEffect(() => { + if (instance) start(); + return stop; + }, [instance, start, stop]); + + return ( + + + {status} · cycle {cycle}/{CYCLES} + + + {mounted && instance ? : null} + + + + Start + + { + stop(); + setStatus('stopped'); + }} + > + Stop + + + + ); +} + +Issue407ListUnmount.metadata = { + name: 'Issue #407: useRiveList unmount', + description: + 'Mount/unmount a useRiveList consumer; crashed in release builds', +} satisfies Metadata; + +const styles = StyleSheet.create({ + container: { flex: 1, backgroundColor: '#fff', padding: 16 }, + status: { fontSize: 16, fontFamily: 'monospace', marginBottom: 12 }, + box: { + height: 60, + justifyContent: 'center', + borderWidth: 1, + borderColor: '#ddd', + borderRadius: 8, + paddingHorizontal: 12, + }, + list: { fontSize: 15 }, + buttons: { flexDirection: 'row', gap: 12, marginTop: 16 }, + button: { + backgroundColor: '#007AFF', + paddingHorizontal: 20, + paddingVertical: 10, + borderRadius: 8, + }, + buttonText: { color: '#fff', fontWeight: '600' }, +}); From 4b2ea9a3a109f3218bb16d8f9c2b45dca378e0dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikl=C3=B3s=20Fazekas?= Date: Tue, 6 Oct 2026 17:28:12 +0200 Subject: [PATCH 3/4] fix: make useRiveList operations no-ops once the list property is disposed The operations useRiveList returns (getInstanceAt, addInstance, addInstanceAt, removeInstance, removeInstanceAt, swap) closed over the property, so one called after unmount or after the instance changed (a stale closure from a timer or an awaited continuation) called into a disposed list and crashed release builds the same way as #407. They now read the property through useDisposableMemo's liveRef, as useRiveProperty and useRiveTrigger already do: after disposal they warn and resolve without touching it, and a stale operation after an instance change acts on the current list. Also corrects the cleanup comments: only the unsubscribe function returned by addListener is safe to call there, and it doesn't throw. --- src/hooks/__tests__/useRiveList.test.ts | 111 +++++++++++++++---- src/hooks/useRiveList.ts | 140 +++++++++++++----------- 2 files changed, 162 insertions(+), 89 deletions(-) diff --git a/src/hooks/__tests__/useRiveList.test.ts b/src/hooks/__tests__/useRiveList.test.ts index 4fa1bbfa..23268c94 100644 --- a/src/hooks/__tests__/useRiveList.test.ts +++ b/src/hooks/__tests__/useRiveList.test.ts @@ -18,6 +18,22 @@ function createMockListProperty() { addListener: jest.fn(track('addListener', () => () => {})), removeListeners: jest.fn(track('removeListeners', () => {})), getLengthAsync: jest.fn(track('getLengthAsync', () => Promise.resolve(3))), + getInstanceAtAsync: jest.fn( + track('getInstanceAtAsync', () => Promise.resolve(undefined)) + ), + addInstanceAsync: jest.fn( + track('addInstanceAsync', () => Promise.resolve()) + ), + addInstanceAtAsync: jest.fn( + track('addInstanceAtAsync', () => Promise.resolve()) + ), + removeInstanceAsync: jest.fn( + track('removeInstanceAsync', () => Promise.resolve()) + ), + removeInstanceAtAsync: jest.fn( + track('removeInstanceAtAsync', () => Promise.resolve()) + ), + swapAsync: jest.fn(track('swapAsync', () => Promise.resolve())), dispose: jest.fn(() => { disposed = true; }), @@ -33,20 +49,47 @@ function createMockViewModelInstance(property: MockListProperty) { } describe('useRiveList', () => { + const globals = globalThis as { __DEV__?: boolean }; + let previousDev: boolean | undefined; + let warn: jest.SpyInstance; + + beforeEach(() => { + // Release builds dispose synchronously on unmount, before the listener + // effect's cleanup runs. + previousDev = globals.__DEV__; + globals.__DEV__ = false; + warn = jest.spyOn(console, 'warn').mockImplementation(() => {}); + }); + + afterEach(() => { + globals.__DEV__ = previousDev; + warn.mockRestore(); + }); + + it('does not call into the list property after it is disposed on unmount', async () => { + const property = createMockListProperty(); + const instance = createMockViewModelInstance(property); + + const { unmount } = renderHook(() => useRiveList('team', instance)); + await act(async () => {}); + unmount(); + + expect(property.dispose).toHaveBeenCalled(); + expect(property.callsAfterDispose).toEqual([]); + }); + it('does not call into a list property disposed by a dependency change', async () => { const first = createMockListProperty(); const second = createMockListProperty(); - const firstInstance = createMockViewModelInstance(first); - const secondInstance = createMockViewModelInstance(second); const { rerender, unmount } = renderHook( ({ instance }: { instance: ViewModelInstance }) => useRiveList('team', instance), - { initialProps: { instance: firstInstance } } + { initialProps: { instance: createMockViewModelInstance(first) } } ); await act(async () => {}); - rerender({ instance: secondInstance }); + rerender({ instance: createMockViewModelInstance(second) }); await act(async () => {}); expect(first.dispose).toHaveBeenCalled(); @@ -54,24 +97,46 @@ describe('useRiveList', () => { unmount(); }); - it('does not call into the list property after it is disposed on unmount', async () => { - const globals = globalThis as { __DEV__?: boolean }; - const previousDev = globals.__DEV__; - // Release builds dispose synchronously on unmount, before the listener - // effect's cleanup runs. - globals.__DEV__ = false; - try { - const property = createMockListProperty(); - const instance = createMockViewModelInstance(property); - - const { unmount } = renderHook(() => useRiveList('team', instance)); - await act(async () => {}); - unmount(); - - expect(property.dispose).toHaveBeenCalled(); - expect(property.callsAfterDispose).toEqual([]); - } finally { - globals.__DEV__ = previousDev; - } + it('turns operations called after unmount into warned no-ops', async () => { + const property = createMockListProperty(); + const instance = createMockViewModelInstance(property); + const { result, unmount } = renderHook(() => useRiveList('team', instance)); + await act(async () => {}); + const stale = result.current; + unmount(); + + const item = {} as ViewModelInstance; + await expect(stale.getInstanceAt(0)).resolves.toBeUndefined(); + await stale.addInstance(item); + await stale.addInstanceAt(item, 0); + await stale.removeInstance(item); + await stale.removeInstanceAt(0); + await stale.swap(0, 1); + + expect(property.callsAfterDispose).toEqual([]); + expect(warn).toHaveBeenCalledTimes(6); + expect(warn.mock.calls[0]?.[0]).toContain( + "getInstanceAt('team') called after dispose" + ); + }); + + it('routes a stale operation to the current list after a dependency change', async () => { + const first = createMockListProperty(); + const second = createMockListProperty(); + const { result, rerender, unmount } = renderHook( + ({ instance }: { instance: ViewModelInstance }) => + useRiveList('team', instance), + { initialProps: { instance: createMockViewModelInstance(first) } } + ); + await act(async () => {}); + const stale = result.current; + + rerender({ instance: createMockViewModelInstance(second) }); + await act(async () => {}); + await stale.getInstanceAt(0); + + expect(first.callsAfterDispose).toEqual([]); + expect(second.getInstanceAtAsync).toHaveBeenCalledWith(0); + unmount(); }); }); diff --git a/src/hooks/useRiveList.ts b/src/hooks/useRiveList.ts index bf62b97a..c75ab1db 100644 --- a/src/hooks/useRiveList.ts +++ b/src/hooks/useRiveList.ts @@ -1,5 +1,8 @@ -import { useCallback, useEffect, useState } from 'react'; -import type { ViewModelInstance } from '../specs/ViewModel.nitro'; +import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; +import type { + ViewModelInstance, + ViewModelListProperty, +} from '../specs/ViewModel.nitro'; import type { UseRiveListResult } from '../types'; import { useDisposableMemo } from './useDisposableMemo'; @@ -25,15 +28,25 @@ export function useRiveList( setError(null); }, [path, viewModelInstance]); + // Nulled by useDisposableMemo the moment the property is disposed. The + // operations read it instead of the captured `property`: calling into a + // disposed list crashes release builds (#407). + const liveRef = useRef(undefined); + const wasEverLive = useRef(false); const property = useDisposableMemo( () => { if (!viewModelInstance) return undefined; return viewModelInstance.listProperty(path); }, (p) => p?.dispose(), - [viewModelInstance, path] + [viewModelInstance, path], + liveRef ); + if (liveRef.current) { + wasEverLive.current = true; + } + useEffect(() => { if (viewModelInstance && !property) { setError( @@ -50,13 +63,13 @@ export function useRiveList( }); return () => { - // The property may already be disposed here, and calling one of its - // methods then crashes release builds (#407); dispose() removes the - // native listeners itself. + // Only the unsubscribe function is safe here: the property may already + // be disposed, and calling one of its methods then crashes release + // builds (#407). try { removeListener(); } catch { - // Already disposed by useDisposableMemo. + // Defensive: unsubscribing should not throw. } }; }, [property]); @@ -97,68 +110,63 @@ export function useRiveList( }); }, []); - const getInstanceAt = useCallback( - (index: number) => { - return property - ? property.getInstanceAtAsync(index) - : Promise.resolve(undefined); - }, - [property] - ); - - const addInstance = useCallback( - (instance: ViewModelInstance) => { - return property - ? afterMutation(property.addInstanceAsync(instance)) - : Promise.resolve(); - }, - [property, afterMutation] - ); - - const addInstanceAt = useCallback( - (instance: ViewModelInstance, index: number) => { - return property - ? afterMutation(property.addInstanceAtAsync(instance, index)) - : Promise.resolve(); - }, - [property, afterMutation] - ); - - const removeInstance = useCallback( - (instance: ViewModelInstance) => { - return property - ? afterMutation(property.removeInstanceAsync(instance)) - : Promise.resolve(); - }, - [property, afterMutation] - ); - - const removeInstanceAt = useCallback( - (index: number) => { - return property - ? afterMutation(property.removeInstanceAtAsync(index)) - : Promise.resolve(); - }, - [property, afterMutation] - ); - - const swap = useCallback( - (index1: number, index2: number) => { - return property - ? afterMutation(property.swapAsync(index1, index2)) - : Promise.resolve(); - }, - [property, afterMutation] - ); + const operations = useMemo(() => { + const live = (operation: string) => { + const liveProperty = liveRef.current; + if (!liveProperty && wasEverLive.current) { + console.warn( + `useRiveList: ${operation}('${path}') called after dispose. ` + + 'The list property has been cleaned up — this is likely a stale ' + + 'closure from an async callback that fired after unmount.' + ); + } + return liveProperty; + }; + return { + getInstanceAt: (index: number) => { + const list = live('getInstanceAt'); + return list + ? list.getInstanceAtAsync(index) + : Promise.resolve(undefined); + }, + addInstance: (instance: ViewModelInstance) => { + const list = live('addInstance'); + return list + ? afterMutation(list.addInstanceAsync(instance)) + : Promise.resolve(); + }, + addInstanceAt: (instance: ViewModelInstance, index: number) => { + const list = live('addInstanceAt'); + return list + ? afterMutation(list.addInstanceAtAsync(instance, index)) + : Promise.resolve(); + }, + removeInstance: (instance: ViewModelInstance) => { + const list = live('removeInstance'); + return list + ? afterMutation(list.removeInstanceAsync(instance)) + : Promise.resolve(); + }, + removeInstanceAt: (index: number) => { + const list = live('removeInstanceAt'); + return list + ? afterMutation(list.removeInstanceAtAsync(index)) + : Promise.resolve(); + }, + swap: (index1: number, index2: number) => { + const list = live('swap'); + return list + ? afterMutation(list.swapAsync(index1, index2)) + : Promise.resolve(); + }, + }; + // `property` keeps the operations' identity tied to the current list. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [property, path, afterMutation]); return { length, - getInstanceAt, - addInstance, - addInstanceAt, - removeInstance, - removeInstanceAt, - swap, + ...operations, error, }; } From ce6a6aff9356618541a20880dad384749d9feef5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikl=C3=B3s=20Fazekas?= Date: Tue, 6 Oct 2026 17:53:05 +0200 Subject: [PATCH 4/4] refactor: simplify the useRiveList disposal guard --- src/hooks/__tests__/useRiveList.test.ts | 95 +++++------------ src/hooks/useRiveList.ts | 135 ++++++++++++------------ 2 files changed, 91 insertions(+), 139 deletions(-) diff --git a/src/hooks/__tests__/useRiveList.test.ts b/src/hooks/__tests__/useRiveList.test.ts index 23268c94..c87fcf39 100644 --- a/src/hooks/__tests__/useRiveList.test.ts +++ b/src/hooks/__tests__/useRiveList.test.ts @@ -2,38 +2,27 @@ import { renderHook, act } from '@testing-library/react-native'; import { useRiveList } from '../useRiveList'; import type { ViewModelInstance } from '../../specs/ViewModel.nitro'; -// Calling a method on a disposed Nitro object dereferences a null native -// object in release builds (#407), so the mock records every such call. +// Calling a method on a disposed Nitro object crashes release builds (#407), +// so the mock records every such call. function createMockListProperty() { let disposed = false; const callsAfterDispose: string[] = []; - const track = - (name: string, impl: () => T) => + const method = + (name: string, result: T) => () => { if (disposed) callsAfterDispose.push(name); - return impl(); + return result; }; return { callsAfterDispose, - addListener: jest.fn(track('addListener', () => () => {})), - removeListeners: jest.fn(track('removeListeners', () => {})), - getLengthAsync: jest.fn(track('getLengthAsync', () => Promise.resolve(3))), + addListener: jest.fn(method('addListener', () => {})), + removeListeners: jest.fn(method('removeListeners', undefined)), + getLengthAsync: jest.fn(method('getLengthAsync', Promise.resolve(3))), getInstanceAtAsync: jest.fn( - track('getInstanceAtAsync', () => Promise.resolve(undefined)) + method('getInstanceAtAsync', Promise.resolve(undefined)) ), - addInstanceAsync: jest.fn( - track('addInstanceAsync', () => Promise.resolve()) - ), - addInstanceAtAsync: jest.fn( - track('addInstanceAtAsync', () => Promise.resolve()) - ), - removeInstanceAsync: jest.fn( - track('removeInstanceAsync', () => Promise.resolve()) - ), - removeInstanceAtAsync: jest.fn( - track('removeInstanceAtAsync', () => Promise.resolve()) - ), - swapAsync: jest.fn(track('swapAsync', () => Promise.resolve())), + addInstanceAsync: jest.fn(method('addInstanceAsync', Promise.resolve())), + swapAsync: jest.fn(method('swapAsync', Promise.resolve())), dispose: jest.fn(() => { disposed = true; }), @@ -51,26 +40,22 @@ function createMockViewModelInstance(property: MockListProperty) { describe('useRiveList', () => { const globals = globalThis as { __DEV__?: boolean }; let previousDev: boolean | undefined; - let warn: jest.SpyInstance; beforeEach(() => { - // Release builds dispose synchronously on unmount, before the listener - // effect's cleanup runs. + // Release builds dispose synchronously on unmount, before effect cleanups. previousDev = globals.__DEV__; globals.__DEV__ = false; - warn = jest.spyOn(console, 'warn').mockImplementation(() => {}); }); afterEach(() => { globals.__DEV__ = previousDev; - warn.mockRestore(); }); it('does not call into the list property after it is disposed on unmount', async () => { const property = createMockListProperty(); - const instance = createMockViewModelInstance(property); - - const { unmount } = renderHook(() => useRiveList('team', instance)); + const { unmount } = renderHook(() => + useRiveList('team', createMockViewModelInstance(property)) + ); await act(async () => {}); unmount(); @@ -80,16 +65,15 @@ describe('useRiveList', () => { it('does not call into a list property disposed by a dependency change', async () => { const first = createMockListProperty(); - const second = createMockListProperty(); - const { rerender, unmount } = renderHook( ({ instance }: { instance: ViewModelInstance }) => useRiveList('team', instance), { initialProps: { instance: createMockViewModelInstance(first) } } ); await act(async () => {}); - - rerender({ instance: createMockViewModelInstance(second) }); + rerender({ + instance: createMockViewModelInstance(createMockListProperty()), + }); await act(async () => {}); expect(first.dispose).toHaveBeenCalled(); @@ -97,46 +81,19 @@ describe('useRiveList', () => { unmount(); }); - it('turns operations called after unmount into warned no-ops', async () => { + it('turns operations called after unmount into no-ops', async () => { const property = createMockListProperty(); - const instance = createMockViewModelInstance(property); - const { result, unmount } = renderHook(() => useRiveList('team', instance)); + const { result, unmount } = renderHook(() => + useRiveList('team', createMockViewModelInstance(property)) + ); await act(async () => {}); - const stale = result.current; + const { getInstanceAt, addInstance, swap } = result.current; unmount(); - const item = {} as ViewModelInstance; - await expect(stale.getInstanceAt(0)).resolves.toBeUndefined(); - await stale.addInstance(item); - await stale.addInstanceAt(item, 0); - await stale.removeInstance(item); - await stale.removeInstanceAt(0); - await stale.swap(0, 1); + await expect(getInstanceAt(0)).resolves.toBeUndefined(); + await addInstance({} as ViewModelInstance); + await swap(0, 1); expect(property.callsAfterDispose).toEqual([]); - expect(warn).toHaveBeenCalledTimes(6); - expect(warn.mock.calls[0]?.[0]).toContain( - "getInstanceAt('team') called after dispose" - ); - }); - - it('routes a stale operation to the current list after a dependency change', async () => { - const first = createMockListProperty(); - const second = createMockListProperty(); - const { result, rerender, unmount } = renderHook( - ({ instance }: { instance: ViewModelInstance }) => - useRiveList('team', instance), - { initialProps: { instance: createMockViewModelInstance(first) } } - ); - await act(async () => {}); - const stale = result.current; - - rerender({ instance: createMockViewModelInstance(second) }); - await act(async () => {}); - await stale.getInstanceAt(0); - - expect(first.callsAfterDispose).toEqual([]); - expect(second.getInstanceAtAsync).toHaveBeenCalledWith(0); - unmount(); }); }); diff --git a/src/hooks/useRiveList.ts b/src/hooks/useRiveList.ts index c75ab1db..942bb89e 100644 --- a/src/hooks/useRiveList.ts +++ b/src/hooks/useRiveList.ts @@ -1,4 +1,4 @@ -import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; +import { useCallback, useEffect, useRef, useState } from 'react'; import type { ViewModelInstance, ViewModelListProperty, @@ -28,11 +28,9 @@ export function useRiveList( setError(null); }, [path, viewModelInstance]); - // Nulled by useDisposableMemo the moment the property is disposed. The - // operations read it instead of the captured `property`: calling into a - // disposed list crashes release builds (#407). + // Cleared when the property is disposed; calling into a disposed list + // crashes release builds (#407), so the operations read it from here. const liveRef = useRef(undefined); - const wasEverLive = useRef(false); const property = useDisposableMemo( () => { if (!viewModelInstance) return undefined; @@ -43,10 +41,6 @@ export function useRiveList( liveRef ); - if (liveRef.current) { - wasEverLive.current = true; - } - useEffect(() => { if (viewModelInstance && !property) { setError( @@ -64,13 +58,8 @@ export function useRiveList( return () => { // Only the unsubscribe function is safe here: the property may already - // be disposed, and calling one of its methods then crashes release - // builds (#407). - try { - removeListener(); - } catch { - // Defensive: unsubscribing should not throw. - } + // be disposed. + removeListener(); }; }, [property]); @@ -110,63 +99,69 @@ export function useRiveList( }); }, []); - const operations = useMemo(() => { - const live = (operation: string) => { - const liveProperty = liveRef.current; - if (!liveProperty && wasEverLive.current) { - console.warn( - `useRiveList: ${operation}('${path}') called after dispose. ` + - 'The list property has been cleaned up — this is likely a stale ' + - 'closure from an async callback that fired after unmount.' - ); - } - return liveProperty; - }; - return { - getInstanceAt: (index: number) => { - const list = live('getInstanceAt'); - return list - ? list.getInstanceAtAsync(index) - : Promise.resolve(undefined); - }, - addInstance: (instance: ViewModelInstance) => { - const list = live('addInstance'); - return list - ? afterMutation(list.addInstanceAsync(instance)) - : Promise.resolve(); - }, - addInstanceAt: (instance: ViewModelInstance, index: number) => { - const list = live('addInstanceAt'); - return list - ? afterMutation(list.addInstanceAtAsync(instance, index)) - : Promise.resolve(); - }, - removeInstance: (instance: ViewModelInstance) => { - const list = live('removeInstance'); - return list - ? afterMutation(list.removeInstanceAsync(instance)) - : Promise.resolve(); - }, - removeInstanceAt: (index: number) => { - const list = live('removeInstanceAt'); - return list - ? afterMutation(list.removeInstanceAtAsync(index)) - : Promise.resolve(); - }, - swap: (index1: number, index2: number) => { - const list = live('swap'); - return list - ? afterMutation(list.swapAsync(index1, index2)) - : Promise.resolve(); - }, - }; - // `property` keeps the operations' identity tied to the current list. - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [property, path, afterMutation]); + const getInstanceAt = useCallback((index: number) => { + const list = liveRef.current; + return list ? list.getInstanceAtAsync(index) : Promise.resolve(undefined); + }, []); + + const addInstance = useCallback( + (instance: ViewModelInstance) => { + const list = liveRef.current; + return list + ? afterMutation(list.addInstanceAsync(instance)) + : Promise.resolve(); + }, + [afterMutation] + ); + + const addInstanceAt = useCallback( + (instance: ViewModelInstance, index: number) => { + const list = liveRef.current; + return list + ? afterMutation(list.addInstanceAtAsync(instance, index)) + : Promise.resolve(); + }, + [afterMutation] + ); + + const removeInstance = useCallback( + (instance: ViewModelInstance) => { + const list = liveRef.current; + return list + ? afterMutation(list.removeInstanceAsync(instance)) + : Promise.resolve(); + }, + [afterMutation] + ); + + const removeInstanceAt = useCallback( + (index: number) => { + const list = liveRef.current; + return list + ? afterMutation(list.removeInstanceAtAsync(index)) + : Promise.resolve(); + }, + [afterMutation] + ); + + const swap = useCallback( + (index1: number, index2: number) => { + const list = liveRef.current; + return list + ? afterMutation(list.swapAsync(index1, index2)) + : Promise.resolve(); + }, + [afterMutation] + ); return { length, - ...operations, + getInstanceAt, + addInstance, + addInstanceAt, + removeInstance, + removeInstanceAt, + swap, error, }; }