davidjerleke / embla-carousel

A lightweight carousel library with fluid motion and great swipe precision.
https://www.embla-carousel.com
MIT License
5.39k stars 166 forks source link

Can't import `EmblaCaroouselType`, `EmblaOptionsType`, `EmblaPluginType` from `embla-carousel-react`, `embla-carousel-vue` and `embla-carousel-svelte` #647

Closed proninyaroslav closed 6 months ago

proninyaroslav commented 6 months ago

Bug is related to

Embla Carousel version

Describe the bug

Since version v8.0.0-rc15, ESLint says that the SliderPropsOptions type does not have a loop property. This happens to all properties that are in EmblaOptionsType:

image

Also because of this, useEmblaCarousel has the options argument type Partial<OptionsType> | undefined instead of EmblaOptionsType | undefined, which causes a linter error:

image

davidjerleke commented 6 months ago

Thanks @proninyaroslav,

I’ve noted this bug a couple of days ago and already started working on it. However, thanks for creating a bug report so I can connect it to a pull request and devs can track it 👍🏻.

Best, David

davidjerleke commented 6 months ago

@proninyaroslav a bug fix for this was just released in v8.0.0-rc16. Try it out and let me know if it's working as expected.

proninyaroslav commented 6 months ago

@davidjerleke Updated to v8.0.0-rc17 and still have this problem.

davidjerleke commented 6 months ago

@proninyaroslav then I suggest you create a CodeSandbox with your setup in order to demonstrate the problem. You didn’t provide one in your bug report but it’s a requirement as mentioned in the contribution guidelines.

davidjerleke commented 6 months ago

@proninyaroslav I think it's better to get the types directly from the main package. Because embla-carousel is a dependency, you can do this:

import { EmblaOptionsType } from 'embla-carousel'; // Get it like so
import useEmblaCarousel from 'embla-carousel-react';

I will remove the type re-exports from the library wrappers like embla-carousel-react, embla-carousel-vue etc. and will recommend the above demonstrated approach in the docs. This reduces the code complexity in the project significantly. This will be added to the docs here:

Best, David

proninyaroslav commented 6 months ago

@davidjerleke It works, thank you.

davidjerleke commented 6 months ago

Turns out, this approach doesn’t work with pnpm so I need to solve this differently. Stay tuned.

div-cowboy commented 6 months ago

@davidjerleke Please let us know when the pnpm solution is ready 🙏

Appreciate all you do

davidjerleke commented 5 months ago

@div-cowboy, @proninyaroslav. I've added examples of how to import types to the docs. There's also information about how to do it with pnpm:

[!IMPORTANT]
Remember to switch to the React tab if you're using embla-carousel-react etc.