diff --git a/example/__tests__/use-rive-trigger.harness.tsx b/example/__tests__/use-rive-trigger.harness.tsx new file mode 100644 index 00000000..88659383 --- /dev/null +++ b/example/__tests__/use-rive-trigger.harness.tsx @@ -0,0 +1,214 @@ +import { + describe, + it, + expect, + render, + waitFor, + cleanup, +} from 'react-native-harness'; +import { useEffect, useState } from 'react'; +import { View } from 'react-native'; +import { + Fit, + RiveFileFactory, + RiveView, + useRiveTrigger, + type RiveFile, +} from '@rive-app/react-native'; +import type { ViewModelInstance } from '@rive-app/react-native'; + +const DATABINDING = require('../assets/rive/databinding.riv'); + +function expectDefined(value: T): asserts value is NonNullable { + expect(value).toBeDefined(); +} + +async function loadGordonInstance() { + const file = await RiveFileFactory.fromSource(DATABINDING, undefined); + const vm = file.viewModelByName('Person'); + expectDefined(vm); + const instance = vm.createInstanceByName('Gordon'); + expectDefined(instance); + return { file, instance }; +} + +// ─── Test context types ──────────────────────────────────────────── + +type TriggerContext = { + triggerCount: number; + triggerFn: (() => void) | null; + error: Error | null; + renderCount: number; +}; + +function createTriggerContext(): TriggerContext { + return { triggerCount: 0, triggerFn: null, error: null, renderCount: 0 }; +} + +// ─── Test component: stable callback ─────────────────────────────── +// RiveView with dataBind is required so the Rive render loop runs +// and pollChanges() dispatches trigger events to listeners. + +function StableTriggerComponent({ + file, + instance, + context, +}: { + file: RiveFile; + instance: ViewModelInstance; + context: TriggerContext; +}) { + context.renderCount++; + + const { trigger, error } = useRiveTrigger('jump', instance, { + onTrigger: () => { + context.triggerCount++; + }, + }); + + useEffect(() => { + context.triggerFn = trigger; + context.error = error; + }, [context, trigger, error]); + + return ( + + + + ); +} + +// ─── Test component: unstable callback (issue #230) ──────────────── + +function UnstableTriggerComponent({ + file, + instance, + context, +}: { + file: RiveFile; + instance: ViewModelInstance; + context: TriggerContext; +}) { + 'use no memo'; + + const [, setTick] = useState(0); + context.renderCount++; + + const onTrigger = () => { + context.triggerCount++; + }; + + const { trigger, error } = useRiveTrigger('jump', instance, { onTrigger }); + + useEffect(() => { + context.triggerFn = trigger; + context.error = error; + }, [context, trigger, error]); + + // Force re-renders to change callback identity + useEffect(() => { + const interval = setInterval(() => setTick((t) => t + 1), 50); + const timeout = setTimeout(() => clearInterval(interval), 300); + return () => { + clearInterval(interval); + clearTimeout(timeout); + }; + }, []); + + return ( + + + + ); +} + +// ─── Tests ───────────────────────────────────────────────────────── + +describe('useRiveTrigger hook', () => { + it('receives trigger events from JS trigger()', async () => { + const { file, instance } = await loadGordonInstance(); + const context = createTriggerContext(); + + await render( + + ); + + await waitFor( + () => { + expect(context.triggerFn).not.toBeNull(); + }, + { timeout: 3000 } + ); + + expect(context.error).toBeNull(); + + // Fire trigger and wait for it — pollChanges() runs on frame ticks, + // so we wait for each trigger individually to avoid coalescing. + context.triggerFn!(); + + await waitFor( + () => { + expect(context.triggerCount).toBeGreaterThanOrEqual(1); + }, + { timeout: 5000 } + ); + + cleanup(); + }); + + it('receives triggers with unstable callback after re-renders (#230)', async () => { + const { file, instance } = await loadGordonInstance(); + const context = createTriggerContext(); + + await render( + + ); + + // Wait for the re-render burst to complete (300ms of re-renders every 50ms) + await waitFor( + () => { + expect(context.renderCount).toBeGreaterThanOrEqual(3); + }, + { timeout: 2000 } + ); + + await waitFor( + () => { + expect(context.triggerFn).not.toBeNull(); + }, + { timeout: 3000 } + ); + + expect(context.error).toBeNull(); + + // Fire trigger AFTER the re-render burst — before the fix, this was lost + context.triggerFn!(); + + await waitFor( + () => { + expect(context.triggerCount).toBeGreaterThanOrEqual(1); + }, + { timeout: 5000 } + ); + + cleanup(); + }); +}); diff --git a/src/hooks/__tests__/useRiveProperty.test.ts b/src/hooks/__tests__/useRiveProperty.test.ts index 7d280d8b..70264d45 100644 --- a/src/hooks/__tests__/useRiveProperty.test.ts +++ b/src/hooks/__tests__/useRiveProperty.test.ts @@ -48,9 +48,11 @@ describe('useRiveProperty', () => { }); const { result } = renderHook(() => - useRiveProperty(mockInstance, 'favDrink/type', { - getProperty: (vmi, path) => (vmi as any).enumProperty(path), - }) + useRiveProperty( + mockInstance, + 'favDrink/type', + (vmi: any, path: string) => vmi.enumProperty(path) + ) ); // The mock's addListener emits 'Tea' synchronously — React batches it with the @@ -66,9 +68,11 @@ describe('useRiveProperty', () => { }); const { result } = renderHook(() => - useRiveProperty(mockInstance, 'favDrink/type', { - getProperty: (vmi, path) => (vmi as any).enumProperty(path), - }) + useRiveProperty( + mockInstance, + 'favDrink/type', + (vmi: any, path: string) => vmi.enumProperty(path) + ) ); act(() => { @@ -81,9 +85,11 @@ describe('useRiveProperty', () => { it('should return undefined when viewModelInstance is null', () => { const { result } = renderHook(() => - useRiveProperty(null, 'favDrink/type', { - getProperty: (vmi, path) => (vmi as any).enumProperty(path), - }) + useRiveProperty( + null, + 'favDrink/type', + (vmi: any, path: string) => vmi.enumProperty(path) + ) ); const [value] = result.current; @@ -94,9 +100,11 @@ describe('useRiveProperty', () => { const mockInstance = createMockViewModelInstance({}); const { result } = renderHook(() => - useRiveProperty(mockInstance, 'nonexistent/path', { - getProperty: (vmi, path) => (vmi as any).enumProperty(path), - }) + useRiveProperty( + mockInstance, + 'nonexistent/path', + (vmi: any, path: string) => vmi.enumProperty(path) + ) ); const [, , error] = result.current; @@ -108,9 +116,11 @@ describe('useRiveProperty', () => { const mockInstance = createMockViewModelInstance({}); const { result } = renderHook(() => - useRiveProperty(mockInstance, 'nonexistent/path', { - getProperty: (vmi, path) => (vmi as any).enumProperty(path), - }) + useRiveProperty( + mockInstance, + 'nonexistent/path', + (vmi: any, path: string) => vmi.enumProperty(path) + ) ); // Error already set by useEffect (property not found on valid instance) @@ -131,9 +141,11 @@ describe('useRiveProperty', () => { // Start with undefined instance (simulates async file loading) const { result } = renderHook( (props: { instance: ViewModelInstance | undefined }) => - useRiveProperty(props.instance, 'text', { - getProperty: (vmi, path) => (vmi as any).stringProperty(path), - }), + useRiveProperty( + props.instance, + 'text', + (vmi: any, path: string) => vmi.stringProperty(path) + ), { initialProps: { instance: undefined } } ); @@ -156,9 +168,11 @@ describe('useRiveProperty', () => { // Start with undefined instance const { result, rerender } = renderHook( (props: { instance: ViewModelInstance | undefined }) => - useRiveProperty(props.instance, 'text', { - getProperty: (vmi, path) => (vmi as any).stringProperty(path), - }), + useRiveProperty( + props.instance, + 'text', + (vmi: any, path: string) => vmi.stringProperty(path) + ), { initialProps: { instance: undefined } } ); @@ -197,9 +211,11 @@ describe('useRiveProperty', () => { const { result, rerender } = renderHook( (props: { path: string }) => - useRiveProperty(mockInstance, props.path, { - getProperty: (vmi, p) => (vmi as any).enumProperty(p), - }), + useRiveProperty( + mockInstance, + props.path, + (vmi: any, p: string) => vmi.enumProperty(p) + ), { initialProps: { path: 'drinks/tea' } } ); @@ -222,9 +238,11 @@ describe('useRiveProperty', () => { const { result, rerender } = renderHook( (props: { instance: ViewModelInstance }) => - useRiveProperty(props.instance, 'prop/path', { - getProperty: (vmi, p) => (vmi as any).enumProperty(p), - }), + useRiveProperty( + props.instance, + 'prop/path', + (vmi: any, p: string) => vmi.enumProperty(p) + ), { initialProps: { instance: mockInstance1 } } ); diff --git a/src/hooks/useRiveBoolean.ts b/src/hooks/useRiveBoolean.ts index 42c9ad57..657d9ad6 100644 --- a/src/hooks/useRiveBoolean.ts +++ b/src/hooks/useRiveBoolean.ts @@ -5,9 +5,8 @@ import { import type { UseRivePropertyResult } from '../types'; import { useRiveProperty } from './useRiveProperty'; -const BOOLEAN_PROPERTY_OPTIONS = { - getProperty: (vmi: ViewModelInstance, p: string) => vmi.booleanProperty(p), -}; +const getBooleanProperty = (vmi: ViewModelInstance, p: string) => + vmi.booleanProperty(p); /** * Hook for interacting with boolean ViewModel instance properties. @@ -23,6 +22,6 @@ export function useRiveBoolean( const [value, setValue, error] = useRiveProperty< ViewModelBooleanProperty, boolean - >(viewModelInstance, path, BOOLEAN_PROPERTY_OPTIONS); + >(viewModelInstance, path, getBooleanProperty); return { value, setValue, error }; } diff --git a/src/hooks/useRiveColor.ts b/src/hooks/useRiveColor.ts index e454bb69..bd6ce38e 100644 --- a/src/hooks/useRiveColor.ts +++ b/src/hooks/useRiveColor.ts @@ -6,9 +6,8 @@ import type { import { useRiveProperty } from './useRiveProperty'; import { RiveColor } from '../core/RiveColor'; -const COLOR_PROPERTY_OPTIONS = { - getProperty: (vmi: ViewModelInstance, p: string) => vmi.colorProperty(p), -}; +const getColorProperty = (vmi: ViewModelInstance, p: string) => + vmi.colorProperty(p); export interface UseRiveColorResult { value: RiveColor | undefined; @@ -30,7 +29,7 @@ export function useRiveColor( const [rawValue, setRawValue, error] = useRiveProperty< ViewModelColorProperty, number - >(viewModelInstance, path, COLOR_PROPERTY_OPTIONS); + >(viewModelInstance, path, getColorProperty); const value = rawValue !== undefined ? RiveColor.fromInt(rawValue) : undefined; diff --git a/src/hooks/useRiveEnum.ts b/src/hooks/useRiveEnum.ts index 12f39b15..ee5d67ba 100644 --- a/src/hooks/useRiveEnum.ts +++ b/src/hooks/useRiveEnum.ts @@ -5,9 +5,8 @@ import { import type { UseRivePropertyResult } from '../types'; import { useRiveProperty } from './useRiveProperty'; -const ENUM_PROPERTY_OPTIONS = { - getProperty: (vmi: ViewModelInstance, p: string) => vmi.enumProperty(p), -}; +const getEnumProperty = (vmi: ViewModelInstance, p: string) => + vmi.enumProperty(p); /** * Hook for interacting with enum ViewModel instance properties. @@ -23,6 +22,6 @@ export function useRiveEnum( const [value, setValue, error] = useRiveProperty< ViewModelEnumProperty, string - >(viewModelInstance, path, ENUM_PROPERTY_OPTIONS); + >(viewModelInstance, path, getEnumProperty); return { value, setValue, error }; } diff --git a/src/hooks/useRiveNumber.ts b/src/hooks/useRiveNumber.ts index 1e06979f..773d30e2 100644 --- a/src/hooks/useRiveNumber.ts +++ b/src/hooks/useRiveNumber.ts @@ -5,9 +5,8 @@ import { import type { UseRivePropertyResult } from '../types'; import { useRiveProperty } from './useRiveProperty'; -const NUMBER_PROPERTY_OPTIONS = { - getProperty: (vmi: ViewModelInstance, p: string) => vmi.numberProperty(p), -}; +const getNumberProperty = (vmi: ViewModelInstance, p: string) => + vmi.numberProperty(p); /** * Hook for interacting with number ViewModel instance properties. @@ -23,6 +22,6 @@ export function useRiveNumber( const [value, setValue, error] = useRiveProperty< ViewModelNumberProperty, number - >(viewModelInstance, path, NUMBER_PROPERTY_OPTIONS); + >(viewModelInstance, path, getNumberProperty); return { value, setValue, error }; } diff --git a/src/hooks/useRiveProperty.ts b/src/hooks/useRiveProperty.ts index 04e2e130..d6baecd2 100644 --- a/src/hooks/useRiveProperty.ts +++ b/src/hooks/useRiveProperty.ts @@ -7,28 +7,24 @@ import { import { useDisposableMemo } from './useDisposableMemo'; /** - * Base hook for all ViewModelInstance property interactions. - * This hook provides a unified interface for working with different types of - * Rive properties (boolean, number, string, enum, trigger) while maintaining - * type safety and proper cleanup. + * Base hook for all ViewModelInstance value-property interactions + * (number, string, boolean, color, enum). * - * @template P - The type of the property (e.g., ViewModelBooleanProperty, ViewModelNumberProperty) + * Not used for triggers — see {@link useRiveTrigger} which manages its own + * property lifecycle to avoid coupling callback identity to native disposal. + * + * @template P - The type of the property (e.g., ViewModelBooleanProperty) * @template T - The primitive type of the property value (number, boolean, string) * * @param viewModelInstance - The source ViewModelInstance * @param path - Property path in the ViewModelInstance - * @param options - Configuration for working with the property + * @param getProperty - Function to get the property from a ViewModelInstance * @returns A tuple containing [value, setter, error, property] */ export function useRiveProperty

