yaodingyd / react-flickity-component

A React.js component for using @desandro's Flickity
314 stars 51 forks source link

Wrong type definition of flickityRef #68

Closed Profesor08 closed 5 years ago

Profesor08 commented 5 years ago

In index.d.ts type of flickityRef is HTMLDivElement. It is wrong. It makes troubles on binding events or something else, using refference.

<Flickity flickityRef={ref => (this.flkty = ref)} />
...
this.flkty.on("select", () => {
  console.log(`current index is ${this.flkty.selectedIndex}`);
});

Property 'on' does not exist on type 'HTMLDivElement'.

DonGissel commented 5 years ago

This gets very annoying when using the module in TypeScript. Please change the reference type to Flickity instead. 😄

andyfangaf commented 5 years ago

👍

theolampert commented 5 years ago

@DonGissel thanks for the catch, will update and make a release this week.

theolampert commented 5 years ago

@DonGissel @Profesor08 Can one of you two please verify if #74 fixes the issue for you?

DonGissel commented 5 years ago

I'm currently trying to debug the thing, but unfortunately, TypeScript insights aren't too happy. Since the returned scope is basically just a reference to the "local" Flickity (in this case the React-wrapper), as opposed to the one actually describing the Flickity module, the methods available on Flickity still aren't exposed correctly.

Sadly my knowledge of TS isn't such that I can figure out how to restructure the code to keep the React-definitions in place while including the definitions for the "real" Flickty, but it might be something to do with piggybacking onto @types/flickity. Either way: it doesn't work as expected; at least not yet. ;)

I'll have another go at it when I have more time.

theolampert commented 5 years ago

@DonGissel Thanks for the feedback, I probably jumped the gun a bit pushing it into the last release. I'll need to test it properly and take another look.