Do not warn about icon-image expressions evaluating to '' (#9380)

* do not warn on empty icon image, treat it as unset * clean up hasIcon condition in symbol bucket
mapbox · Mar 5, 2020 · fa36168 · fa36168
1 parent 2374fe3
commit fa36168
Show file tree

Hide file tree

Showing 4 changed files with 35 additions and 9 deletions.
diff --git a/src/data/bucket/symbol_bucket.js b/src/data/bucket/symbol_bucket.js
@@ -81,7 +81,7 @@ export type CollisionArrays = {
 export type SymbolFeature = {|
  sortKey: number | void,
  text: Formatted | void,
- icon: ResolvedImage | void,
+ icon: ?ResolvedImage,
  index: number,
  sourceLayerIndex: number,
  geometry: Array<Array<Point>>,
@@ -415,7 +415,7 @@ class SymbolBucket implements Bucket {
  // this allows us to fire the styleimagemissing event if image evaluation returns null
  // the only way to distinguish between null returned from a coalesce statement with no valid images
  // and null returned because icon-image wasn't defined is to check whether or not iconImage.parameters is an empty object
- const hasIcon = (iconImage.value.kind !== 'constant' || !!iconImage.value.value) && Object.keys(iconImage.parameters).length > 0;
+ const hasIcon = iconImage.value.kind !== 'constant' || !!iconImage.value.value || Object.keys(iconImage.parameters).length > 0;
  const symbolSortKey = layout.get('symbol-sort-key');
 
  this.features = [];
@@ -453,7 +453,7 @@ class SymbolBucket implements Bucket {
  }
  }
 
- let icon: ResolvedImage | void;
+ let icon: ?ResolvedImage;
  if (hasIcon) {
  // Expression evaluation will automatically coerce to Image
  // but plain string token evaluation skips that pathway so do the

diff --git a/src/style-spec/expression/definitions/image.js b/src/style-spec/expression/definitions/image.js
@@ -30,13 +30,11 @@ export default class ImageExpression implements Expression {
 
  evaluate(ctx: EvaluationContext) {
  const evaluatedImageName = this.input.evaluate(ctx);
- let available = false;
 
- if (ctx.availableImages && ctx.availableImages.indexOf(evaluatedImageName) > -1) {
- available = true;
- }
+ const value = ResolvedImage.fromString(evaluatedImageName);
+ if (value && ctx.availableImages) value.available = ctx.availableImages.indexOf(evaluatedImageName) > -1;
 
- return new ResolvedImage({name: evaluatedImageName, available});
+ return value;
  }
 
  eachChild(fn: (_: Expression) => void) {

diff --git a/src/style-spec/expression/types/resolved_image.js b/src/style-spec/expression/types/resolved_image.js
@@ -18,7 +18,8 @@ export default class ResolvedImage {
  return this.name;
  }
 
- static fromString(name: string): ResolvedImage {
+ static fromString(name: string): ResolvedImage | null {
+ if (!name) return null; // treat empty values as no image
  return new ResolvedImage({name, available: false});
  }
 

diff --git a/test/unit/ui/map.test.js b/test/unit/ui/map.test.js
@@ -2075,6 +2075,33 @@ test('Map', (t) => {
  });
  });
 
+ t.test('map does not fire `styleimagemissing` for empty icon values', (t) => {
+ const map = createMap(t);
+
+ map.on('load', () => {
+ map.on('idle', () => {
+ t.end();
+ });
+
+ map.addSource('foo', {
+ type: 'geojson',
+ data: {type: 'Point', coordinates: [0, 0]}
+ });
+ map.addLayer({
+ id: 'foo',
+ type: 'symbol',
+ source: 'foo',
+ layout: {
+ 'icon-image': ['case', true, '', '']
+ }
+ });
+
+ map.on('styleimagemissing', ({id}) => {
+ t.fail(`styleimagemissing fired for value ${id}`);
+ });
+ });
+ });
+
  t.end();
 });