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

TypeScript Types #21

Closed
shellscape opened this issue Aug 26, 2021 · 5 comments
Closed

TypeScript Types #21

shellscape opened this issue Aug 26, 2021 · 5 comments

Comments

@shellscape
Copy link

Yes, I hate being that guy who opens this issue. Would you be willing to add some very basic types to the package, or would you accept a contribution for that?

@valeriangalliat
Copy link
Owner

Hey! I used to accept PRs for types, but my understanding is that DefinitivelyTyped is a better place for those since you can tag the version the types are made for (which is not possible inside this repo), which prevents confusion when the JS source eventually drift away from the types.

That said, I don't expect the public API for markdown-it-highlightjs to change in the next... few years? So considering that, feel free to make a PR here if you prefer. =)

@shellscape
Copy link
Author

Yeah I understand that. I had the same position about DefinitelyTyped for a long time, then I started using TypeScript daily for work and started to understand the perspective of users who asked for it in the lib itself. So I get both points of view, both are valid. That said, I'd be happy to be your go-to for any updates to types added here. I've been around a long time on Github/open source, no signs of disappearing anytime soon.

@valeriangalliat
Copy link
Owner

Oh alright, no problem then, thank you!

Out of curiosity, I'm guessing the main advantage of having the types in the package is to be able to update them in the same PR as the code? So there's a higher chance of the types staying in sync overtime compared to having to make a separate PR on DefinitivelyTyped? Not having to maintain an extra @types/ dependency could be another one... I'm interested to know other reasons from a TypeScript user perspective :)

@valeriangalliat
Copy link
Owner

Hey! I ended up rewriting this package in TypeScript so there's now native types support with v4.0.1. Cheers!

@shellscape
Copy link
Author

Hey thanks for checking back in on this one, and for the continued effort. Apologies for missing your followup reply.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants