Skip to content
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

Open
SinesioMM opened this issue Nov 28, 2023 · 3 comments
Open

Move validateAndUnwrapRefs to the animations hook level #275

SinesioMM opened this issue Nov 28, 2023 · 3 comments

Comments

@SinesioMM
Copy link

SinesioMM commented Nov 28, 2023

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:

export function createInAnimation(refs: ComponentRefs): gsap.core.Animation {
  const { self } = assertAndUnwrapRefs(refs);
  const timeline = gsap.timeline();
  
  timeline.add(fadeFromTo(self));

  return timeline;
}
export function createInAnimation(refs: ComponentRefs): gsap.core.Animation {
  const [isValid, unwrappedRefs] = validateAndUnwrapRefs(refs);
  const timeline = gsap.timeline();

  if(isValid) {
    const { self } = unwrappedRefs;
    timeline.add(fadeFromTo(self));
  }

  return timeline;
}

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.

@leroykorterink
Copy link
Collaborator

leroykorterink commented Nov 28, 2023

The useAnimation accepts any function that returns a GSAP timeline, I don't want to force the use of useRefs with useAnimation. Instead, we can create a higher order function that does the validation for the function that returns the gsap.core.Animation.

Function signature

function validateRefsForAnimationFactory<T extends Refs>(
  refs: T,
  animation: (refs: NonNullableRecord<UnwrapRefs<T>>) => gsap.core.Animation,
): () => gsap.core.Animation;

Usage

type 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>;
}

@leroykorterink
Copy link
Collaborator

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.

@leroykorterink
Copy link
Collaborator

leroykorterink commented Feb 16, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants