Skip to content

Conversation

@lucaswoj
Copy link
Contributor

@lucaswoj lucaswoj commented Dec 2, 2025

Fixes #6795

Problem

When calling GeoJSONSource#updateData to remove features, symbol layers would incorrectly keep displaying the removed features or hide features at the wrong times during zoom operations.

Root Cause

The code was iterating through tile.latestFeatureIndex.featureIndexArray to determine which tiles needed reloading. However, featureIndexArray excludes symbol features. This meant that tiles containing symbol features would not be marked for reload when those features were updated or removed.

Solution

Changed the reload detection logic to iterate through layers[GEOJSON_TILE_LAYER_NAME], which includes all features including symbol features. This ensures tiles are properly reloaded when any feature is updated, regardless of layer type.

Testing

Added integration regression test at test/integration/render/tests/regressions/mapbox-gl-js#6795/

Launch Checklist

  • Confirm your changes do not include backports from Mapbox projects (unless with compliant license) - if you are not sure about this, please ask!
  • Briefly describe the changes in this PR.
  • Link to related issues.
  • Write tests for all new functionality.
  • Post benchmark scores.
  • Add an entry to CHANGELOG.md under the ## main section.

@codecov
Copy link

codecov bot commented Dec 2, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.43%. Comparing base (cf0f690) to head (f007782).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6801      +/-   ##
==========================================
+ Coverage   92.42%   92.43%   +0.01%     
==========================================
  Files         288      288              
  Lines       23809    23809              
  Branches     5057     5058       +1     
==========================================
+ Hits        22006    22009       +3     
+ Misses       1803     1800       -3     

☔ 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.

@lucaswoj lucaswoj marked this pull request as ready for review December 2, 2025 16:11
@HarelM
Copy link
Collaborator

HarelM commented Dec 3, 2025

Since I biased towards my solution I'll let @wayofthefuture decide which solution he prefers.
Regardless, I would like to avoid updating render tests for this issue.
Also please take a look at how I changed the unit tests in geojson source to only relay on public APIs and not use "internal" methods.
You can wait with these request for changes above to a decision about which solution to use first, to avoid redundant work.

@lucaswoj
Copy link
Contributor Author

lucaswoj commented Dec 3, 2025

I would like to avoid updating render tests for this issue.

I added a regression test for this bug. No existing tests were updated. We can remove the regression test if you prefer

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.

geoJsonSource.updateData not working properly after updating to 5.13.0

2 participants