Need to remove and re-add source if any prop besides data is different

mapbox · Nov 24, 2017 · d3a11cb · d3a11cb
1 parent 8f6e334
commit d3a11cb
Show file tree

Hide file tree

Showing 2 changed files with 111 additions and 8 deletions.
diff --git a/src/style-spec/diff.js b/src/style-spec/diff.js
@@ -100,6 +100,36 @@ const operations = {
 
 };
 
+function addSource(sourceId, after, commands) {
+    commands.push({ command: operations.addSource, args: [sourceId, after[sourceId]] });
+}
+
+function removeSource(sourceId, commands, sourcesRemoved) {
+    commands.push({ command: operations.removeSource, args: [sourceId] });
+    sourcesRemoved[sourceId] = true;
+}
+
+function updateSource(sourceId, after, commands, sourcesRemoved) {
+    removeSource(sourceId, commands, sourcesRemoved);
+    addSource(sourceId, after, commands);
+}
+
+function canUpdateGeoJSON(before, after, sourceId) {
+    let prop;
+    for (prop in before[sourceId]) {
+        if (!before[sourceId].hasOwnProperty(prop)) continue;
+        if (prop !== 'data' && !isEqual(before[sourceId][prop], after[sourceId][prop])) {
+            return false;
+        }
+    }
+    for (prop in after[sourceId]) {
+        if (!after[sourceId].hasOwnProperty(prop)) continue;
+        if (prop !== 'data' && !isEqual(before[sourceId][prop], after[sourceId][prop])) {
+            return false;
+        }
+    }
+    return true;
+}
 
 function diffSources(before, after, commands, sourcesRemoved) {
     before = before || {};
@@ -111,25 +141,21 @@ function diffSources(before, after, commands, sourcesRemoved) {
     for (sourceId in before) {
         if (!before.hasOwnProperty(sourceId)) continue;
         if (!after.hasOwnProperty(sourceId)) {
-            commands.push({ command: operations.removeSource, args: [sourceId] });
-            sourcesRemoved[sourceId] = true;
+            removeSource(sourceId, commands, sourcesRemoved);
         }
     }
 
     // look for sources to add/update
     for (sourceId in after) {
         if (!after.hasOwnProperty(sourceId)) continue;
         if (!before.hasOwnProperty(sourceId)) {
-            commands.push({ command: operations.addSource, args: [sourceId, after[sourceId]] });
+            addSource(sourceId, after, commands);
         } else if (!isEqual(before[sourceId], after[sourceId])) {
-            if (before[sourceId].type === 'geojson' && after[sourceId].type === 'geojson') {
-                // geojson sources use setGeoJSONSourceData command to update
+            if (before[sourceId].type === 'geojson' && after[sourceId].type === 'geojson' && canUpdateGeoJSON(before, after, sourceId)) {
                 commands.push({ command: operations.setGeoJSONSourceData, args: [sourceId, after[sourceId].data] });
             } else {
                 // no update command, must remove then add
-                commands.push({ command: operations.removeSource, args: [sourceId] });
-                commands.push({ command: operations.addSource, args: [sourceId, after[sourceId]] });
-                sourcesRemoved[sourceId] = true;
+                updateSource(sourceId, after, commands, sourcesRemoved);
             }
         }
     }

diff --git a/test/unit/style-spec/diff.test.js b/test/unit/style-spec/diff.test.js
@@ -141,6 +141,83 @@ t('diff', (t) => {
         }]}
     ], 'update a geojson source');
 
+    t.deepEqual(diffStyles({
+        sources: {
+            foo: {
+                type: 'geojson',
+                data: { type: 'FeatureCollection', features: [] }
+            }
+        }
+    }, {
+        sources: {
+            foo: {
+                type: 'geojson',
+                data: { type: 'FeatureCollection', features: [] },
+                cluster: true
+            }
+        }
+    }), [
+        { command: 'removeSource', args: ['foo'] },
+        { command: 'addSource', args: ['foo', {
+            type: 'geojson',
+            cluster: true,
+            data: { type: 'FeatureCollection', features: [] }
+        }]}
+    ], 'remove and re-add a source if cluster changes');
+
+    t.deepEqual(diffStyles({
+        sources: {
+            foo: {
+                type: 'geojson',
+                data: { type: 'FeatureCollection', features: [] },
+                cluster: true
+            }
+        }
+    }, {
+        sources: {
+            foo: {
+                type: 'geojson',
+                data: { type: 'FeatureCollection', features: [] },
+                cluster: true,
+                clusterRadius: 100
+            }
+        }
+    }), [
+        { command: 'removeSource', args: ['foo'] },
+        { command: 'addSource', args: ['foo', {
+            type: 'geojson',
+            cluster: true,
+            clusterRadius: 100,
+            data: { type: 'FeatureCollection', features: [] }
+        }]}
+    ], 'remove and re-add a source if cluster radius changes');
+
+    t.deepEqual(diffStyles({
+        sources: {
+            foo: {
+                type: 'geojson',
+                data: { type: 'FeatureCollection', features: [] },
+                cluster: true,
+                clusterRadius: 100
+            }
+        }
+    }, {
+        sources: {
+            foo: {
+                type: 'geojson',
+                data: { type: 'FeatureCollection', features: [] },
+                cluster: true
+            }
+        }
+    }), [
+        { command: 'removeSource', args: ['foo'] },
+        { command: 'addSource', args: ['foo', {
+            type: 'geojson',
+            cluster: true,
+            data: { type: 'FeatureCollection', features: [] }
+        }]}
+    ], 'remove and re-add a source if cluster radius changes (before and after swapped)');
+
     t.deepEqual(diffStyles({}, {
         metadata: { 'mapbox:author': 'nobody' }
     }), [], 'ignore style metadata');