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

Added instructions for TypeScript types #6

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

max-programming
Copy link

No description provided.

Copy link
Member

@canterberry canterberry left a comment

Choose a reason for hiding this comment

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

Thanks! I'm not a TypeScript user myself, so I rely on folks like you to bridge the gap in supporting TS. I appreciate your contribution.

That said, do you think index.d.ts is something that should live in this repo so users don't have to install a separate package to resolve type definitions? I would prefer that, if possible, which would make TypeScript just work right out of the box, so to speak, without any extra work by the dev.

I'm also okay with having it in a separate package like you've proposed here, with one caveat: give yourself an attribution in the README for the TS package so it's clear those are managed separately. Something like, "TypeScript definitions provided by Max Programming" or similar.

@max-programming
Copy link
Author

Thanks for the reply @canterberry!

I think it's possible to add types to a pure JavaScript package according to this answer, and I didn't know about that.

So we could go two ways as you wish.

  • I'll add type definitions in the index.d.ts file right in this package
  • We could go with the existing type definitions on DefinitelyTyped.

@canterberry
Copy link
Member

Right here in this package, then, if you please! Seamless TS support.

Thanks again for doing this.

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

Successfully merging this pull request may close these issues.

2 participants