Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 63 additions & 0 deletions src/hooks/useStateMachineInputs.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
import { EventType, StateMachineInput, Rive } from '@rive-app/canvas';
import { useCallback, useEffect, useMemo, useState } from 'react';

/**
* Custom hook for fetching multiple stateMachine inputs from a rive file.
* Particularly useful for fetching multiple inputs from a variable number of input names.
*
* @param rive - Rive instance
* @param stateMachineName - Name of the state machine
* @param inputNames - Name and initial value of the inputs
* @returns StateMachineInput[]
*/
export default function useStateMachineInputs(
rive: Rive | null,
stateMachineName?: string,
inputNames?: {
name: string;
initialValue?: number | boolean;
}[]
) {
const [inputMap, setInputMap] = useState<Map<string, StateMachineInput>>(new Map());
Copy link
Copy Markdown
Contributor

@bodymovin bodymovin Nov 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a reason why the map needs to be stored in the state instead of the array?
It's great that it is being used as an intermediate store while building the inputs for performance, but that seems like an implementation detail of syncInputs.
In the end it's exposed as an array and It forces to use the useMemo hook.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope not at all anymore.
It was a Map for the initial implementation I planned, which is returning the Map instead of an Array. I thought it would be a better implementation as the user gets to choose which input they want to use, without having to map over the whole array again to find it.
But I realized that could be handled by preprocessing the inputNames array passed to the hook. ex) filtering out the unnecessary inputNames before calling the hook.

So yeah, there's no reason for it to be a Map instead of an Array now.
I'll change that! Thanks for pointing it out!


const syncInputs = useCallback(() => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

out of curiosity, in the previous implementation this method was defined in the useEffect hook directly. Was there a reason for moving it outside?
As far as I know, it's a good practice to declare methods that are only used in a useEffect, inside the useEffect hook itself. It helps with encapsulation. And in this particular case doesn't need the useCallback hook.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's true that declaring functions that are only used in a useEffect within the hook is helpful for encapsulation, but I think it kind of hurts the readability of the code in many cases.
I personally like to leave the useEffect hook as concise as possible so that I can understand the flow of the useEffects quickly without scrolling up and down too much.
(Besides, I find useEffects to be where the majority of unexpected bugs occur, so I try to keep them easy to understand in most cases.)
Since there's just one useEffect in this case, and since the function gets recreated whenever the useEffect runs (as they have the same dependency array), I don't really mind whether the function is defined inside or outside the hook.

If putting functions within useEffects is the convention Rive follows, I'm more than happy to move it back into the useEffect! Let me know what you think!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hey! Sorry about the delay. Yes it'd be good to have the method inside the useEffect itself.
We're building a similar API for data binding, and we'll take this approach as well.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0fc363e Done!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

if (!rive || !stateMachineName || !inputNames) return;

const riveInputs = rive.stateMachineInputs(stateMachineName);
if (!riveInputs) return;

// To optimize lookup time from O(n) to O(1) in the following loop
const riveInputLookup = new Map<string, StateMachineInput>(riveInputs.map(input => [input.name, input]));

setInputMap(() => {
const newMap = new Map<string, StateMachineInput>();

// Iterate over inputNames instead of riveInputs to preserve array order
inputNames.forEach(inputName => {
const riveInput = riveInputLookup.get(inputName.name);
if (!riveInput) return;

if (inputName.initialValue !== undefined) {
riveInput.value = inputName.initialValue;
}

newMap.set(inputName.name, riveInput);
});

return newMap;
});
}, [inputNames, rive, stateMachineName]);

useEffect(() => {
syncInputs();
if (rive) {
rive.on(EventType.Load, syncInputs);

return () => {
rive.off(EventType.Load, syncInputs);
};
}
}, [rive, stateMachineName, inputNames, syncInputs]);

return useMemo(() => Array.from(inputMap.values()), [inputMap]);
}
18 changes: 16 additions & 2 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,22 @@ import useRive from './hooks/useRive';
import useStateMachineInput from './hooks/useStateMachineInput';
import useResizeCanvas from './hooks/useResizeCanvas';
import useRiveFile from './hooks/useRiveFile';
import useStateMachineInputs from './hooks/useStateMachineInputs';

export default Rive;
export { useRive, useStateMachineInput, useResizeCanvas, useRiveFile , RiveProps };
export { RiveState, UseRiveParameters, UseRiveFileParameters, UseRiveOptions } from './types';
export {
useRive,
useStateMachineInput,
useStateMachineInputs,
useResizeCanvas,
useRiveFile,
RiveProps,
};
export {
RiveState,
UseRiveParameters,
UseRiveFileParameters,
UseRiveOptions,
} from './types';

export * from '@rive-app/canvas';