mediamonks / react-kit

A collection of React hooks, components and utilities we use at Media.Monks
https://mediamonks.github.io/react-kit/
MIT License
11 stars 2 forks source link

Move validateAndUnwrapRefs to the animations hook level #275

Open SinesioMM opened 1 year ago

SinesioMM commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 9 months ago

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.