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-ifying turf-great-circle #2733

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

Conversation

thomas-hervey
Copy link

This change is intended to typescript-ify the turf-great-circles module following the typescript-ifying discussion #2538.

Summary of changes:

  • Converted /index.js into /index.ts
  • Added additional tests to /test.ts to cover different start and end input types
  • Added /lib/arc.d.ts including class GreatCircle

Template requirements:

  • Is this a bug fix, new functionality, or a breaking change?
    • This is an update of the turf-great-circles module to use typescript.
  • Have read and followed the steps for preparing a pull request.

@thomas-hervey thomas-hervey changed the title Typescript-ifying turf-great-circles Typescript-ifying turf-great-circle Oct 16, 2024
@thomas-hervey
Copy link
Author

This is my first (draft) PR to TurfJS in preparation for other enhancement requests to turf-great-circle.
@smallsaucepan given that you've begun typescript-ifying other modules, this PR should help contribute to this discussion #2538.

@smallsaucepan
Copy link
Member

Fantastic. Thanks @thomas-hervey. Will take a look at this shortly.

@smallsaucepan
Copy link
Member

@thomas-hervey it would be great if we could take the opportunity to do away with the whole lib/arc.* section and import as a third party module. There is https://www.npmjs.com/package/arc (which it looks like we've copy pasted into Turf at some point) but that doesn't have types.

Are you possibly in a position to submit types to DefinitelyTyped for the arc JS lib, so we can import it nicely into Turf as part of your PR? If not - and don't feel obliged - I'll give it a whirl and let you know when it's ready to integrate.

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.

2 participants