( viewModelInstance: ViewModelInstance | null | undefined, path: string, - options: { - /** Function to get the property from a ViewModelInstance */ - getProperty: (vm: ViewModelInstance, path: string) => P | undefined; - /** Optional override callback for property events (mainly used by triggers) */ - onPropertyEventOverride?: (...args: any[]) => void; - } + getProperty: (vm: ViewModelInstance, path: string) => P | undefined ): [ T | undefined, (value: T | ((prevValue: T | undefined) => T)) => void, @@ -38,13 +34,13 @@ export function useRiveProperty

( const property = useDisposableMemo( () => { if (!viewModelInstance) return undefined; - return options.getProperty( + return getProperty( viewModelInstance, path ) as unknown as ObservableViewModelProperty; }, (p) => p?.dispose(), - [options, viewModelInstance, path] + [viewModelInstance, path] ); // Always start undefined — the listener delivers the current value as its first emission. @@ -75,15 +71,11 @@ export function useRiveProperty

( // undefined → value without waiting for a property change. // (Legacy addListener does NOT emit on subscribe — only on changes. // Experimental valueStream emits the current value as its first element.) - if (!options.onPropertyEventOverride) { - setValue(property.value); - } + setValue(property.value); - const removeListener = options.onPropertyEventOverride - ? property.addListener(options.onPropertyEventOverride) - : property.addListener((newValue) => { - setValue(newValue); - }); + const removeListener = property.addListener((newValue) => { + setValue(newValue); + }); return () => { try { @@ -93,7 +85,7 @@ export function useRiveProperty

( // Native dispose() handles listener cleanup, so this is safe to ignore. } }; - }, [options, property]); + }, [property]); // Set the value of the property (no-op if property isn't available yet). // Uses tracked `value` from state for updater functions — avoids a synchronous diff --git a/src/hooks/useRiveString.ts b/src/hooks/useRiveString.ts index 16216c8b..e182ddff 100644 --- a/src/hooks/useRiveString.ts +++ b/src/hooks/useRiveString.ts @@ -5,9 +5,8 @@ import { import type { UseRivePropertyResult } from '../types'; import { useRiveProperty } from './useRiveProperty'; -const STRING_PROPERTY_OPTIONS = { - getProperty: (vmi: ViewModelInstance, p: string) => vmi.stringProperty(p), -}; +const getStringProperty = (vmi: ViewModelInstance, p: string) => + vmi.stringProperty(p); /** * Hook for interacting with string ViewModel instance properties. @@ -23,6 +22,6 @@ export function useRiveString( const [value, setValue, error] = useRiveProperty< ViewModelStringProperty, string - >(viewModelInstance, path, STRING_PROPERTY_OPTIONS); + >(viewModelInstance, path, getStringProperty); return { value, setValue, error }; } diff --git a/src/hooks/useRiveTrigger.ts b/src/hooks/useRiveTrigger.ts index 4a478006..57c82327 100644 --- a/src/hooks/useRiveTrigger.ts +++ b/src/hooks/useRiveTrigger.ts @@ -1,23 +1,22 @@ -import { useCallback, useMemo } from 'react'; -import { - type ViewModelInstance, - type ViewModelTriggerProperty, -} from '../specs/ViewModel.nitro'; +import { useCallback, useEffect, useRef, useState } from 'react'; +import { type ViewModelInstance } from '../specs/ViewModel.nitro'; import type { UseRiveTriggerResult, UseViewModelInstanceTriggerParameters, } from '../types'; -import { useRiveProperty } from './useRiveProperty'; - -const getTriggerProperty = (vmi: ViewModelInstance, p: string) => - vmi.triggerProperty(p); +import { useDisposableMemo } from './useDisposableMemo'; /** * Hook for interacting with trigger ViewModel instance properties. * + * Manages its own property lifecycle (separate from useRiveProperty) because + * triggers take a user callback whose identity may change across renders. + * Storing the callback in a ref avoids coupling it to native property disposal. + * * @param path - The path to the trigger property - * @param viewModelInstance - The ViewModelInstance containing the trigger property to operate on - * @returns A trigger function that can be called to fire the trigger + * @param viewModelInstance - The ViewModelInstance containing the trigger property + * @param params - Optional parameters including onTrigger callback + * @returns A trigger function and any error */ export function useRiveTrigger( path: string, @@ -26,18 +25,47 @@ export function useRiveTrigger( ): UseRiveTriggerResult { const { onTrigger } = params ?? {}; - const triggerOptions = useMemo( - () => ({ - getProperty: getTriggerProperty, - onPropertyEventOverride: onTrigger, - }), - [onTrigger] + const onTriggerRef = useRef(onTrigger); + onTriggerRef.current = onTrigger; + + const property = useDisposableMemo( + () => { + if (!viewModelInstance) return undefined; + return viewModelInstance.triggerProperty(path); + }, + (p) => p?.dispose(), + [viewModelInstance, path] ); - const [_, __, error, property] = useRiveProperty< - ViewModelTriggerProperty, - undefined - >(viewModelInstance, path, triggerOptions); + const [error, setError] = useState(null); + + useEffect(() => { + setError(null); + }, [path, viewModelInstance]); + + useEffect(() => { + if (viewModelInstance && !property) { + setError( + new Error(`Property "${path}" not found in the ViewModel instance`) + ); + } + }, [viewModelInstance, property, path]); + + useEffect(() => { + if (!property) return; + + const removeListener = property.addListener(() => { + onTriggerRef.current?.(); + }); + + return () => { + try { + removeListener(); + } catch { + // Property may already be disposed by useDisposableMemo (deps change). + } + }; + }, [property]); const trigger = useCallback(() => { if (property) {