Fix stale tiles being shown when calling VectorTileSource#setTiles wh…

…ile the map is moving (#913)
maplibre · Jan 28, 2022 · 321c416 · 321c416
1 parent c8e8345
commit 321c416
Show file tree

Hide file tree

Showing 5 changed files with 36 additions and 17 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,7 @@
 
 ### 🐞 Bug fixes
 
+- Fix stale tiles being shown when calling VectorTileSource#setTiles while the map is moving.
 - *...Add new stuff here...*
 
 ## 2.1.0

diff --git a/src/source/vector_tile_source.test.ts b/src/source/vector_tile_source.test.ts
@@ -9,13 +9,13 @@ import fixturesSource from '../../test/fixtures/source.json';
 import {getMockDispatcher, getWrapDispatcher} from '../util/test/util';
 import Map from '../ui/map';
 
-function createSource(options, transformCallback?) {
+function createSource(options, transformCallback?, clearTiles = () => {}) {
     const source = new VectorTileSource('id', options, getMockDispatcher(), options.eventedParent);
     source.onAdd({
         transform: {showCollisionBoxes: false},
         _getMapId: () => 1,
         _requestManager: new RequestManager(transformCallback),
-        style: {sourceCaches: {id: {clearTiles: () => {}}}},
+        style: {sourceCaches: {id: {clearTiles}}},
         getPixelRatio() { return 1; }
     } as any as Map);
 
@@ -343,4 +343,15 @@ describe('VectorTileSource', () => {
             tiles: ['http://example2.com/{z}/{x}/{y}.png']
         });
     });
+
+    test('setTiles only clears the cache once the TileJSON has reloaded', done => {
+        const clearTiles = jest.fn();
+        const source = createSource({tiles: ['http://example.com/{z}/{x}/{y}.pbf']}, undefined, clearTiles);
+        source.setTiles(['http://example2.com/{z}/{x}/{y}.pbf']);
+        expect(clearTiles.mock.calls).toHaveLength(0);
+        source.once('data', () => {
+            expect(clearTiles.mock.calls).toHaveLength(1);
+            done();
+        });
+    });
 });
diff --git a/src/source/vector_tile_source.ts b/src/source/vector_tile_source.ts
@@ -97,6 +97,7 @@ class VectorTileSource extends Evented implements Source {
         this._tileJSONRequest = loadTileJSON(this._options, this.map._requestManager, (err, tileJSON) => {
             this._tileJSONRequest = null;
             this._loaded = true;
+            this.map.style.sourceCaches[this.id].clearTiles();
             if (err) {
                 this.fire(new ErrorEvent(err));
             } else if (tileJSON) {
@@ -132,8 +133,6 @@ class VectorTileSource extends Evented implements Source {
 
         callback();
 
-        const sourceCache = this.map.style.sourceCaches[this.id];
-        sourceCache.clearTiles();
         this.load();
     }
 

diff --git a/src/style/style.test.ts b/src/style/style.test.ts
@@ -48,6 +48,7 @@ function createGeoJSONSource() {
 }
 
 class StubMap extends Evented {
+    style: Style;
     transform: Transform;
     private _requestManager: RequestManager;
 
@@ -68,6 +69,12 @@ class StubMap extends Evented {
 
 const getStubMap = () => new StubMap() as any;
 
+function createStyle(map = getStubMap()) {
+    const style = new Style(map);
+    map.style = style;
+    return style;
+}
+
 let sinonFakeXMLServer;
 let sinonFakeServer;
 let _self;
@@ -258,7 +265,7 @@ describe('Style#loadJSON', () => {
     });
 
     test('creates sources', done => {
-        const style = new Style(getStubMap());
+        const style = createStyle();
 
         style.on('style.load', () => {
             expect(style.sourceCaches['mapLibre'] instanceof SourceCache).toBeTruthy();
@@ -276,7 +283,7 @@ describe('Style#loadJSON', () => {
     });
 
     test('creates layers', done => {
-        const style = new Style(getStubMap());
+        const style = createStyle();
 
         style.on('style.load', () => {
             expect(style.getLayer('fill') instanceof StyleLayer).toBeTruthy();
@@ -302,7 +309,7 @@ describe('Style#loadJSON', () => {
     test('transforms sprite json and image URLs before request', done => {
         const map = getStubMap();
         const transformSpy = jest.spyOn(map._requestManager, 'transformRequest');
-        const style = new Style(map);
+        const style = createStyle(map);
 
         style.on('style.load', () => {
             expect(transformSpy).toHaveBeenCalledTimes(2);
@@ -319,7 +326,7 @@ describe('Style#loadJSON', () => {
     });
 
     test('emits an error on non-existant vector source layer', done => {
-        const style = new Style(getStubMap());
+        const style = createStyle();
         style.loadJSON(createStyleJSON({
             sources: {
                 '-source-id-': {type: 'vector', tiles: []}
@@ -407,7 +414,7 @@ describe('Style#_remove', () => {
 
 describe('Style#update', () => {
     test('on error', done => {
-        const style = new Style(getStubMap());
+        const style = createStyle();
         style.loadJSON({
             'version': 8,
             'sources': {
@@ -449,7 +456,7 @@ describe('Style#setState', () => {
     });
 
     test('do nothing if there are no changes', done => {
-        const style = new Style(getStubMap());
+        const style = createStyle();
         style.loadJSON(createStyleJSON());
         jest.spyOn(style, 'addLayer').mockImplementation(() => done('test failed'));
         jest.spyOn(style, 'removeLayer').mockImplementation(() => done('test failed'));
@@ -574,7 +581,7 @@ describe('Style#addSource', () => {
     });
 
     test('fires "data" event', done => {
-        const style = new Style(getStubMap());
+        const style = createStyle();
         style.loadJSON(createStyleJSON());
         const source = createSource();
         style.once('data', () => { done(); });
@@ -585,7 +592,7 @@ describe('Style#addSource', () => {
     });
 
     test('throws on duplicates', done => {
-        const style = new Style(getStubMap());
+        const style = createStyle();
         style.loadJSON(createStyleJSON());
         const source = createSource();
         style.on('style.load', () => {
@@ -607,7 +614,7 @@ describe('Style#addSource', () => {
                 done();
             }
         };
-        const style = new Style(getStubMap());
+        const style = createStyle();
         style.loadJSON(createStyleJSON({
             layers: [{
                 id: 'background',
@@ -795,7 +802,7 @@ describe('Style#addLayer', () => {
     });
 
     test('throws on non-existant vector source layer', done => {
-        const style = new Style(getStubMap());
+        const style = createStyle();
         style.loadJSON(createStyleJSON({
             sources: {
                 // At least one source must be added to trigger the load event
@@ -864,7 +871,7 @@ describe('Style#addLayer', () => {
     });
 
     test('reloads source', done => {
-        const style = new Style(getStubMap());
+        const style = createStyle();
         style.loadJSON(extend(createStyleJSON(), {
             'sources': {
                 'mapLibre': {
@@ -891,7 +898,7 @@ describe('Style#addLayer', () => {
     });
 
     test('#3895 reloads source (instead of clearing) if adding this layer with the same type, immediately after removing it', done => {
-        const style = new Style(getStubMap());
+        const style = createStyle();
         style.loadJSON(extend(createStyleJSON(), {
             'sources': {
                 'mapLibre': {
@@ -928,7 +935,7 @@ describe('Style#addLayer', () => {
     });
 
     test('clears source (instead of reloading) if adding this layer with a different type, immediately after removing it', done => {
-        const style = new Style(getStubMap());
+        const style = createStyle();
         style.loadJSON(extend(createStyleJSON(), {
             'sources': {
                 'mapLibre': {

diff --git a/src/ui/control/logo_control.test.ts b/src/ui/control/logo_control.test.ts
@@ -34,6 +34,7 @@ function createSource(options, logoRequired) {
             _skuToken: '1234567890123',
             canonicalizeTileset: tileJSON => tileJSON.tiles
         },
+        style: {sourceCaches: {id: {clearTiles() {}}}},
         transform: {angle: 0, pitch: 0, showCollisionBoxes: false},
         _getMapId: () => 1
     }as any as Map);