Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(react): export EmblaViewportRefType from bundle #947

Merged
merged 1 commit into from
Jul 23, 2024

Conversation

junlarsen
Copy link
Contributor

Hi David, thanks for your awesome work on Embla!

I recently wrote a small wrapper component around the react plugin and found it would be beneficial to pass around the Embla viewport ref type (for example uses like forwardRef).

This small patch re-exports EmblaViewportRefType from embla-carousel-react. Let me know if there are any questions!

@davidjerleke davidjerleke added feature request New feature or request react Issue is related to React typescript Issue is related to TypeScript labels Jul 21, 2024
@davidjerleke davidjerleke self-requested a review July 21, 2024 07:19
Copy link
Owner

@davidjerleke davidjerleke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @junlarsen,

Thanks for your contribution! I think these changes make sense. We also need to export it from the index file:

export { UseEmblaCarouselType } from './components/useEmblaCarousel'

The reason why is because package managers like pnpm have introduced new behavior to the frontend world. For example, pnpm doesn’t allow imports from any other file than the index file, so any imports with paths to specific files will fail.

@junlarsen
Copy link
Contributor Author

Right, missed that one. I'll get on it tonight :)

@davidjerleke
Copy link
Owner

@junlarsen this is approved. Nicely done 👍🏻. Feel free to squash the commits into a single commit. Otherwise I will do it before merging this PR.

@davidjerleke davidjerleke added the resolved This issue is resolved label Jul 23, 2024
@davidjerleke davidjerleke merged commit 79d243c into davidjerleke:master Jul 23, 2024
@davidjerleke
Copy link
Owner

Thanks @junlarsen 👍🏻!

@davidjerleke
Copy link
Owner

@junlarsen your fix has been included in v8.1.8 which I just released.

@junlarsen
Copy link
Contributor Author

Great, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request react Issue is related to React resolved This issue is resolved typescript Issue is related to TypeScript
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants