fix: Add check for onStart and improve valueSetter implementation

[MatiPl01]

Summary

This PR improves the valueSetter in terms of:

• code quality and readability (less nesting, better code separation),
• TS usage (removes multiple type casts),
• implementation safety (added check for onStart presence in the AnimationObject (see Crash in valueSetter: animation.onStart is undefined (useDerivedValue) #8974) and adds a check whether the value returned by the animation factory function is really an AnimationObject - before we just checked if a value is a function and assumed that the value it returns is AnimationObject without performing any additional checks)

[tjzel]

Also a question - I looked into the issue you linked about missing onStart - where do we ever have a path when an animation object has onFrame but doesn't have onStart?

[MatiPl01]

I don't know, I just added this check as it seemed to be reasonable. Maybe when someone assigns a custom function to the shared value which has onFrame field but not onStart but it is a very unlikely case.

[tjzel]

I think we should find it first to understand the issue instead of patching it with more if conditions.

[tjzel]

This code looks confusing. If someone assigned i.e. a number to a shared value, which invoked the value setter, we then create an empty object and check if it has onFrame or onStart properties.

How about something like

Suggested change

	const valueObject =
	typeof unwrappedValue === 'object' && unwrappedValue !== null
	? (unwrappedValue as Record<string, unknown>)
	: {};
	// Check if the value returned by the factory function is not an AnimationObject
	if (valueObject.onFrame === undefined || valueObject.onStart === undefined) {
	const isAnimationObject =
	typeof unwrappedValue === 'object' && unwrappedValue !== null && unwrappedValue.onFrame;
	if(!isAnimationObject)

[MatiPl01]

I wanted to avoid multiple type casts to Record<string, unknown>. Since we cannot use a type guard function, I decided to create a valueObject variable.

Unfortunately, TS won't allow use to access the onFrame prop on the unwrappedValue as long as its resolved type is just object.

[tjzel]

We have to excessively cast unfortunately. More TS-ish ways will hurt performance

[MatiPl01]

Can you take another look? I think it is more readable and doesn't overuse TS type casts.