Limitations of the turf geotypes package

[josxha]

Description

This discussion is about the use of geotypes for geo features. There are some problems or limitations of the package I'd like to collect and discuss.

Possible solutions

0. Implement implementations and workaround (currently solution and not suitable on the long term).
1. Talk to the dart turf team about these limitations and find a solution together.
2. Try other package for geojson types (e.g. geobase).
3. Use an own base package for geojson types (most work but better integrity).

[josxha]

nullable feature.id field

geotypes uses null as a feature id if no id is set.
From the geojson specification:

If a Feature has a commonly used identifier, that identifier SHOULD be included as a member of the Feature object with the name "id", and the value of this member is either a JSON string or number.

This means if there is no identifier, the "id" field should not exist in the json.

MapLibre Native does not allow null as a feature id and excepts that the id key does not exist in the json. We currently have a workaround to mitigate this problem in our example app.

flutter-maplibre/example/lib/animation_page.dart

Lines 40 to 49 in aba6de7

	final response = await get(
	Uri.parse('https://maplibre.org/maplibre-gl-js/docs/assets/hike.geojson'),
	);
	final geojsonLine = response.body;
	final geojson = FeatureCollection.fromJson(
	jsonDecode(geojsonLine) as Map<String, Object?>,
	);
	final lineString = geojson.features.first.geometry! as LineString;
	// TODO: setting the id is currently required as geotypes would set it to null, Feature.id documented at https://datatracker.ietf.org/doc/html/rfc7946#section-3.2
	geojson.features.first.id = 1;

[lukas-h]

I think we can figure out a change that won't break with the RFC or backwards-compatibility

[josxha]

const constructors

Many of the data classes in maplibre have const constructors. The const keyword can never be used when initializing new instances because the types from geotypes don't have const constructors. This mostly affects showcases and does not affect implementations where the data has to get loaded first.

flutter-maplibre/example/lib/layers_circle_page.dart

Lines 36 to 42 in aba6de7

	CircleLayer(
	points: _points,
	color: Colors.orange.withOpacity(0.5),
	radius: 20,
	strokeColor: Colors.red,
	strokeWidth: 2,
	),

This is mostly a nit as it has limited usage in real world usages.

[smallTrogdor]

True, but could also be plus one for another package / own base package.

[lukas-h]

we are happy to include const constructors in geotypes package :) We welcome a PR and would love to give guidance/pointers on the implementation

[josxha]

No toString() overrides

We use toString() in our example app, for example to show the Position that has been clicked by the user. Because geotypes does not provide a toString() representation itself, we need to do it.

flutter-maplibre/example/lib/events_page.dart

Lines 72 to 73 in aba6de7

	String _formatPosition(Position point) =>
	'${point.lng.toStringAsFixed(3)}, ${point.lat.toStringAsFixed(3)}';

flutter-maplibre/lib/src/map_events.dart

Lines 72 to 73 in aba6de7

	@override
	String toString() => 'MapEventClick(point: $point)';

[smallTrogdor]

Would be a plus one for me to use another package / implement own base package for types.

[lukas-h]

would be happy to receive a PR for that :)

[josxha]

missing == and hashCode overrides for LineString and Polygon

While some geotypes classes have a hash code and equals override, LineString and Polygon does not. It is necessary to compare if instances contain the same data. Currently both are only equal or have the same hashCode if they are the same object.
Comparing large lists of Positions can be expensive, there could be some additional optimizations like comparing the list lengths first before checking every item of the list.

[smallTrogdor]

These kind of optimizations are already done in the collection package. I would recommend using those before implementing own ListEquality methods.
(e.g. see https://github.com/dart-lang/core/blob/main/pkgs/collection/lib/src/equality.dart#L175)

[josxha]

Using num

From the geojson specs:

JavaScript Object Notation (JSON), and the terms object, member, name, value, array, number, true, false, and null, are to be interpreted as defined in [RFC7159].

and from the JSON specs:

The representation of numbers is similar to that used in most programming languages. A number is represented in base 10 using decimal digits. It contains an integer component that may be prefixed with an optional minus sign, which may be followed by a fraction part and/or an exponent part. Leading zeros are not allowed. [...]

From a JavaScript perspective, num as a data type for the number type described in the geojson specs seems natural. Though, as dart is not only used in the JavaScript world, all other programming languages like swift, kotlin or c expect a double or int. When interacting with those languages, all nums need to explicitly converted to a double using toDouble().

While the definition of the number data type applies to num as it is a super type of int and double, it applies to double itself as well.

flutter-maplibre/lib/src/platform/android/widget_state.dart

Lines 206 to 209 in aba6de7

	: pigeon.LngLat(
	lng: _options.initCenter!.lng.toDouble(),
	lat: _options.initCenter!.lat.toDouble(),
	),

[lukas-h]

this is from an old era when dart didn't allow developers to write e.g. 0 as a valid double literal. We can discuss this

[smallTrogdor]

Seeing all these issues (especially the feature.id and the missing equality for LineString and Polygon) would point me honestly towards looking into other packages or building your own base type package.

[josxha]

Let's take some time to find the right approach here. I had a quick chat with @lukas-h and he wants to take a look at this thread and get back to me. (:
There is a package called turf_equality for equality checks but it's build on top of the package, not within. The feature.id could potentially be treated as bug and is fixable.

[lukas-h]

the idea to have a separate library for turf_equality was to have the core packages very small.
Equality for polygons e.g. is quite tricky and has many levels (precision, shifted polygons, reversed geometries) of what to actually accept as "equal" and we wanted to have the developers make a conscious choice.

[lukas-h]

@josxha It would be great to hop on a discord call to discuss all the things – we are super happy to make informed changes to the packages :)

[lukas-h]

we'd be happy to incorporate your feedback into the package. We also have a student team supporting us to work on the project soon

[josxha]

Thanks for your feedback @lukas-h. I appreciate your openess for contributions. Though, before contributing, I'd like to check if there is an other base package that already fits the needs of this package. I don't want to push altering design ideas on the geotypes package if not required. (: