FormidableLabs / nuka-carousel

Small, fast, and accessibility-first React carousel library with an easily customizable UI and behavior to fit your brand and site.
https://commerce.nearform.com/open-source/nuka-carousel
Other
3.06k stars 593 forks source link

[v6] Expand ref type, use React.forwardRef #969

Closed fritz-c closed 7 months ago

fritz-c commented 2 years ago

The prop innerRef does not allow the ref to point to a null value, even though that is the unavoidable initial value prior to first render. This will require library users to jump through some TypeScript hoops to make sure they can pass their ref to innerRef. https://github.com/FormidableLabs/nuka-carousel/blob/df842370ff3cc1b4f95d6171a2998c31a608da0d/packages/nuka/src/types.ts#L330

One option is to convert the innerRef prop to a native-to-React ref prop by wrapping the carousel function in React.forwardRef, which, given the right element type, will factor in the nullability of the ref.

We could alternatively retain the innerRef and just expand the type, so we would be free to set a collection of imperative APIs ( e.g., MyCarousel.nextSlide(), MyCarousel.goToSlide(), etc) to the native ref via useImperativeHandle, though I feel like our existing API gives enough flexibility that the extra API surface would only cause confusion.

This is a v6 issue because renaming the prop would break people's code, and expanding the type could, too. For example, TypeScript code that has no null checks before calling something like myCarouselInnerRef.current.getBoundingClientRect() would suddenly start to get a type error about getBoundingClientRect not existing on null.