-
-
Notifications
You must be signed in to change notification settings - Fork 193
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
Add TypeScript usage examples to the docs #658
Conversation
Note We need to add this to the docs and also update all react CodeSandbox examples with the same approach. |
@davidjerleke I finalized the typescript page and also updated the way types are imported in the Sandboxes. Kindly take a look at it and let me know you need additional help 👍 |
025fbad
to
0fb4d99
Compare
Hey @davidjerleke , I've been working on my UI kit library here and I have the carousel component working well with only using Or should we look into making a PR for creating a Or would it be strategic to make a package solely for the types? like Based on your comment here , |
@zaaakher
So you should import types from the core package I don’t think it’s common practice to make a separate types package because Embla has first class TypeScript support. It’s built with TypeScript. I think type packages are mainly used for packages built without TypeScript. I hope my explanation makes sense? Best, |
True true, I agree.
That doesn't seem to be working as expected when using pnpm workspace (and in my project here I can do as you mentioned and everything works fine. But currently i'm migrating to a proper monorepo structure using pnpm (in the monorepo branch) and it seems |
Looking at this I was able to find
It's most likely a |
shadcn-ui is using this library but they still didn't migrate to the new typescript approach. And they're using So there's a chance they might stumble upon a similar problem if not an identical one to mine when they update embla carousel as per your suggestion here and change the way they import types. Of course a simple solution would be to explicitly install |
@zaaakher that’s valuable information. Thank you. I haven’t used |
Making |
@zaaakher I’m not sure how that would affect npm/yarn and pnpm for that matter. Is that a conventional solution? |
It doesn't feel like it and i'm very doubtful if it'd work.
That's a good solution, I can't think of a better one at the moment. Although is there a possible scenario in the future where a library wrapper uses a slightly different type from the one in the core library? (i.e. something unique to react or vue or server components) Currently I'm using types the original way 👇 import useEmblaCarousel, { EmblaOptionsType } from "embla-carousel-react"; I think it could be a good developer experience when consumers of this library have to install a single package (depending on their framework) where they can import hooks, types, and possibly components. Honestly I never knew I could import from a package that's not listed in my package.json (other than the node built-in packages), I just learned I could do that from you 😄 (although that didn't work with pnpm). I wish I have more time on my hand to test it out with pnpm alternatives like lerna, rush, or bolt. Nevertheless, I have a feeling they all have a similar approach to how they put aside dependencies-of-dependencies to save disk space and increase performance within a monorepo structure. I understand there could be performance/size trade-offs depending on the approach you'd like to take and I'm here if you wanna bounce ideas around, I'm just thinking out loud. |
@zaaakher I will try the copy types approach then. Thanks for your thoughts 👍🏻. Wow. The frontend tooling world is so broken. It’s really a nightmare to try to support everything. Feels like we spend 95% of our time with tools… |
True. It's the same cycle where an individual or a group of people make tool, then it gains popularity, then it's a either they have to start adapting to others or others have to adapt to them. |
Workaround: import { UseEmblaCarouselType } from "embla-carousel-react";
type EmblaCarouselType = NonNullable<UseEmblaCarouselType[1]>; |
@zaaakher I’ve been considering my options and I haven’t gotten anywhere to be honest. Copying typesCopying the types from the core library won’t work if someone build a wrapper or plugin that resides in a separate repository. So a requirement for this approach is that every Embla official package has to be in this repository. Separate type packagesOne solution is to create a separate package Add core library to peer dependenciesThis is how the plugins do. They only need types from the core package and not modules, so they have the core package as a peer dependency. And it seems like pnpm doesn’t isolate packages listed under peer dependencies like it does with packages under dependencies. I don’t know if npm or yarn or pnpm will throw warnings for this but maybe it’s worth to at least test this because this is easy to do. If I test npm and yarn, could you help me with how to test pnpm? Or maybe it should be under devDependencies? Doesn’t that make sense? Best, |
Hi @davidjerleke , I've been reading about So I think the best option for me is to install ConfirmationI tested it in my UI library kit and I installed I then published the ui kit Final thoughts
Although my So I think a caveat we could mention in the docs is to install I hope that was helpful 👍 Thanks, |
6603469
to
6eae893
Compare
Co-authored-by: David Jerleke <david.jerleke@gmail.com> Co-authored-by: Zakher Masri <46135573+zaaakher@users.noreply.github.com>
@zaaakher again, thanks for all your contributions and making this library better 🌟! |
@davidjerleke You're very welcome brother 🌹 |
Added typescript page for the docs
This is just a starting point since I had to wing it since I couldn't get the gatspy docs running locally in my PC. I will add more when I get the chance god willing