Skip to content

[editable-layers] Update to turf 7 and use geojson types#221

Closed
timnyborg wants to merge 5 commits intovisgl:masterfrom
nabat-ai:turf-7-and-geojson
Closed

[editable-layers] Update to turf 7 and use geojson types#221
timnyborg wants to merge 5 commits intovisgl:masterfrom
nabat-ai:turf-7-and-geojson

Conversation

@timnyborg
Copy link
Contributor

@timnyborg timnyborg commented Feb 28, 2025

This is an attempt at updating editable-layers to rely on turf.js@7 to remove the type carnage that came from competing geojson, turf.js, and nebula.gl type definitions.

  • Mostly changes to replace the custom nebula Feature and FeatureCollection types with geojson's, narrowing to exclude GeometryCollection (which is unsupported)
  • Updates a few calls where turf.js functions now require FeatureCollection inputs (intersect, difference, etc.) or return a Feature (centroid)
  • Updates a couple values in tests, due to an upstream fix in @turf/area
  • Drops a lot of unneeded ts-expect-errors. Leaves a few type assertions in place to avoid additional functional changes, but the type safety is much improved
  • Updates "mode handlers", even though they seem to be long dead. Should they just be dropped from the package?
  • Drops code from long, long dead lib folder, and an unused curve-utils file, rather than trying to update turf usages there.

Assuming the PR is otherwise acceptable, I'm unsure what other work might need to be done. Other modules in the repo specify turf 6.5, so we might want to update them as well (examples, examples-wip, react, website)

Addresses #202

@timnyborg timnyborg changed the title Turf 7 and geojson [editable-layers] Update to turf 7 and use geojson types Feb 28, 2025

export type BoundingBoxArray = [number, number, number, number];
import { Feature, FeatureCollection } from 'geojson';
export { Feature, FeatureCollection } from 'geojson';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably want to drop this export, and have all files import directly from geojson instead - this was a quick way to make the switch.

@charlieforward9
Copy link
Collaborator

@timnyborg id like to see this land before integrating the editable-layers into my repo. What additional work is required here?

@timnyborg
Copy link
Contributor Author

timnyborg commented Apr 30, 2025

@charlieforward9 as far as I'm concerned, none, as this is passing the test suite for me. It's really a question for the repo owners, though.

Note that you can use this library with turf@7, which is almost entirely backwards-compatible with v6, just with the odd type assertion around Feature, etc.

@timnyborg timnyborg marked this pull request as ready for review April 30, 2025 11:02
@charlieforward9
Copy link
Collaborator

That's fantastic to hear. In this case, let's see if @ibgreen can review and land it! I'm eager to get editable layers as up to date and functional as possible as I have 2 projects depending on it

@charlieforward9
Copy link
Collaborator

@timnyborg I just got granted write access to this repo! If you are still interested in landing this, having it updated and verified would be a huge help. I am working through all the editable-layer PR's and issues and working to get them into production in the coming weeks. Your work is highly valued!

Copy link
Collaborator

@charlieforward9 charlieforward9 left a comment

Choose a reason for hiding this comment

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

@timnyborg, let me know if you can update this. Otherwise, Ill add it to my TODOs. Thank you

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