mifi / react-lottie-player

Fully declarative React Lottie player
MIT License
505 stars 53 forks source link

Export LottieProps #67

Closed kb-ig closed 2 years ago

kb-ig commented 2 years ago

How would you feel about exporting the LottieProps type?

I think it would be handy in scenarios where we may want to offload some logic that determine what props to pass in to the player so that it's outside of the main component that renders the player. As an over-simplified example:

const getLottieProps = (componentProps: MyProps): LottieProps => {
  // Some logic that determines the props
  const play = true;
  const goTo = 123;
  return { play, goTo };
}

const MyComponent = (props: MyProps) => {
  return (
    <div className='my-animation'>
      <AnimationHeader/>
        <Lottie {...getLottieProps(props)}/>
      <AnimationFooter/>
    </div>
    );
}
mifi commented 2 years ago

Hi. I think this can be solved in the containing component by using state for these props and then calling setState when the props value is known?

kb-ig commented 2 years ago

Yeah, that's one option. Although sometimes its desirable to have a pure stateless component.

There's definitely a few options to get around the example I gave, but I feel it would be beneficial to allow consumers of this library to access the LottieProps type. I dont really see any reason to keep it private.

Another use case would be extending the type. Perhaps my component is a simple wrapper around the lottie player component and I need a few extra properties, eg.

  type MyProps = LottieProps & {
    myHeaderText: string;
    myFooterText: string
  };

  const MyComponent = (props: MyProps) => {
    return (
      <div className='my-animation'>
        <AnimationHeader text={props.myHeaderText}/>
          <Lottie {...props}/>
        <AnimationFooter text={props.myFooterText}/>
      </div>
    );
  }
mifi commented 2 years ago

Oops sorry, I think I misread. You're only talking about a typescript change here, right? TBH I'm not so proficient in TS, but if it's just a change to index.d.ts that doesn't break anything, then I'm open for a PR!

mifi commented 2 years ago

i think your issue might be a duplicate of #46 ?

kb-ig commented 2 years ago

Yeah, just a little TS change - should be fairly unobtrusive.

My ticket is indeed a dup, sorry about that. I'll close this one off and I'll have a look at opening a PR for that change this week.