Literal expression validation does not match documentation

[dereklieu]

The style spec documentation for literal expressions says it requires an array or object in the second argument. However, in Studio, an expression like the one below passes validation.

[ "literal", "#333333" ]

mapbox-gl-js version: @mapbox/mapbox-gl-style-spec@13.11.0

browser: N/A

Steps to Trigger Behavior

Save this style, upload to Studio, and try clicking on the single layer.

{
    "layers": [
        {
            "id": "this-be-the-problem",
            "layout": {},
            "paint": {
                "fill-extrusion-color": [
                    "case",
                    [
                        "boolean",
                        false
                    ],
                    "#333333",
                    [
                        "literal",
                        "#999999"
                    ]
                ]
            },
            "source": "composite",
            "source-layer": "building",
            "type": "fill-extrusion"
        }
    ],
    "name": "Problematic Literal",
    "sources": {
        "composite": {
            "type": "vector",
            "url": "mapbox://mapbox.mapbox-streets-v8"
        }
    },
    "version": 8
}

I'm sorry I don't have a better way to illustrate this, but Studio uses validator functions when you try uploading any style.

This causes issues in our json editor, which enforces Studio limitations around object literals and only allows arrays in literal expressions.

I'm not sure how long this behavior has been around, but it's likely that stricting up this validation could break production styles. It would be worth seeing how many styles this would affect, cc @samanpwbb.

[asheemmamoowala]

@dereklieu literal expression are most useful when working with literal arrays or objects, but can also be used to represent any other type of Javascript literal.

Here's a comment from the parsing code:

mapbox-gl-js/src/style-spec/expression/parsing_context.js

Lines 122 to 125 in d5bce5a

	// If an expression's arguments are all literals, we can evaluate
	// it immediately and replace it with a literal value in the
	// parsed/compiled result. Expressions that expect an image should
	// not be resolved here so we can later get the available images.

Internally, some expressions may be computable at compile time and folded into constants, and continue to be represented by literal expressions. This is a legitimate use case.

I think the documentation is correct to recommend using literal for only the cases where it is appropriate. Could this be solved by just updating the documentation?

cc @ryanhamley

[dereklieu]

Sounds good - this seems like a docs issue then. We will loosen the validation causing this error in Studio.

[ryanhamley]

I'm confused by the Studio validation on this. This is much stricter than how the literal expression works.

The expression will accept any value type

mapbox-gl-js/src/style-spec/expression/definitions/literal.js

Lines 21 to 44 in 517b39f

	static parse(args: $ReadOnlyArray<mixed>, context: ParsingContext) {
	if (args.length !== 2)
	return context.error(`'literal' expression requires exactly one argument, but found ${args.length - 1} instead.`);
	if (!isValue(args[1]))
	return context.error(`invalid value`);
	const value = (args[1]: any);
	let type = typeOf(value);
	// special case: infer the item type if possible for zero-length arrays
	const expected = context.expectedType;
	if (
	type.kind === 'array' &&
	type.N === 0 &&
	expected &&
	expected.kind === 'array' &&
	(typeof expected.N !== 'number' || expected.N === 0)
	) {
	type = expected;
	}
	return new Literal(type, value);
	}

This function shows what is considered a valid value:

mapbox-gl-js/src/style-spec/expression/values.js

Lines 34 to 68 in 517b39f

	export function isValue(mixed: mixed): boolean {
	if (mixed === null) {
	return true;
	} else if (typeof mixed === 'string') {
	return true;
	} else if (typeof mixed === 'boolean') {
	return true;
	} else if (typeof mixed === 'number') {
	return true;
	} else if (mixed instanceof Color) {
	return true;
	} else if (mixed instanceof Collator) {
	return true;
	} else if (mixed instanceof Formatted) {
	return true;
	} else if (mixed instanceof ResolvedImage) {
	return true;
	} else if (Array.isArray(mixed)) {
	for (const item of mixed) {
	if (!isValue(item)) {
	return false;
	}
	}
	return true;
	} else if (typeof mixed === 'object') {
	for (const key in mixed) {
	if (!isValue(mixed[key])) {
	return false;
	}
	}
	return true;
	} else {
	return false;
	}
	}

We might want to adjust the documentation to make it clearer that the literal expression is valid with all types, but it seems like the Studio validation may be too strict as well.

Edit: I hadn't seen @dereklieu 's comment about loosening the validation before posting this but it seems like we're on the same page then

[dereklieu]

Yeah @ryanhamley my guess based on the code is that we followed the docs which gives the strong impression that you're only supposed to use arrays and objects. Studio doesn't support object types yet, hence the validation.