Slight mods to PR 6800

src/source/geojson_source.test.ts

@@ -977,7 +977,7 @@ describe('GeoJSONSource.shoudReloadTile', () => {
 
         expect(result).toBeTruthy();
     });
-    
+
     test('returns false when tile contains a feature that is being removed but was never added', async () => {
         const diff: GeoJSONSourceDiff = {remove: [0]};
         let shouldReloadTileOptions: GeoJSONSourceShouldReloadTileOptions = undefined;
@@ -1030,8 +1030,8 @@ describe('GeoJSONSource.shoudReloadTile', () => {
 
         expect(result).toBe(false);
     });
-    
-    test('handles string feature ids and returns no bounds since feature does not exist', async () => {
+
+    test('handles string feature ids and returns no shouldReloadTileOptions since feature does not exist', async () => {
         const diff: GeoJSONSourceDiff = {remove: ['abc']};
 
         let shouldReloadTileOptions: GeoJSONSourceShouldReloadTileOptions = undefined;
@@ -1042,7 +1042,7 @@ describe('GeoJSONSource.shoudReloadTile', () => {
         });
         await source.updateData(diff, true);
 
-        expect(shouldReloadTileOptions.affectedBounds).toHaveLength(0);
+        expect(shouldReloadTileOptions).toBeUndefined();
     });
 
     test('handles cluster', async () => {
@@ -1060,4 +1060,4 @@ describe('GeoJSONSource.shoudReloadTile', () => {
         expect(shouldReloadTileOptions).toBeUndefined();
     });
 
-});
+});

src/source/geojson_source.ts

@@ -417,7 +417,7 @@ export class GeoJSONSource extends Evented implements Source {
                 this.fire(new Event('dataabort', {dataType: 'source'}));
                 return;
             }
-            const affectedBounds = this._applyDiffIfNeeded(diff);
+            const shouldReloadTileOptions = this._getShouldReloadTileOptions(diff);
 
             let resourceTiming: PerformanceResourceTiming[] = null;
             if (result.resourceTiming && result.resourceTiming[this.id]) {
@@ -432,7 +432,7 @@ export class GeoJSONSource extends Evented implements Source {
             // although GeoJSON sources contain no metadata, we fire this event to let the TileManager
             // know its ok to start requesting tiles.
             this.fire(new Event('data', {...eventData, sourceDataType: 'metadata'}));
-            this.fire(new Event('data', {...eventData, sourceDataType: 'content', shouldReloadTileOptions: affectedBounds !== undefined ? {affectedBounds} : undefined}));
+            this.fire(new Event('data', {...eventData, sourceDataType: 'content', shouldReloadTileOptions}));
         } catch (err) {
             this._isUpdatingWorker = false;
             if (this._removed) {
@@ -449,14 +449,13 @@ export class GeoJSONSource extends Evented implements Source {
     }
 
     /**
-     * Apply a diff to the source data and return the bounds of the affected geometries.
-     * @param diff - The diff to apply.
-     * @returns The bounds of the affected geometries, undefined if the diff is not applicable or all geometries are affected.
+     * Get options for use in determining whether to reload a tile based on the changed features.
+     * @param diff - The GeoJSONSourceDiff to apply.
+     * @returns A GeoJSONSourceShouldReloadTileOptions object which contains an array of affected bounds by the update.
      */
-    private _applyDiffIfNeeded(diff: GeoJSONSourceDiff): LngLatBounds[] | undefined {
-        if (!diff) {
-            return undefined;
-        }
+    private _getShouldReloadTileOptions(diff: GeoJSONSourceDiff): GeoJSONSourceShouldReloadTileOptions | undefined {
+        if (!diff) return undefined;
+
         const promoteId = typeof this.promoteId === 'string' ? this.promoteId : undefined;
 
         // Lazily convert `this._data` to updateable if it's not already
@@ -472,33 +471,34 @@ export class GeoJSONSource extends Evented implements Source {
         }
         const affectedGeometries = applySourceDiff(this._data.updateable, diff, promoteId);
 
-        if (this._options.cluster || diff.removeAll) {
+        if (diff.removeAll || this._options.cluster) {
             return undefined;
         }
-            
-        return affectedGeometries
+
+        const affectedBounds = affectedGeometries
             .filter(Boolean)
             .map(g => getGeoJSONBounds(g));
+        if (!affectedBounds.length) return undefined;
+
+        return {affectedBounds};
     }
 
     /**
      * Determine whether a tile should be reloaded based on a set of options associated with a {@link MapSourceDataChangedEvent}.
      * @internal
      */
-    shouldReloadTile(tile: Tile, {affectedBounds}: GeoJSONSourceShouldReloadTileOptions) : boolean {
-        if (tile.state === 'loading') {
-            return true;
-        }
-        if (tile.state === 'unloaded') {
-            return false;
-        }
+    shouldReloadTile(tile: Tile, shouldReloadTileOptions: GeoJSONSourceShouldReloadTileOptions) : boolean {
+        if (!shouldReloadTileOptions) return false;
+        if (tile.state === 'loading') return true;
+        if (tile.state === 'unloaded') return false;
+
         // Update the tile if it WILL NOW contain an updated feature.
         const {buffer, extent} = this.workerOptions.geojsonVtOptions;
         const tileBounds = tileIdToLngLatBounds(
             tile.tileID.canonical,
             buffer / extent
         );
-        for (const bounds of affectedBounds) {
+        for (const bounds of shouldReloadTileOptions.affectedBounds) {
             if (tileBounds.intersects(bounds)) {
                 return true;
             }

src/source/geojson_source_diff.ts

@@ -125,36 +125,42 @@ export function toUpdateable(data: UpdateableGeoJSON, promoteId?: string) {
  * 1. Remove operations (removeAll, remove)
  * 2. Add operations (add)
  * 3. Update operations (update)
- * @returns an array of geometries that were affected by the diff
+ * @returns an array of geometries that were affected by the diff - with the exception of removeAll which does not return any affected geometries.
  */
 export function applySourceDiff(updateable: Map<GeoJSONFeatureId, GeoJSON.Feature>, diff: GeoJSONSourceDiff, promoteId?: string): GeoJSON.Geometry[] {
     const affectedGeometries: GeoJSON.Geometry[] = [];
+
     if (diff.removeAll) {
         updateable.clear();
     }
     else if (diff.remove) {
         for (const id of diff.remove) {
-            if (updateable.has(id)) {
-                affectedGeometries.push(updateable.get(id).geometry);
-                updateable.delete(id);
-            }
+            const existing = updateable.get(id);
+            if (!existing) continue;
+
+            affectedGeometries.push(existing.geometry);
+            updateable.delete(id);
         }
     }
 
     if (diff.add) {
         for (const feature of diff.add) {
             const id = getFeatureId(feature, promoteId);
-            if (id != null) {
-                affectedGeometries.push(feature.geometry);
-                updateable.set(id, feature);
-            }
+            if (id == null) continue;
+
+            // we are allowed to replace duplicate features with the same id, so need to check for existing
+            const existing = updateable.get(id);
+            if (existing) affectedGeometries.push(existing.geometry);
+
+            affectedGeometries.push(feature.geometry);
+            updateable.set(id, feature);
         }
     }
 
     if (diff.update) {
         for (const update of diff.update) {
-            let feature = updateable.get(update.id);
-            if (!feature) continue;
+            const existing = updateable.get(update.id);
+            if (!existing) continue;
 
             const changeGeometry = !!update.newGeometry;
 
@@ -167,7 +173,7 @@ export function applySourceDiff(updateable: Map<GeoJSONFeatureId, GeoJSON.Featur
             if (!changeGeometry && !changeProps) continue;
 
             // clone once since we'll mutate
-            feature = {...feature};
+            const feature = {...existing};
             updateable.set(update.id, feature);
             affectedGeometries.push(feature.geometry);