Drop warning thrown when image equates to "" and image expression type is not used

[tristen]

mapbox-gl-js version: v1.4.0-beta.1

Question

icon-image: "" is a valid value, so I was confused when I saw this warning thrown.

As its a common scenario to write an expression that chooses some features to use "" and others to use one, I think this sends a confusing message to a user.

Understanding the rationale behind adding it in v1.4.0 as part of the new image expression type, can we limit the warning thrown to when its in use?

Links to related documentation

• Image expression type PRs
  • Image expression #8684
  • Make image operator compatible with styleimagemissing event #8775

[ryanhamley]

can we limit the warning thrown to when its in use

I'm not sure I understand this part. What do you think would be the expected outcome of icon-image: "'? Should we just treat it as if icon-image was never defined?

[samanpwbb]

Throwing this warning seems consistent with existing warnings, for example, when a field value is the incorrect type in a get expression. I think it's useful.

I do wonder if Mapbox GL should have separate development and production modes – where, depending on your NODE_ENV value at build time, warnings either are or are not displayed. This would eliminate the concern of console spam while freeing us up to surface more helpful warnings.

[tristen]

I'm not sure I understand this part. What do you think would be the expected outcome of icon-image: "'? Should we just treat it as if icon-image was never defined?

Yes. It should be treated as if icon-image was never defined.

Throwing this warning seems consistent with existing warnings, for example, when a field value is the incorrect type in a get expression. I think it's useful.

An incorrect type in a get expression is an example of something wrong and yeah, a warning should be thrown. However, providing "" as a possible option is a legitimate thing to do.

I do wonder if Mapbox GL should have separate development and production modes – where, depending on your NODE_ENV value at build time, warnings either are or are not displayed. This would eliminate the concern of console spam while freeing us up to surface more helpful warnings.

Yeah, that crossed my mind too (similar to how React does this) +1

[asheemmamoowala]

However, providing "" as a possible option is a legitimate thing to do

I agree with this. Even in development mode, the error message or the styleimagemissing event for this case do not make any sense - they don't help the developer understand which layer, layer-property, or for which feature the problem exists.

More so, "" is the absence of a valid image name, where image name must be 1+ alphanumeric characters.