Repository navigation
Conversation
A change to any other prop of a GeoJSON <Source> (e.g. tolerance, maxzoom) re-sent props.data, which overwrote data set with setData/updateData, and the option itself was not applied. Now setData is only called for a changed data prop; other changed props are reported with the same warning as other source types. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
[Medium impact] This PR appears safe to merge. SummaryThis PR prevents unrelated GeoJSON prop changes from overwriting data set directly on the map.
Reviews (1) · Last reviewed commit: "fix: only call setData on a GeoJSON sour..." · Reviewed by Greptile |
This branch has not been deployed
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
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.
Disclaimer: Found and fixed with Clause Opus 5.5 High. Description also written by Claude and reviewed by me.
The problem
updateSource()callssetData(props.data)on a GeoJSON<Source>when any prop changes, not onlydata. So changingtolerance,maxzoom,bufferorpromoteIdsendsprops.dataagain.Some apps don't pass their features through the
dataprop. They render the source once with a placeholder, such asdata={EMPTY}, and then push the real features themselves withmap.getSource(id).setData(...)orupdateData(...). That saves React from deep-comparing large GeoJSON on every render. From react-map-gl's side,datanever changes, so it should leave the source alone.If such an app changes any other prop,
tolerancein this example, react-map-gl callssetData(EMPTY). That replaces the app's features with the empty placeholder, and the map shows nothing until the app pushes its data again. The newtolerancedoesn't take effect either: MapLibre and Mapbox read that option only when the source is added, so the source keeps the old value.The change
For a
geojsonsource,setDatais now called only whendatachanged. Any other changed prop gets the sameUnable to update <Source> prop: ...warning that the other source types already log. Applied tosource.tsinreact-maplibreandreact-mapbox; I left themapbox-legacycopy inmodules/mainalone.Relation to #2618: that PR applies
cluster,clusterMaxZoomandclusterRadiuschanges withsetClusterOptionsand skipssetDatawhen only those changed (maplibre only). This PR covers the remaining props and both packages. The two touch the same lines, so whichever lands second needs a small rebase: in the loop over the changed keys, the cluster keys go tosetClusterOptionsinstead of the warning.New test in both packages: data set with
source.setData()is kept aftertolerancechanges, and the warning is logged. It fails without the change. The existingSource/Layertest still covers a changeddataprop.Validation: TypeScript build,
yarn lint, and all 86 node/headless tests pass.Note
Medium Risk
Changes core Source update behavior for all GeoJSON sources; low blast radius but could affect apps that relied on incidental
setDataon non-data prop changes.Overview
Fixes GeoJSON
<Source>updates so imperativesetData/updateDatais not wiped when unrelated props change.updateSourcein react-mapbox and react-maplibre used to callsetData(props.data)on every prop change fortype="geojson". Apps that mount with placeholderdataand push features via the map API would lose those features when e.g.tolerancechanged, because React re-sent the empty placeholder.The geojson branch now iterates changed props:
setDataruns only whendatachanged; other keys logUnable to update <Source> prop: …, matching behavior for non-geojson sources. Tests cover keeping imperative data after atolerancechange and asserting the warning.Reviewed by Cursor Bugbot for commit 226fc15. Bugbot is set up for automated code reviews on this repo. Configure here.