chore(carousel): update embla-carousel package

[Xxsource98]

I updated the 'embla-carousel' package because when using 'npx shadcn-ui@latest add carousel,' it downloads the latest 'embla-carousel' package which no longer exports types. It was necessary to use the base 'embla-carousel' package to obtain types like 'EmblaCarouselType,' 'EmblaOptionsType,' and 'EmblaPluginType.'

[vercel[bot]]

@Xxsource98 is attempting to deploy a commit to the shadcn-pro Team on Vercel.

A member of the Team first needs to authorize it.

[shadcn]

@Xxsource98 Thanks so much for the PR.

I'm taking a look at @davidjerleke suggestions on https://github.com/davidjerleke/embla-carousel/pull/658#issuecomment-1877082410 and the issue with pnpm.

I wonder if we should go with a devDependency instead. We'll list embla-carousel as a devDependency and update the cli to install it for you.

What do you think?

[davidjerleke]

Hello from the Embla side!

My two cents after my conversation with @zaaakher and his efforts:

embla-carousel is technically already installed behind the scene because embla-carousel-react depends on it but pnpm automatically hides it in a /node_modules/.pnpm and makes it inaccessible to me to import the types from.

This package manager behavior is clearly different from npm and yarn and I think that the pnpm creators knew this or should have known that this could cause problems in some cases. There’s always a cost to introducing a new/different behavior to a fundamental thing like a frontend package manager because the devs using it will pay the price. I hope the pros of pnpm makes it worth the effort.

In my opinion, installing embla-carousel as a devDependency makes most sense because it’s only needed during development. This is similar to installing a type package that belongs to a library/framework like @types/react.

However, if you guys have any other ideas on how to solve this, please share.

Best, David

[zaaakher]

I started a discussion here to clue the pnpm maintainers in with us. Hopefully they'll have the best insight in this matter.

[Xxsource98]

Hi, in my opinion, the best approach right now is using devDependency. However, CLI needs to be updated for download dev dependencies for components.

[Xxsource98]

I jus released v8.0.0-rc19 which includes another bug fix. Since this PR isn’t merged yet, how about bumping all Embla versions to v8.0.0-rc19 instead?

I just did it :)

[zaaakher]

Just for closure,

pnpm making dependencies of dependencies inaccessible is actually a feature of pnpm as mentioned here

[shadcn]

@Xxsource98 wdyt about the fix from https://github.com/shadcn-ui/ui/issues/2281 (deriving the types from UseEmblaCarouselType)?

Update: I think we still need to support adding devDependencies from the cli though. Going to fix this.

[Xxsource98]

@Xxsource98 wdyt about the fix from #2281 (deriving the types from UseEmblaCarouselType)?

Update: I think we still need to support adding devDependencies from the cli though. Going to fix this.

Hi, sorry for late answer. Yeah it should be enough. Or maybe just mention it in docs and wait until devDependencies will be available to download from cli?

[huri3l]

Hello gentlemen!

I found a bug in the embla-carousel package and @davidjerleke already fixed it in the version v8.0.0-rc20.

Should this PR include the latest version to update?

[davidjerleke]

@huri3l I think that’s a good idea. Since this isn’t merged yet, let just bump all Embla Carousel packages to v8.0.0-rc20.

@Xxsource98 would you mind doing that?

[davidjerleke]

@Xxsource98 just curious, don’t mean to rush you or anything: Is this PR waiting for something before it can be merged?

[Xxsource98]

@Xxsource98 just curious, don’t mean to rush you or anything: Is this PR waiting for something before it can be merged?

Well I don't think so. We found solution and I just updated embla-carousel packages in my commits. It is ready to merge.