Skip to content

Commit 8d04baa

Browse files
wayofthefuturewayofthefuture
authored
deprecate updateable (#6819)
Co-authored-by: wayofthefuture <[email protected]>
1 parent a1d1972 commit 8d04baa

File tree

4 files changed

+49
-66
lines changed

4 files changed

+49
-66
lines changed

‎src/source/geojson_source.ts‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import {extend, warnOnce, type ExactlyOne} from '../util/util';
33
import {EXTENT} from '../data/extent';
44
import {ResourceType} from '../util/request_manager';
55
import {browser} from '../util/browser';
6-
import {applySourceDiff, isUpdateableGeoJSON, mergeSourceDiffs, toUpdateable} from './geojson_source_diff';
6+
import {applySourceDiff, mergeSourceDiffs, toUpdateable} from './geojson_source_diff';
77
import {getGeoJSONBounds} from '../util/geojson_bounds';
88
import {MessageType} from '../util/actor_messages';
99
import {tileIdToLngLatBounds} from '../tile/tile_id_to_lng_lat_bounds';
@@ -464,10 +464,9 @@ export class GeoJSONSource extends Evented implements Source {
464464

465465
// Lazily convert `this._data` to updateable if it's not already
466466
if (!this._data.url && !this._data.updateable) {
467-
if (!isUpdateableGeoJSON(this._data.geojson, promoteId)) {
468-
throw new Error(`GeoJSONSource "${this.id}": GeoJSON data is not compatible with updateData`);
469-
}
470-
this._data = {updateable: toUpdateable(this._data.geojson, promoteId)};
467+
const updateable = toUpdateable(this._data.geojson, promoteId);
468+
if (!updateable) throw new Error(`GeoJSONSource "${this.id}": GeoJSON data is not compatible with updateData`);
469+
this._data = {updateable};
471470
}
472471

473472
if (!this._data.updateable) {

‎src/source/geojson_source_diff.test.ts‎

Lines changed: 19 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,51 +1,51 @@
11
import {describe, beforeEach, test, expect} from 'vitest';
22
import {setPerformance} from '../util/test/util';
3-
import {type GeoJSONFeatureId, type GeoJSONSourceDiff, isUpdateableGeoJSON, toUpdateable, applySourceDiff, mergeSourceDiffs} from './geojson_source_diff';
3+
import {type GeoJSONFeatureId, type GeoJSONSourceDiff, toUpdateable, applySourceDiff, mergeSourceDiffs} from './geojson_source_diff';
44

55
beforeEach(() => {
66
setPerformance();
77
});
88

9-
describe('isUpdateableGeoJSON', () => {
9+
describe('toUpdateable', () => {
1010
test('feature without id is not updateable', () => {
1111
// no feature id -> false
12-
expect(isUpdateableGeoJSON({
12+
expect(toUpdateable({
1313
type: 'Feature',
1414
geometry: {
1515
type: 'Point',
1616
coordinates: [0, 0]
1717
},
1818
properties: {},
19-
})).toBe(false);
19+
})).toBeUndefined();
2020
});
2121

2222
test('feature with id is updateable', () => {
2323
// has a feature id -> true
24-
expect(isUpdateableGeoJSON({
24+
expect(toUpdateable({
2525
type: 'Feature',
2626
id: 'feature_id',
2727
geometry: {
2828
type: 'Point',
2929
coordinates: [0, 0]
3030
},
3131
properties: {},
32-
})).toBe(true);
32+
})).toBeDefined();
3333
});
3434

3535
test('promoteId missing is not updateable', () => {
36-
expect(isUpdateableGeoJSON({
36+
expect(toUpdateable({
3737
type: 'Feature',
3838
id: 'feature_id',
3939
geometry: {
4040
type: 'Point',
4141
coordinates: [0, 0]
4242
},
4343
properties: {},
44-
}, 'propId')).toBe(false);
44+
}, 'propId')).toBeUndefined();
4545
});
4646

4747
test('promoteId present is updateable', () => {
48-
expect(isUpdateableGeoJSON({
48+
expect(toUpdateable({
4949
type: 'Feature',
5050
geometry: {
5151
type: 'Point',
@@ -54,11 +54,11 @@ describe('isUpdateableGeoJSON', () => {
5454
properties: {
5555
propId: 'feature_id',
5656
},
57-
}, 'propId')).toBe(true);
57+
}, 'propId')).toBeDefined();
5858
});
5959

6060
test('feature collection with unique ids is updateable', () => {
61-
expect(isUpdateableGeoJSON({
61+
expect(toUpdateable({
6262
type: 'FeatureCollection',
6363
features: [{
6464
type: 'Feature',
@@ -77,11 +77,11 @@ describe('isUpdateableGeoJSON', () => {
7777
},
7878
properties: {},
7979
}]
80-
})).toBe(true);
80+
})).toBeDefined();
8181
});
8282

8383
test('feature collection with unique promoteIds is updateable', () => {
84-
expect(isUpdateableGeoJSON({
84+
expect(toUpdateable({
8585
type: 'FeatureCollection',
8686
features: [{
8787
type: 'Feature',
@@ -102,11 +102,11 @@ describe('isUpdateableGeoJSON', () => {
102102
propId: 'feature_id_2',
103103
},
104104
}]
105-
}, 'propId')).toBe(true);
105+
}, 'propId')).toBeDefined();
106106
});
107107

108108
test('feature collection without unique ids is not updateable', () => {
109-
expect(isUpdateableGeoJSON({
109+
expect(toUpdateable({
110110
type: 'FeatureCollection',
111111
features: [{
112112
type: 'Feature',
@@ -116,11 +116,11 @@ describe('isUpdateableGeoJSON', () => {
116116
},
117117
properties: {},
118118
}]
119-
})).toBe(false);
119+
})).toBeUndefined();
120120
});
121121

122122
test('feature collection with duplicate feature ids is not updateable', () => {
123-
expect(isUpdateableGeoJSON({
123+
expect(toUpdateable({
124124
type: 'FeatureCollection',
125125
features: [{
126126
type: 'Feature',
@@ -139,15 +139,13 @@ describe('isUpdateableGeoJSON', () => {
139139
},
140140
properties: {},
141141
}]
142-
})).toBe(false);
142+
})).toBeUndefined();
143143
});
144144

145145
test('geometries are not updateable', () => {
146-
expect(isUpdateableGeoJSON({type: 'Point', coordinates: [0, 0]})).toBe(false);
146+
expect(toUpdateable({type: 'Point', coordinates: [0, 0]})).toBeUndefined();
147147
});
148-
});
149148

150-
describe('toUpdateable', () => {
151149
test('works with a single feature - feature id', () => {
152150
const updateable = toUpdateable({
153151
type: 'Feature',

‎src/source/geojson_source_diff.ts‎

Lines changed: 23 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -60,69 +60,55 @@ function getFeatureId(feature: GeoJSON.Feature, promoteId?: string): GeoJSONFeat
6060
}
6161

6262
/**
63-
* Returns true if the data is a valid GeoJSON object that can be updated.
63+
* Converts a GeoJSON object into a map of feature IDs to GeoJSON features.
64+
* @param data - The GeoJSON object to convert.
65+
* @param promoteId - If set, the feature id will be set to the promoteId property value.
66+
* @returns A map of feature IDs to GeoJSON features, or `undefined` if the GeoJSON object is not a valid updateable object.
6467
*
6568
* Features must have unique identifiers to be updateable. IDs can come from:
6669
* - The feature's `id` property (standard GeoJSON)
6770
* - A promoted property specified by `promoteId` (e.g., a "name" property)
6871
*/
69-
export function isUpdateableGeoJSON(data: GeoJSON.GeoJSON | undefined, promoteId?: string): data is UpdateableGeoJSON {
70-
// null can be updated
72+
export function toUpdateable(data: GeoJSON.GeoJSON | undefined, promoteId?: string): Map<GeoJSONFeatureId, GeoJSON.Feature> | undefined {
73+
const updateable = new Map<GeoJSONFeatureId, GeoJSON.Feature>();
74+
75+
// null can be updated - empty updateable
7176
if (data == null) {
72-
return true;
77+
return updateable;
7378
}
7479

75-
// {} can be updated
80+
// {} can be updated - empty updateable
7681
if (data.type == null) {
77-
return true;
82+
return updateable;
7883
}
7984

8085
// a single feature with an id can be updated, need to explicitly check against null because 0 is a valid feature id that is falsy
8186
if (data.type === 'Feature') {
82-
return getFeatureId(data, promoteId) != null;
87+
const id = getFeatureId(data, promoteId);
88+
if (id == null) return undefined;
89+
90+
updateable.set(id, data);
91+
return updateable;
8392
}
8493

85-
// a feature collection can be updated if every feature has an id, and the ids are all unique
86-
// this prevents us from silently dropping features if ids get reused
94+
// a feature collection can be updated if every feature has a unique id, which prevents the silent dropping of features
8795
if (data.type === 'FeatureCollection') {
8896
const seenIds = new Set<GeoJSONFeatureId>();
97+
8998
for (const feature of data.features) {
9099
const id = getFeatureId(feature, promoteId);
91-
if (id == null) {
92-
return false;
93-
}
94-
95-
if (seenIds.has(id)) {
96-
return false;
97-
}
100+
if (id == null) return undefined;
98101

102+
if (seenIds.has(id)) return undefined;
99103
seenIds.add(id);
100-
}
101-
102-
return true;
103-
}
104-
105-
return false;
106-
}
107104

108-
export function toUpdateable(data: UpdateableGeoJSON, promoteId?: string) {
109-
const updateable = new Map<GeoJSONFeatureId, GeoJSON.Feature>();
110-
111-
// empty updateable
112-
if (data == null || data.type == null) {
113-
return updateable;
114-
}
105+
updateable.set(id, feature);
106+
}
115107

116-
if (data.type === 'Feature') {
117-
updateable.set(getFeatureId(data, promoteId)!, data);
118108
return updateable;
119109
}
120110

121-
for (const feature of data.features) {
122-
updateable.set(getFeatureId(feature, promoteId)!, feature);
123-
}
124-
125-
return updateable;
111+
return undefined;
126112
}
127113

128114
/**

‎src/source/geojson_worker_source.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import {VectorTileWorkerSource} from './vector_tile_worker_source';
99
import {createExpression} from '@maplibre/maplibre-gl-style-spec';
1010
import {isAbortError} from '../util/abort_error';
1111
import {toVirtualVectorTile} from './vector_tile_overzoomed';
12-
import {isUpdateableGeoJSON, type GeoJSONSourceDiff, applySourceDiff, toUpdateable, type GeoJSONFeatureId} from './geojson_source_diff';
12+
import {type GeoJSONSourceDiff, applySourceDiff, toUpdateable, type GeoJSONFeatureId} from './geojson_source_diff';
1313
import type {WorkerTileParameters, WorkerTileResult} from './worker_source';
1414
import type {LoadVectorTileResult} from './vector_tile_worker_source';
1515
import type {RequestParameters} from '../util/ajax';
@@ -234,15 +234,15 @@ export class GeoJSONWorkerSource extends VectorTileWorkerSource {
234234
*/
235235
async loadGeoJSONFromUrl(request: RequestParameters, promoteId: string, abortController: AbortController): Promise<GeoJSON.GeoJSON> {
236236
const response = await getJSON<GeoJSON.GeoJSON>(request, abortController);
237-
this._dataUpdateable = isUpdateableGeoJSON(response.data, promoteId) ? toUpdateable(response.data, promoteId) : undefined;
237+
this._dataUpdateable = toUpdateable(response.data, promoteId);
238238
return response.data;
239239
}
240240

241241
/**
242242
* Loads GeoJSON from a string and sets the sources updateable GeoJSON object.
243243
*/
244244
_loadGeoJSONFromObject(data: GeoJSON.GeoJSON, promoteId: string): GeoJSON.GeoJSON {
245-
this._dataUpdateable = isUpdateableGeoJSON(data, promoteId) ? toUpdateable(data, promoteId) : undefined;
245+
this._dataUpdateable = toUpdateable(data, promoteId);
246246
return data;
247247
}
248248

0 commit comments

Comments
 (0)