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

refactor: replace turf with vanilla TS version #388

Conversation

danditomaso
Copy link
Collaborator

  • Implement custom bbox and lineString modules
  • Remove external @Turf dependency
  • Add TypeScript types for GeoJSON features

@danditomaso danditomaso requested a review from Hunter275 January 29, 2025 16:17
@danditomaso danditomaso changed the title Refactor: Replace turf with vanilla refactor: Replace turf with vanilla Jan 29, 2025
@danditomaso danditomaso changed the title refactor: Replace turf with vanilla refactor: replace turf with vanilla TS version Jan 29, 2025
@dzienisz
Copy link
Contributor

Do you think it's better to maintain this code by ourselves?

@danditomaso
Copy link
Collaborator Author

Do you think it's better to maintain this code by ourselves?

For now yes, but the plan is to eventually not need this code in any case, as we will be switching our map library to use leaflet. Plus, its only about 100 lines of very understandable TS without any chance of exploits or other things we need to worry about.

@Hunter275
Copy link
Member

@danditomaso

How soon do you think we could replace the map with Leaflet? Does it make sense to just wait for that instead of merge this?

@Hunter275 Hunter275 added the dependencies Pull requests that update a dependency file label Jan 31, 2025
@danditomaso
Copy link
Collaborator Author

danditomaso commented Jan 31, 2025

@Hunter275 Yeah, I'm good to wait as this library will go away once we switch to leaflet. I should be able to get to the map replacement next week

@danditomaso danditomaso closed this Feb 1, 2025
@danditomaso danditomaso deleted the refactor/replace-turf-with-vanilla branch February 2, 2025 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants