Add updateable support for GeoJSON-VT

[wayofthefuture]

In progress...

• Briefly describe the changes in this PR.
• Include before/after visuals or gifs if this PR includes visual changes.
• Write tests for all new functionality.
• Document any changes to public APIs.
• Post benchmark scores.
• Add an entry to CHANGELOG.md under the ## main section.

[socket-security]

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff	Package	Supply Chain Security	Vulnerability	Quality	Maintenance	License
	@​maplibre/​geojson-vt@​6.0.1

View full report

[wayofthefuture]

@HarelM It looks sooooo much better...

[HarelM]

Ok, only one test is failing now.
I'm able to see the difference in terms of slightly different rendering.
When I tired to use the old suplercluster library the test passes.
When I debugged it a bit I see that getTile for 0,0,0 is returning a different number of features (61 vs 58) which explains the rendering diff.
So there's a need to debug this further and see what changed.
I'm guessing it is related to the changes I made, might be the dead code part that I removed.
I think it should be fairly easy to reproduce this in a geojson-vt test.
I'll see if I can work on that next week.

[wayofthefuture]

@HarelM I copied places.json into geojson-vt and old supercluster lib and compared the 0,0,0 tile generated:

test('generates clusters properly', () => {
    const index = new ClusterTileIndex({radius: 25});
    index.load(places2.features);
    const tile = index.getTile(0, 0, 0);
    writeFileSync('out.json', JSON.stringify(tile.features));
});

Both tiles contained 58 points, and JSON.stringify(tile.features)===JSON.stringify(tile.features) (new geojson-vt -> old supercluster lib) was an exact match in both libraries. The only difference was the tile metadata {transformed: true, source: null, x: 0, y: 0, z: 0} in the newer.

In other words, no progress, but wanted to let you know.

[HarelM]

Did you pass the same cluster parameters?

[wayofthefuture]

Only the radius

[HarelM]

I was able to reproduce this fairly easily on maplibre by replacing the initialization in the factory method. What you did sounds like what I would've done to try and reproduce this, but I'll give it anothe go early next week nevertheless.

[wayofthefuture]

Found it:

    // wraps features (ie extreme west and extreme east)
    features = wrap(features, options);

https://github.com/maplibre/geojson-vt/blob/ada402f655274d09c77b903f759535edad6a23e4/src/geojsonvt.ts#L79-L80

    if (!options.cluster) features = wrap(features, options);

fixes it. however, when applying a source diff the features would be wrapped there as well. so i think the solution may be to update expected.png in the test.

[HarelM]

Can you better explain why updating the image is the right solition?
I don't mind updating the image, just curious as to why.

[wayofthefuture]

The source data contains wrapped features. It would be complex to have wrapped for geojsonvt and unwrapped for supercluster. It's a confusing but very slight difference between the images:

expected / actual:

[HarelM]

Feel free to update the image and I'll merge this. I think both options are valid I guess, aligning then to the same processing makes sense.

[codecov]

Codecov Report

❌ Patch coverage is 83.01887% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.70%. Comparing base (0912f0f) to head (63022b2).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
src/source/geojson_worker_source.ts	80.00%	7 Missing ⚠️
src/data/bucket/line_bucket.ts	33.33%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7172      +/-   ##
==========================================
+ Coverage   92.68%   92.70%   +0.02%     
==========================================
  Files         289      289              
  Lines       24085    24073      -12     
  Branches     5102     5108       +6     
==========================================
- Hits        22322    22316       -6     
+ Misses       1763     1757       -6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[HarelM]

Might be interesting to increase coverage while we are at it.

[wayofthefuture]

In order to hardcode updateable I had to remove the non-updateable tests.. let me know what you think. The tests weren't very good anyway because they simulated geojsonvt worker options that don't exist. But we still have to consider geojson sources without ids...

[wayofthefuture]

Fyi also see the misc changes commit above