Mapbox-gl-js: Validation of "format" expression fails when options are provided

Created on 21 May 2019  路  5Comments  路  Source: mapbox/mapbox-gl-js

mapbox-gl-js-style-spec version: 13.7.0 (also tested against versions from 13.1.0 onward)

Steps to Trigger Behavior

  1. Clone this repo, cd to src/style-spec/, run npm install and npm run build
  2. Use the gl-style-validate to validate a style that has a text-filed "format" expression, such as the one in test/integration/render-tests/text-field/formatted-text-color/style.json:
    ./bin/gl-style-validate ../../test/integration/render-tests/text-field/formatted-text-color/style.json

  3. Observe an unexpected validation error:
    layers[0].layout.text-field[1]: Bare objects invalid. Use ["literal", {...}] instead.

Expected Behavior

Running gl-style-validate on a style that contains a text-field "format" expression containing a valid options object should pass.

Actual Behavior

Running gl-style-validate on a style that contains a text-field "format" expression containing results in an error message: layers[0].layout.text-field[1]: Bare objects invalid. Use ["literal", {...}] instead.

jamesseppi at C02W42ZZHV2Q in ~/CODE/mapbox-gl-js/src/style-spec (release-liquid) 
$ ./bin/gl-style-validate ../../test/integration/render-tests/text-field/formatted-text-color/style.json 
../../test/integration/render-tests/text-field/formatted-text-color/style.json:40: layers[0].layout.text-field[1]: Bare objects invalid. Use ["literal", {...}] instead.

If the options are instead changed to be empty objects, then the validation succeeds. For example, if the linked test fixture is modified to have the following "format" expression, then the validation succeeds:

"text-field": ["format",
    ["get", "name_en"], [{}],
    "Italy", { } ,
    "\n", {},
    ["get", "name"], {},
    "Italia", {}
]

cc @asheemmamoowala @tristen

bug

Most helpful comment

Hi @jseppi, I got some help from @asheemmamoowala and found the root cause for this issue. It's all about the Javascript quirk that Number(...) has a different runtime reflection type than new Number(...) I'm preparing a PR and it should be ready for review by tomorrow. I think it's possible the behavior I found creates other validation bugs so I want to just finish checking on that.

All 5 comments

I started a branch with a currently-failing test fixture: 8271-format-options-validation, though I have not been successful in figuring out how to fix the bug.

@jseppi Hi, I'm super new and I tried to follow all the associated library version upgrades, but I don't have enough context to understand all the teams and software components that needed updates or were updated. I can dig in on this, it will just take me a few tries and some time (and gaining a bunch of context that I need eventually anyway).

Is it correct that the format expression bug itself is fixed in patches we already have for mapbox-gl-style-spec? If so, are you waiting on gl-js to integrate something so you can resolve this?

@ahk Apologies for any confusion on my part, and thanks for taking a look!

Is it correct that the format expression bug itself is fixed in patches we already have for mapbox-gl-style-spec?

No, that is the bug that this issue is reporting. The format expression does not properly validate with the gl-style-validate tool from the current version (13.7.0) of mapbox-gl-style-spec.

That's what it looked like in your comment.

The expressions mentioned in that comment (text-radial-offset, text-variable-anchor, and text-justify: "anchor") work and are validated correctly. format is the one that does not get properly validated.

Hey there @ahk - checking on any progress here

Hi @jseppi, I got some help from @asheemmamoowala and found the root cause for this issue. It's all about the Javascript quirk that Number(...) has a different runtime reflection type than new Number(...) I'm preparing a PR and it should be ready for review by tomorrow. I think it's possible the behavior I found creates other validation bugs so I want to just finish checking on that.

Was this page helpful?
0 / 5 - 0 ratings