GeoJSON Feature properties stringified in map events

[shishkin]

maplibre-gl-js version: 2.1.9

browser: Chromium 101.0.4951.64

Steps to Trigger Behavior

1. Add GeoJSON source with a Point Feature and properties like:

{
    type: "FeatureCollection",
    features: [{
      type: "Feature",
      geometry: {
        type: "Point",
        coordinates: [0, 0],
      },
      properties: {
        foo: {bar: "baz"}
      },
    }],
  }

2. Make source layer interactive and handle map mouse events.
3. Get typeof e.features[0].properties.foo

Link to Demonstration

https://codepen.io/sergey-shishkin/pen/YzazqxO

Expected Behavior

Event feature properties should have same structure as original data source.

Actual Behavior

Event feature properties turned to strings.

[HarelM]

I think there was a similar issue related to ids of geojson features. If I need to guess this is probably related to some serialization and deserialization due to data passing between the worker and the main thread. But I would recommend investigating it and sending a PR in case you find the root cause :-)

[shishkin]

Thanks @HarelM, could you point me to the direction of investigation? I'm not familiar with the codebase. I've already looked through the code of GeoJSON sources and workers but couldn't spot anything obviously wrong.

[HarelM]

Geojson source and worker would be my guess as well...

[rduivenvoorde]

Does this not be because of the fact that originally geojson (see geojson.org) defines so called ‘ OpenGIS Simple Features Implementation Specification’ features, in which properties almost always is a table like (not tree like) structure? What do other geojson libs do with your not so standard geojson?

[shishkin]

GeoJSON.js supports object properties just fine: https://codepen.io/sergey-shishkin/pen/gOegopW.

[rotu]

The serialize/deserialize implementations seem fishy to me.

I don't see a reason not to use structuredClone. Could probably result in some perf gains too.

Edit: I missed the obvious; this does rest on the structured clone algorithm but it does more work to maintain object classes. It still looks like a footgun in places (like as a side effect, it implicitly guts the buffers of the source object).

I would look at the custom deserializer of FeaturePositionMap. That seems to be a possible culprit.

[guimochila]

I am facing the same problem and I have to confess that it took me awhile to understand that was Maplibre who was "stringifying" the properties property. =/

Edit: Any object inside properties are being serialized.

[guimochila]

@HarelM @rotu I have been trying to debug this issue but it is bit more complex than I initially thought.

I might be wrong but the issue here seems to come from @mapbox/vector-tile package. On feature_index.ts line 100, it starts a new VectorTiles:

loadVTLayers(): {[_: string]: VectorTileLayer} {
        if (!this.vtLayers) {
            this.vtLayers = new vt.VectorTile(new Protobuf(this.rawTileData)).layers;
            this.sourceLayerCoder = new DictionaryCoder(this.vtLayers ? Object.keys(this.vtLayers).sort() : ['_geojsonTileLayer']);
        }
        return this.vtLayers;
    }

The properties property is already deconstructed and any object is stringified:

On line 203 feature is serialized:

        const sourceLayerName = this.sourceLayerCoder.decode(sourceLayerIndex);
        const sourceLayer = this.vtLayers[sourceLayerName];
        const feature = sourceLayer.feature(featureIndex);

All objects inside properties are returned as stringified:

I am not sure how to approach for a solution as it seems to be in a mapbox library. Maybe a function to try to deserialize the properties in properties? Assuming that the issue is really from where I am saying. 😆

[HarelM]

Ahh, that's interesting. The geojson source is transforming the geojson into tiles so what you say makes a lot of sense. If you think the bug is in the serialization or deserialization of the vt mapbox library you should be able to create a test there to show the problem (without all the wrapping that is done in the geojson source etc in this lib). If you manage to do that, you can consider opening a PR to that library. Once this PR is merged and there is a new version of this library we could bump the version here and have that fix here too. Does this sound like a good plan to you?

[guimochila]

@HarelM That sound good, I have just worked in a workaround, tell me what do you think?

import vt from '@mapbox/vector-tile';

export function deserializeFeature(feature: vt.VectorTileFeature) {
    if (!feature || Object.keys(feature.properties).length === 0) {
        return feature;
    }

    for (const key in feature.properties) {
        if (typeof feature.properties[key] === 'string') {
            try {
                feature.properties[key] = JSON.parse(feature.properties[key] as string);
            } catch (e) {
                continue;
            }
        }
    }

    return feature;
}

This actually works and I will probably need to implement this in my project so I can make it work. But with this code GeoJSONFeature already returns all objects. It can be more detailed and check if there is an object inside the string but I am not sure about performance though.

[shishkin]

@guimochila I use a similar approach but use superjson to ensure Date's are serialized and deserialized correctly:

function serializeFeature(f: Feature): Feature {
  return {
    ...f,
    properties: {
      ...f.properties,
      _json: superjson.stringify(f.properties),
    },
  };
}

function deserializeFeature(f: Feature): Feature {
  const json = f.properties._json as string;
  if (!json) return f;

  const properties = superjson.parse(json) as FeatureProperties;
  f.properties = Object.assign({}, f.properties, properties);
  return f;
}

serializeFeature is used before setting GeoJSON source and deserializeFeature is used when I need original feature object inside maplibre-gl event handlers.

This is likely not the most efficient way though.

[guimochila]

@shishkin That looks nice, however I agree with you this is maybe not the most efficient way and I guess the issue still in the @mapbox/vector-tile deserialization.

[guimochila]

I am just wondering if this is really a bug in @mapbox/vector-tile deserialization or that is how it is by design and it is from Maplibre responsibility to parse the data back. Just food for the thoughts.

[HarelM]

From my point of view, serialization and deserialization should always be symmetric - if you can serialize an object with properties without an error saying it's not supported then deserializing it back should return a clone of the original object. But opening an issue/PR in the original mapbox project can help in getting their input on this...

[guimochila]

@HarelM, @mapbox/vector-tile doesn't seem to have much maintenance lately. Not sure if we should wait for something on their side or come up with a solution.

[HarelM]

Well... 5 days isn't a lot to wait (although I agree this repo isn't very active). I recommend creating a fork and send a PR with a fix to improve changes of this being fixed/merged. If the PR is ignored, we can decide to publish this fork and use it here instead of using the original library.

[reinhard-sanz]

@HarelM @guimochila And updates on this? Sadly https://github.com/mapbox/vector-tile-js looks pretty dead to me :/

[HarelM]

The main question here is if someone would like to fork and maintain it, otherwise, we'll be in the same situation, but with a fork... I haven't seen a PR that addresses this issue, and I think the mvt spec isn't great with objects in properties, so this isn't just for geojson but for regular tiles as well. I'm not sure there's a good solution beside designing the next vector tiles format, which will probably take years... :-/ Sorry...

[reinhard-sanz]

No worries! Implementing a workaround for my use case shouldn't be too hard anyway :)

[mourner]

Apologies for leaving https://github.com/mapbox/vector-tile-js unmaintained for so long — I revived and modernized it recently (see v2.0 and plans for v3).

Regarding the issue at hand — unfortunately, this is limited not by encoding/decoding but by the Vector Tile Specification itself. The MVT spec doesn't have a way of encoding nested properties. Perhaps the new MRT format could address this.