Remove AllGeoJSON type, @turf/meta fixes - #3158
Draft
mfedderly wants to merge 5 commits into
Draft
Conversation
Excludes meta which needs its underlying typing cleaned up first.
This cleans up the ts-expect-error usage that was introduced when this was migrated to TypeScript. At the time we did this to reduce the risk of bugs and API breaks within the 7.x branch, while still allowing us to consolidate the build infrastructure. This introduces duplicate checks against object .type properties, but overall node26 seems to like this version better. Public API: Replace AllGeoJSON with @types/geojson's GeoJSON Public API: Exposes the `false | void` return types for callback and coordEach which better aligns with the jsdoc and actual API before (speeds measured on battery) coordEach - point x 112,890,018 ops/sec ±1.73% (95 runs sampled) coordEach - points x 258,507 ops/sec ±0.35% (97 runs sampled) coordEach - polygon x 82,591,948 ops/sec ±1.37% (95 runs sampled) coordEach - polygons x 124,683 ops/sec ±0.30% (98 runs sampled) after coordEach - point x 130,521,631 ops/sec ±1.87% (94 runs sampled) coordEach - points x 378,232 ops/sec ±0.28% (97 runs sampled) coordEach - polygon x 90,271,698 ops/sec ±1.30% (95 runs sampled) coordEach - polygons x 156,581 ops/sec ±0.30% (99 runs sampled)
mfedderly
changed the title
Make the geojson input definition much more explicit. We handle feature collections and features with null geometries but just continue past them. Explicitly *disallow* nested GeometryCollections. I'd like to standardize on methods in @turf/meta not supporting nested GeometryCollections, and us skipping null geometries when they happen. I think this better aligns with our own internal use of these methods (consumers of geomEach aren't checking for nulls) and nested GeometryObjects would have worked very strangely with the previous implementation where it would recursively call coordEach and restart the *Index parameters being passed into callback.
API Changes: - Nested GeometryCollections are officially disallowed - null geometries are no longer passed to the callback method - Input types for geojson are quite a bit stricter - properties, bbox, id arguments are now explicitly listed as nullable - can this be made stricter? Again a performance improvement despite the duplicate checks on .type before geomEach - point x 107,410,591 ops/sec ±1.58% (92 runs sampled) geomEach - points x 262,352 ops/sec ±0.33% (98 runs sampled) geomEach - polygon x 101,377,233 ops/sec ±1.65% (92 runs sampled) geomEach - polygons x 244,982 ops/sec ±0.29% (97 runs sampled) after geomEach - point x 187,774,693 ops/sec ±2.33% (95 runs sampled) geomEach - points x 622,728 ops/sec ±0.48% (94 runs sampled) geomEach - polygon x 175,331,140 ops/sec ±3.59% (88 runs sampled) geomEach - polygons x 483,427 ops/sec ±0.93% (85 runs sampled)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
@turf/helpers'
AllGeoJSONis actually just @types/geojson'sGeoJSON.In the process of removing it, @turf/meta's internal type inconsistency caused even more typing issues. Because v8 is breaking, we can actually make the changes required to fix the type errors correctly. This is obviously very performance sensitive and will need careful consideration.
I started with coordEach and am posting this early as a draft if anyone has feedback.