-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Move validateAndUnwrapRefs to the animations hook level #275
Comments
The Function signaturefunction validateRefsForAnimationFactory<T extends Refs>(
refs: T,
animation: (refs: NonNullableRecord<UnwrapRefs<T>>) => gsap.core.Animation,
): () => gsap.core.Animation; Usagetype MyRefs = {
root: HTMLDivElement;
};
function createInAnimation(refs: MyRefs): gsap.core.Animation {
return gsap.to(refs.root, { opacity: 1 });
}
function MyComponent(): ReactElement {
const refs = useRefs<MyRefs>();
useAnimation(validateRefsForAnimationFactory(refs, createInAnimation), []);
return <div ref={ref.root}>Root</div>;
} |
With the above suggestion it's still possible that a refs object with unused fields still passes the validation, we must add initial values for all fields to prevent this from happening. |
Narrowing of types is improved in TypeScript 5.4 so we can probably do this: type MyRefs = {
root: HTMLDivElement | null;
rootTwo?: HTMLDivElement;
};
const requiredRefs = ['root'] satisfies RequiredRefs<MyRefs>
function createInAnimation(refs: MutableRefs<MyRefs>) {
if (!validateRefs(refs, requiredRefs)) {
return;
}
const { root } = unwrapRefs(refs);
return gsap.timeline().to(root, { opacity: 1 });
}
function createOutAnimation(refs: MutableRefs<MyRefs>) {
if (!validateRefs(refs, requiredRefs)) {
return;
}
const { root } = unwrapRefs(refs);
return gsap.timeline().to(root, { opacity: 1 });
}
function MyComponent(): ReactElement {
const refs = useRefs<MyRefs>({
root: null
});
useAnimation(() => createInAnimation(refs), []);
useBeforeUnmount(() => createInAnimation(refs).reverse(), []);
return <div ref={ref.root}>Root</div>;
} Potential abstraction that can be implemented on top of the approach described above: type MyRefs = {
root: HTMLDivElement | null;
rootTwo?: HTMLDivElement;
};
function createInAnimation({ root }: MyRefs) {
return gsap.timeline().to(root, { opacity: 1 });
}
function createOutAnimation({ root }: MyRefs) {
return gsap.timeline().to(root, { opacity: 1 });
}
function MyComponent(): ReactElement {
const { refs, inAnimation } = useAnimations<MyRefs>({
inAnimation: createInAnimation,
outAnimation: createOutAnimation,
initialRefs: { root: null },
requiredFieldNames: ['root'],
dependencies: []
});
return <div ref={refs.root}>Root</div>;
} You cannot add additional parameters in the abstraction. |
Last month we were introduced to validateAndUnwrapRefs, a hook available in the repo that allow us to validate and unwrap refs.
While this hook prevents some past issues (ex: adding non-available refs to tweens), it also creates an extra condition across all the animation methods. Here's an example of the previous implementation and the new one:
At first glance this doesn't look like much but on projects that rely on animation this can stack up. In my opinion the methods created in the *.animations.ts should focus more on the timeline tweens and less on checking if elements are available, so I propose that this validation is taken care at the hooks level.
I think the animation method should not be executed in case validations pass. We can have a parameter to make those validations optional, for cases where we want to handle the validation at the animation method level.
Would love to hear some opinions and solutions to make this implementation smoother.
The text was updated successfully, but these errors were encountered: