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

Add TypeScript types #3

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

Add TypeScript types #3

wants to merge 1 commit into from

Conversation

meyer
Copy link

@meyer meyer commented Sep 4, 2019

This PR adds TypeScript types and links to them in the package.json file.

@CLAassistant
Copy link

CLAassistant commented Sep 4, 2019

CLA assistant check
All committers have signed the CLA.

@n8downs
Copy link
Collaborator

n8downs commented Sep 23, 2019

Adding TS definitions here implies that this project will keep them up to date. Since this project is already Flow-based, I'd prefer not to maintain both. If there's a way to automatically keep the TS definitions in sync with the Flow code, I'd accept this PR. Otherwise, I think DefinitelyTyped would be a more appropriate place for these definitions.

@meyer
Copy link
Author

meyer commented Sep 24, 2019

@n8downs yeah, this is definitely a commitment to keeping the types up to date. Given both the low commit frequency to twemoji-parser and the fact that the types themselves are only ~15 LOC in total, I’d say that the types becoming wildly out-of-date is not going to be a huge problem.

As far as maintenance burden goes, we could add a line in the README explaining that the types are third-party contributions and welcome TypeScript PRs.

Regarding an automated solution: I would love a solution for generating types from Flow code, but the flow tooling support is just not there yet and most likely won’t be for quite some time. It’s not even possible to get an entire typed AST from Flow (see this almost four-year-old[!!!] issue: facebook/flow#248). If you’re open to converting this library to TypeScript, we can very easily go the other way though (most likely using flowgen).

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

Successfully merging this pull request may close these issues.

3 participants