Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Coalesce expression doesn't work as expected #5755

Closed
muesliq opened this issue Nov 27, 2017 · 3 comments
Closed

Coalesce expression doesn't work as expected #5755

muesliq opened this issue Nov 27, 2017 · 3 comments
Labels

Comments

@muesliq
Copy link

muesliq commented Nov 27, 2017

mapbox-gl-js version: 0.42.2

Steps to Trigger Behavior

Use this code:

layout: {"text-field": ["coalesce", ["get", "fieldA"], ["get", "fieldB"]]}

where fieldA is null and fieldB is not null

Expected Behavior

Display fieldB.

Actual Behavior

Empty label. (It works though when fieldA is not null, displaying fieldA.)

Note:

A work-around with case did the job. However this seems to be a bug. Sorry if I somehow misunderstood the Style Spec and it isn't!

@anandthakker
Copy link
Contributor

Thanks for the report @muesliq! Could you by any chance provide a live jsfiddle example reproducing this issue?

@jfirebaugh
Copy link
Contributor

This is an outcome of type inference and automatic type assertions. What's happening is that since text-field must be a string, the result of coalesce must be a string, so it's inferred that each of the two gets must also be a string. So there is a string assertion automatically inserted, as if you had written:

["coalesce", ["string", ["get", "fieldA"]], ["string", ["get", "fieldB"]]]

Then when the expression is evaluated, it's found that the first get produces null rather than a string, which causes an error and aborts the evaluation. (The visual result is that you get the default value of the property, an empty string.)

This is arguably working as intended, but I think the behavior is so surprising that we will need to revisit the design.

@anandthakker
Copy link
Contributor

Nice analysis @jfirebaugh.

I think the best fix here is to treat coalesce as a special case when performing inference, not inserting assertions for its arguments, and instead inserting an assertion around the whole coalesce expression where necessary.

anandthakker pushed a commit that referenced this issue Nov 29, 2017
anandthakker pushed a commit that referenced this issue Nov 29, 2017
anandthakker pushed a commit that referenced this issue Nov 29, 2017
anandthakker pushed a commit that referenced this issue Nov 29, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants