diff --git a/src/hooks/__tests__/useDisposableMemo.test.ts b/src/hooks/__tests__/useDisposableMemo.test.ts new file mode 100644 index 00000000..eb3a48fe --- /dev/null +++ b/src/hooks/__tests__/useDisposableMemo.test.ts @@ -0,0 +1,242 @@ +import { renderHook, act } from '@testing-library/react-native'; +import { useDisposableMemo } from '../useDisposableMemo'; + +function createDisposable(label: string) { + let alive = true; + return { + label, + get value(): string { + if (!alive) throw new Error(`${label} was disposed`); + return `value-of-${label}`; + }, + dispose() { + if (!alive) throw new Error(`${label} double-disposed`); + alive = false; + }, + get isAlive() { + return alive; + }, + }; +} + +type Disposable = ReturnType; + +describe('useDisposableMemo', () => { + beforeEach(() => { + jest.useFakeTimers(); + }); + + afterEach(() => { + // Flush any pending deferred disposals from the test + try { + jest.runAllTimers(); + } catch { + // Some tests intentionally have throwing cleanups + } + jest.useRealTimers(); + }); + + it('creates value on first render, available immediately', () => { + const { result } = renderHook(() => + useDisposableMemo( + () => createDisposable('A'), + (d) => d.dispose(), + ['dep-a'] + ) + ); + + expect(result.current.isAlive).toBe(true); + expect(result.current.value).toBe('value-of-A'); + }); + + it('returns same value on re-render with unchanged deps', () => { + const factory = jest.fn(() => createDisposable('A')); + + const { result, rerender } = renderHook(() => + useDisposableMemo(factory, (d) => d.dispose(), ['stable']) + ); + + const firstValue = result.current; + rerender({}); + rerender({}); + + expect(result.current).toBe(firstValue); + expect(factory).toHaveBeenCalledTimes(1); + expect(result.current.isAlive).toBe(true); + }); + + it('disposes old value and creates new one when deps change', () => { + const { result, rerender } = renderHook( + (props: { dep: string }) => + useDisposableMemo( + () => createDisposable(props.dep), + (d) => d.dispose(), + [props.dep] + ), + { initialProps: { dep: 'A' } } + ); + + const first = result.current; + expect(first.label).toBe('A'); + expect(first.isAlive).toBe(true); + + rerender({ dep: 'B' }); + + expect(first.isAlive).toBe(false); + expect(result.current.label).toBe('B'); + expect(result.current.isAlive).toBe(true); + }); + + it('disposes on unmount (via deferred timeout in dev)', () => { + const { result, unmount } = renderHook(() => + useDisposableMemo( + () => createDisposable('A'), + (d) => d.dispose(), + ['dep'] + ) + ); + + const obj = result.current; + expect(obj.isAlive).toBe(true); + + unmount(); + + // Not yet disposed — waiting for setTimeout(0) + expect(obj.isAlive).toBe(true); + + act(() => { + jest.runAllTimers(); + }); + + expect(obj.isAlive).toBe(false); + }); + + it('handles rapid deps cycling A → B → A', () => { + const disposed: string[] = []; + + const { result, rerender } = renderHook( + (props: { dep: string }) => + useDisposableMemo( + () => createDisposable(props.dep), + (d) => { + disposed.push(d.label); + d.dispose(); + }, + [props.dep] + ), + { initialProps: { dep: 'A' } } + ); + + const first = result.current; + + rerender({ dep: 'B' }); + expect(disposed).toEqual(['A']); + const second = result.current; + + rerender({ dep: 'A' }); + expect(disposed).toEqual(['A', 'B']); + + // New 'A' is a fresh instance, not the original + expect(result.current).not.toBe(first); + expect(result.current.label).toBe('A'); + expect(result.current.isAlive).toBe(true); + expect(first.isAlive).toBe(false); + expect(second.isAlive).toBe(false); + }); + + it('handles factory returning undefined', () => { + const cleanup = jest.fn(); + + const { result, rerender } = renderHook( + (props: { dep: string }) => + useDisposableMemo(() => undefined as Disposable | undefined, cleanup, [ + props.dep, + ]), + { initialProps: { dep: 'A' } } + ); + + expect(result.current).toBeUndefined(); + + rerender({ dep: 'B' }); + + // cleanup called with undefined — should not throw + expect(cleanup).toHaveBeenCalledWith(undefined); + }); + + it('survives cleanup throwing', () => { + const { result, rerender } = renderHook( + (props: { dep: string }) => + useDisposableMemo( + () => createDisposable(props.dep), + () => { + throw new Error('cleanup exploded'); + }, + [props.dep] + ), + { initialProps: { dep: 'A' } } + ); + + expect(result.current.label).toBe('A'); + + // Deps change — cleanup throws but new value is still created + rerender({ dep: 'B' }); + + expect(result.current.label).toBe('B'); + expect(result.current.isAlive).toBe(true); + }); + + it('disposes each intermediate value on sequential deps changes', () => { + const disposed: string[] = []; + + const { result, rerender, unmount } = renderHook( + (props: { dep: string }) => + useDisposableMemo( + () => createDisposable(props.dep), + (d) => { + disposed.push(d.label); + d.dispose(); + }, + [props.dep] + ), + { initialProps: { dep: 'A' } } + ); + + rerender({ dep: 'B' }); + rerender({ dep: 'C' }); + rerender({ dep: 'D' }); + + expect(disposed).toEqual(['A', 'B', 'C']); + expect(result.current.label).toBe('D'); + expect(result.current.isAlive).toBe(true); + + unmount(); + act(() => { + jest.runAllTimers(); + }); + + expect(disposed).toEqual(['A', 'B', 'C', 'D']); + }); + + it('compares deps with Object.is semantics', () => { + const factory = jest.fn(() => createDisposable('A')); + + const { rerender } = renderHook( + (props: { dep: number }) => + useDisposableMemo(factory, (d) => d.dispose(), [props.dep]), + { initialProps: { dep: NaN } } + ); + + expect(factory).toHaveBeenCalledTimes(1); + + // NaN === NaN under Object.is + rerender({ dep: NaN }); + expect(factory).toHaveBeenCalledTimes(1); + + // 0 !== -0 under Object.is + rerender({ dep: 0 }); + expect(factory).toHaveBeenCalledTimes(2); + + rerender({ dep: -0 }); + expect(factory).toHaveBeenCalledTimes(3); + }); +}); diff --git a/src/hooks/useDisposableMemo.ts b/src/hooks/useDisposableMemo.ts new file mode 100644 index 00000000..a9b03d6f --- /dev/null +++ b/src/hooks/useDisposableMemo.ts @@ -0,0 +1,111 @@ +import { useRef, useEffect, type DependencyList } from 'react'; + +const UNINITIALIZED = Symbol('UNINITIALIZED'); + +function depsEqual(a: DependencyList, b: DependencyList): boolean { + if (a.length !== b.length) return false; + for (let i = 0; i < a.length; i++) { + if (!Object.is(a[i], b[i])) return false; + } + return true; +} + +/** + * Like `useMemo`, but with a cleanup callback for disposable native resources. + * + * - Value is created synchronously during render (available on first render). + * - When deps change, the old value is cleaned up during render and a new one + * is created. + * - On unmount in production: cleaned up synchronously in effect cleanup. + * - On unmount in development: cleanup is deferred via `setTimeout(0)` so that + * fast refresh and Strict Mode can cancel it when effects re-run. + * + * Replaces the common `useMemo` + dispose-in-`useEffect`-cleanup pattern that + * breaks on fast refresh (HMR re-runs all effect cleanups, disposing the native + * object, but `useMemo` returns the same dead reference): + * + * ```tsx + * // BEFORE — breaks on fast refresh + * const property = useMemo(() => instance?.getProperty(path), [instance, path]); + * useEffect(() => { + * const unsub = property?.addListener(setValue); + * return () => { unsub?.(); property?.dispose(); }; + * }, [property]); + * + * // AFTER + * const property = useDisposableMemo( + * () => instance?.getProperty(path), + * (p) => p?.dispose(), + * [instance, path] + * ); + * useEffect(() => { + * const unsub = property?.addListener(setValue); + * return () => unsub?.(); // only unsubscribe, no dispose + * }, [property]); + * ``` + */ +export function useDisposableMemo( + factory: () => T, + cleanup: (value: T) => void, + deps: DependencyList +): T { + const ref = useRef<{ + value: T; + deps: DependencyList | typeof UNINITIALIZED; + pendingDisposal: ReturnType | null; + }>({ + value: undefined as T, + deps: UNINITIALIZED, + pendingDisposal: null, + }); + const cleanupRef = useRef(cleanup); + cleanupRef.current = cleanup; + + if ( + ref.current.deps === UNINITIALIZED || + !depsEqual(ref.current.deps, deps) + ) { + if (__DEV__ && ref.current.pendingDisposal !== null) { + clearTimeout(ref.current.pendingDisposal); + ref.current.pendingDisposal = null; + } + if (ref.current.deps !== UNINITIALIZED) { + try { + cleanupRef.current(ref.current.value); + } catch { + // Swallow cleanup errors — the old value is being replaced regardless. + } + } + ref.current = { value: factory(), deps, pendingDisposal: null }; + } + + useEffect(() => { + if (__DEV__) { + if (ref.current.pendingDisposal !== null) { + clearTimeout(ref.current.pendingDisposal); + ref.current.pendingDisposal = null; + } + } + return () => { + if (__DEV__) { + const val = ref.current.value; + ref.current.pendingDisposal = setTimeout(() => { + try { + cleanupRef.current(val); + } catch { + // Swallow — object may already be in a bad state. + } + ref.current.pendingDisposal = null; + }, 0); + } else { + try { + cleanupRef.current(ref.current.value); + } catch { + // Swallow — object may already be in a bad state. + } + } + }; + }, []); + + return ref.current.value; +} diff --git a/src/hooks/useRiveList.ts b/src/hooks/useRiveList.ts index 29d90225..f5c55b2a 100644 --- a/src/hooks/useRiveList.ts +++ b/src/hooks/useRiveList.ts @@ -3,6 +3,7 @@ import { useCallback, useEffect, useState, useMemo } from 'react'; import type { ViewModelInstance } from '../specs/ViewModel.nitro'; import type { UseRiveListResult } from '../types'; +import { useDisposableMemo } from './useDisposableMemo'; /** * Hook for interacting with list ViewModel instance properties. @@ -22,10 +23,14 @@ export function useRiveList( setError(null); }, [path, viewModelInstance]); - const property = useMemo(() => { - if (!viewModelInstance) return undefined; - return viewModelInstance.listProperty(path); - }, [viewModelInstance, path]); + const property = useDisposableMemo( + () => { + if (!viewModelInstance) return undefined; + return viewModelInstance.listProperty(path); + }, + (p) => p?.dispose(), + [viewModelInstance, path] + ); useEffect(() => { if (viewModelInstance && !property) { @@ -43,9 +48,13 @@ export function useRiveList( }); return () => { - removeListener(); - property.removeListeners(); - property.dispose(); + 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. + } }; }, [property]); diff --git a/src/hooks/useRiveProperty.ts b/src/hooks/useRiveProperty.ts index 6c13fc50..395e6b80 100644 --- a/src/hooks/useRiveProperty.ts +++ b/src/hooks/useRiveProperty.ts @@ -1,9 +1,10 @@ -import { useCallback, useEffect, useState, useMemo } from 'react'; +import { useCallback, useEffect, useState } from 'react'; import { type ObservableProperty, type ViewModelInstance, type ViewModelProperty, } from '../specs/ViewModel.nitro'; +import { useDisposableMemo } from './useDisposableMemo'; /** * Base hook for all ViewModelInstance property interactions. @@ -34,13 +35,17 @@ export function useRiveProperty

( Error | null, P | undefined, ] { - const property = useMemo(() => { - if (!viewModelInstance) return; - return options.getProperty( - viewModelInstance, - path - ) as unknown as ObservableViewModelProperty; - }, [options, viewModelInstance, path]); + const property = useDisposableMemo( + () => { + if (!viewModelInstance) return undefined; + return options.getProperty( + viewModelInstance, + path + ) as unknown as ObservableViewModelProperty; + }, + (p) => p?.dispose(), + [options, viewModelInstance, path] + ); // Always start undefined — the listener delivers the current value as its first emission. // (iOS experimental: via valueStream; iOS/Android legacy: emitted synchronously on subscribe) @@ -81,8 +86,12 @@ export function useRiveProperty

( }); return () => { - removeListener(); - property.dispose(); + try { + removeListener(); + } catch { + // Property may already be disposed by useDisposableMemo (deps change). + // Native dispose() handles listener cleanup, so this is safe to ignore. + } }; }, [options, property]); diff --git a/src/hooks/useViewModelInstance.ts b/src/hooks/useViewModelInstance.ts index 77227e9d..15631645 100644 --- a/src/hooks/useViewModelInstance.ts +++ b/src/hooks/useViewModelInstance.ts @@ -1,11 +1,12 @@ // TODO: migrate createInstance/createInstanceByName/etc to async equivalents /* eslint-disable @typescript-eslint/no-deprecated */ -import { useMemo, useEffect, useRef } from 'react'; +import { useMemo, useRef } from 'react'; import type { ViewModel, ViewModelInstance } from '../specs/ViewModel.nitro'; import type { RiveFile } from '../specs/RiveFile.nitro'; import type { RiveViewRef } from '../index'; import { callDispose } from '../core/callDispose'; import { ArtboardByName } from '../specs/ArtboardBy'; +import { useDisposableMemo } from './useDisposableMemo'; interface UseViewModelInstanceBaseParams { /** @@ -325,49 +326,30 @@ export function useViewModelInstance( const required = params?.required ?? false; const onInit = params?.onInit; - const prevInstanceRef = useRef<{ - instance: ViewModelInstance | null | undefined; - needsDispose: boolean; - } | null>(null); + const onInitRef = useRef(onInit); + onInitRef.current = onInit; - const result = useMemo(() => { - const created = createInstance( - source, - instanceName, - artboardName, - viewModelName, - useNew - ); - if (created.instance && onInit) { - onInit(created.instance); - } - return created; - // eslint-disable-next-line react-hooks/exhaustive-deps -- onInit excluded intentionally - }, [source, instanceName, artboardName, viewModelName, useNew]); - - // Dispose previous instance if it changed and needed disposal - if ( - prevInstanceRef.current && - prevInstanceRef.current.instance !== result.instance && - prevInstanceRef.current.needsDispose && - prevInstanceRef.current.instance - ) { - callDispose(prevInstanceRef.current.instance); - } - prevInstanceRef.current = result; - - // Cleanup on unmount - useEffect(() => { - return () => { - if ( - prevInstanceRef.current?.needsDispose && - prevInstanceRef.current.instance - ) { - callDispose(prevInstanceRef.current.instance); - prevInstanceRef.current = null; + const result = useDisposableMemo( + () => { + const created = createInstance( + source, + instanceName, + artboardName, + viewModelName, + useNew + ); + if (created.instance && onInitRef.current) { + onInitRef.current(created.instance); } - }; - }, []); + return created; + }, + (r) => { + if (r.needsDispose && r.instance) { + callDispose(r.instance); + } + }, + [source, instanceName, artboardName, viewModelName, useNew] + ); const error = useMemo( () => (result.error ? new Error(result.error) : null),