mapbox / mapbox/mapbox-gl-js

Perform automatic type conversion in expressions when unambiguous

Open
#9,501 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

needs discussion :speech_balloon:
Dominant language
TypeScript
Stars
12.4k
Forks
2.4k
PR merge metrics
No merged PRs in 30d

Description

## Motivation

Currently, expressions don't do any automatic type conversion. Except for colors, for [some reason](https://github.com/mapbox/mapbox-gl-js/issues/8713#issuecomment-541748320):

>Expressions perform only one kind of implicit type conversion: a data expression used in a context where a color is expected will convert a string representation of a color to a color value. In all other cases, if you want to convert between types, you must use one of the type conversion expression operators: to-boolean, to-number, to-string, or to-color.

This means that expressions frequently become very verbose. For instance, I always have to wrap `['feature-state', 'selected']` in `to-boolean` so that undefined (never set) states get coerced to null.

Another example is given [here](https://stackoverflow.com/questions/61020660/how-to-write-a-mapbox-paint-expression-that-accounts-for-zoom-feature-state-an/61022285?noredirect=1#comment108033256_61022285) where my answer is almost readable, and the one with all the type conversions is...not.

For me, even though I write Mapbox-GL expressions all the time, I'm actually not super confident in knowing when I need one and when I don't. Often these errors don't show up until run-time. It's just a constant source of friction. (Another part of this problem is that my first attempt to perform the type conversion is often to use the `['boolean']`operator, which is wrong.)

Another case would be to automatically convert to number the arguments of `['<']` etc.

## Design

My suggestion is to perform automatic type conversion when it's unambiguous. For instance, the operators `!`, `all`, `any` and `case` all take a boolean (and only a boolean) argument. There doesn't seem to be anything stopping Mapbox-GL automatically inserting a `['to-boolean', ...]` around that argument.

I'm not sure what the downsides are. Potentially one could complain that now there is inconsistency, in some places you don't need type conversion, and some places you do. To my mind that's better than always needing it. (And obviously you can still write it if you prefer.) And that didn't seem to be a problem in the case of colors.

Or perhaps there is a small performance hit?

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

The issue does not name any files, tests, or entry points. Start by locating the expression type-checking and operator argument validation code, then determine which conversions are unambiguous and how existing expression tests cover them. Done means the conversion rules, compatibility behavior, and performance implications are defined and tested.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
frontend
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.