Style validation and TypeScript

[smellyshovel]

TL;DR

There is no proposal to use javascript as part of the style here. The proposal is to move away from v8.json and its (proprietary) syntax that currently defines the style specification to a tool that already defines types well - typescript. I.e. instead of having an obscure syntax that no one beside mapbox/maplibre uses and no one actually knows its rules and how to properly add new types, to something very common - typescript - to define those types (and also do the validation well, probably better than what's written at the moment).
There are no plans to change the style spec also. Just how we check it (i.e. check it only prior to sending it to maplibre and not checking it just before applying it to the map).

---

Original text

During the discussion in #1805 (namely this one) it became clear to me that it's not only me who dislikes the current state of things with validation of syles (and the way the spec is defined in general).

The more I read the less it feels like the current solution is the right one.

To describe in short how it works (correct me if I'm wrong at something):

1. Style spec is defined as a json file with some very specific syntax
2. Then this json file is parsed and the types.g.ts file is generated from
3. This types.g.ts file is referenced across all the codebase

I feel like such an approach is... a bit of an overkill.

So I was wondering whether it would be possible to get rid of the first 2 steps and instead define all the style spec manually, writing a .d.ts file by hand.

Also, I suggest to remove all the runtime validation and rely purely on TypeScript. The users of GL JS who care use TypeScript in their consumer-projects anyway. For those who don't, we might release a separate tool to validate the whole style json or even separate parts of it. It could be either a web-based solution (something like TS playground), where you manually copy-paste your style and it tells you what's wrong about it. Or it could be a CLI-tool of some sort.

The pros I can see:

1. Graetly reduced bundle size. All the validations-related code takes a real huge space in the total bundle size.
2. Runtime-performance benefits. No runtime checks - better performance, pretty much obvious point I'd say.
3. Mainetnance and support simplicity. The current solution is really hard to get your head around of.
4. TypeScript is definitely way more flexible than the existing sort of "proprietary" typing (in terms of the spec) soluion.

The cons:

1. Harder to use GL JS in projects that don't use TypeScript. You won't get an error or a warning niether at compile time nor at runtime. Might be hard to debug, but that's why I mentioned a separate validation-tool above.
2. ?

At the time being it's purely an idea. Would like to discuss this with the community, and any feedback is highly appreciated. Especially, taking into account what Harel has mentioned: the spec in its current version is now used not only in GL JS, but also in Native and Docs, so I'm not even sure how realistic is my idea at all.

[HarelM]

@maxammann, @ntadej your feedback would be much welcomed here for native and rust stuff that I might not know how they work.
I personally prefer "compile time checking" over runtime.
@cndv3996 @westinrm @ambientlight @drwestco I would love your take on this as well, I'm sure I forgot other good people here, I'll post this in the slack channel as well.
Style-spec tool can be simply a tsc command using the right definitions I believe.
I think we shouldn't invent a type system, rules, validatiin etc when it exists and we are using it.

There are cons, don't get me wrong, but I think @smellyshovel wrote it well and I generally agree.

[davychxn]

@AdamSzofranMSFT @dunclaw @dbuerer @kircher1 @zhangyiatmicrosoft

I'm a newbie, so, I help you loop more guys that might have more opinions.

[ntadej]

I will process all the discussion some other time, but for now my opinion:

Personally I would go with standalone validation. Styles do not update that often (or do they?). On the other hand what about custom layers? They usually use a subset of the style spec, right? Anyways what would be the behaviour of the client if the style is not OK? This is anyways not something desired in production that there is a possibility of a broken style.

[nyurik]

If I understood the proposal, I believe there are significant disadvantages to using javascript as part of the style:

• we have maplibre-native and an upcoming maplibre-rs implementations -- neither of which would want to use a whole javascript engine as part of their library
• there are other implementations like openlayers that also support this language, and this would break compatibility.
• javascript is powerful, but cannot be executed directly on GPU - whereas we can optimize (at least in theory) some declarative code to run on GPU
• in many hosting usecases, the style is created by the untrusted parties. Allowing users to write any javascript in a style would compromise those sites. Example: let's say Wikipedia allows user-contributed styles that are shown to others. A malicious user creates/modifies a style. Another person logs in and sees that map, and the stylesheet's javascript would have access to the login info of that person, able to do malicious things as another user.

[drwestco]

You're assuming the site/app developer is also using TypeScript. That's not a requirement of using the gl-js map control.

[smellyshovel]

@drwestco as I mentioned, for those who don't use it, there gonna be a standalone tool.

[smellyshovel]

As the worst case scenario (no TS, script-generated style objects) - save to a variable, log it, copy what's logged, paste into the tool. Done. Too much hassle? Please, install TS to automate the process. Everything's pretty much straightforward. I personally don't like the idea of loosing performance points just because someone's too lazy to use a separate tool and don't want to introduce TS in their project.

[drwestco]

Performance is an interesting point. Has anyone measured the performance impact of the validation step when loading style? There's an optional param to style.loadURL and style.loadJSON to skip the validation step, though I can't find any code that's taking advantage.

The rest of the comment, "go install TS" or "go use a separate tool", is pretty developer-hostile IMO.

[smellyshovel]

"go install TS" is pretty developer-hostile IMO

Maybe, yes.

"go use a separate tool" is pretty developer-hostile IMO

Cannot agree. Basically, now it's the same though vice-versa: go disable validation when you're done debugging.

Once again, I'm pretty much against the idea of using TS-only spec/validation (because it turns out to be quite impossible taking into account the other implementations' limitations). It's now more of a strive to find a better solution than the existing one, because, as many guys here agreed, it's far from being perfect and is definitely a point to improve.

So instead of uselessly arguing on the pros/cons of using TS-only validation (uselessly - because it seems like it anyway won't be the final decision), I believe it would be better to find an adequate alternative to what we have now.

[drwestco]

Not sure how eliminating v8.json in favor of hand-writing types.g.ts (or its equivalent) eliminates the need for run-time validation. The style spec allows for constraints and multi-property relationships that go beyond basic type checking. Ultimately, the map control consumes a style JSON that needs to be parsed and validated - this is always going to be a run-time step.

[drwestco]

min/max on numeric properties

"function_stop": {
  "minimum": 0,
  "maximum": 24
}

array properties can have length or min-length constraints, though it looks like min-length isn't currently used.

there are some properties that only allow expressions to refer to zoom and/or feature-state; others that can access any feature property.

fill-color has "requires": [ { "!": "fill-pattern" } ] stating you can have either fill-color or fill-pattern, but not both. Not sure if the validator checks this, or if the code just uses the first property it finds.

[HarelM]

Parsed and validated are two different steps. What if we only parse at runtime and leave the validation to the user to decide. I believe in most cases you don't really define the style, or input it dynamically, you use a predefined, hard coded style.
In the "dynamic" cases we might consider creating a tool that could be used in those situations.
The current validation is far for covering all corner cases.

[smellyshovel]

@drwestco
Min/Max - you're right
Array lengths - achievable with TS
Having some fields while not having some other ones - I think that's the whole purpose of TS.

So, so far I'd say we only need to implement a min/max integer runtime check. Everything else could be covered by TS>

[drwestco]

I'm not aware of Typescript decoration to specify min/max array lengths. Fixed lengths, yes. Also not sure how you'd set up a type that has "this" or "that" but not both.

Right now, the control behavior if validateStyle finds errors is to report it to the JS console and stop loading the style. If that step is eliminated and the JSON is consumed, how does the control handle invalid style information?

[smellyshovel]

specify min/max array lengths

I'm pretty much sure I saw something related one day. I may be mistaken, but still noone stops us from writing all the possible tuple variations by hand. Ugly? Yes. Does its thing? Also yes. Possible? Well, up to a certain upper limit I'd say yes. It's no the best idea for sure, and I'll research more on the topic.

If that step is eliminated and the JSON is consumed, how does the control handle invalid style information?

Breaks at some point as if it would if there were no validation at all. Once again, we're removing it from the runtime. If the user cares that the style might not work for some reason, they'd either: a) enable TypeScript b) use a 3rd-party tool to check what's wrong about their style declaration.

[achou11]

Having attempted some work on the style spec package, I generally agree that the way things are set up are a little hard to work with. My suggestions would be:

• Create a separate module for the style spec + validator. the way it's currently set up is pretty odd based on my brief experience 😅 in general, having the option to do runtime validation is super important because you could be working with untrusted sources, as @nyurik alluded to. there should be a modular way of introducing style validation, whether it's for this library or any other applications.

• Represent the schema using something like JSON Schema and then ingest that file to create a validator in JS/TS via something like ajv. The schema representation in a (relatively) language-agnostic format is generally important for the sake of being used by other language implementations

[smellyshovel]

@achou11 got your point.
I agree that a more general schema-based approach might be even more flexible (and feasible) in the end of the day. The reason I dislike it though, is that it requires a runtime-check step. And probably the main part of my idea was to get rid of any (or almost any) runtime checks. So basically, what I'm trying to say is that if you manage to find something like ajv, but that would perform all the checks in compile time, rather than at runtime - I'm all in. But I don't know any of them, so that's why I stick to the TS option.

Still, huge thanks for your feedback, I appreciate it 👍

[achou11]

Yeah I see your concern. I think the way I'm thinking about this from an architecture standpoint is:

1. Write your schema using a JS module (ajv, typebox, etc) that can:
   • generate a TS declaration file
   • generate a schema file
2. The TS declaration file can be used for the compile-time checks in TS-compatible projects, just as you want :)
3. The schema file can be used to create a validator (runtime and/or compile time) in whatever language, as long as the language has some module/tooling to consume the format

[nyurik]

Knowing schema at the compile time is not always possible. For example, maputnik or any other schema editor would need to update the schema in real time. Also, in some federated/hosting cases the schema is stored on a different host than the code. And stored as an unverified/insecure json blob.

[thehoneymad]

Couple of things here. Speaking from the native's perspective.

1. The style document, which is pure JSON is passed to rendering layer as is per layer. As in a Style document is a collection of layers and how they wish to be rendered.
2. The style schema also works differently based on different tile schema. Creating compile time classes for all possible layers from all possible tile schema is hard.
3. A compiled object will also be hard to pass through a native interface from Android, iOS to the native rendered without not having to be limited by the native language (C++) restraint who recieves it. Generally a CString or any serialized string is a better format for these.
4. This also extends to expressions. They are also pretty dynamic and will be hard to guarantee compile time safety.

Now, that being said, we can write builders atop the schema that allows compile time safe style document writing. But once again, will be somewhat limited.

[StylianosVitalis-TomTom]

I'd vote a +1 on using JSON Schema. CityJSON uses and, from my experience, it has its limitations but it's also now some sort of a standard and, thus, at least makes the solution less proprietary. I understand that this wasn't the original intention in this discussion, but I cannot see how the other shortcomings of a pure TS solution (i.e. non-language agnostic, not supporting certain features such as metadata per property) can be overcome.

Why do we have this schema at all? It's purpose is validation only, right? So why bother at all? If we write types in TS - all the implementations (JS, Native, Rust, whatever else) can simply run TS-compiler checks as the step #1 before going any further. Or am I missing something?

@smellyshovel I am not sure if you would count it as validation, but JSON Schema can also be used for linting in an editor (e.g. VS Code). There is also Schema store which effectively helps users using any common editor to lint without requiring any specific configuration.

Knowing schema at the compile time is not always possible. For example, maputnik or any other schema editor would need to update the schema in real time. Also, in some federated/hosting cases the schema is stored on a different host than the code. And stored as an unverified/insecure json blob.

@nyurik If JSON Schema is used, then extensions are still possible. It all depends on what whether non-defined properties are allowed on every individual object. CityJSON uses a custom wrapper around JSON Schema's validator that basically allows the schema definition to be appended. So, a file will point to the schema definitions of the extended properties. I have to admit, from my experience, this doesn't always work without its limitations, but it's definitely possible.

[westinrm]

TLDR: I don't think typescript is a usable replacement for the JSON file due to extra spec information and code gen for other languages.

List of things in the style spec JSON that can't be represented with TS types, and are only usable at runtime because the whole JSON object is exported at runtime:

• Default values
• Validity range for numeric values
• Expression capabilities and requirements
• SDK versions
• Property exclusions
• Property dependencies
• Source dependencies

You could include this information as a separate const JS object or separate JSON, but that kind of defeats the purpose of removing the style spec JSON and probably makes the status quo worse. Of these the default values and (I think) expression capabilities/requirements are critical information when it comes to generating the property evaluators, which means it's needed in every language that wants to evaluate styles at runtime.

Overall while I agree that the current system is a pain to work with when you're working in TS, having that JSON file is critical to the already existing TS code (for defaults) and also critical for other platforms, where it's used for code generation. While it may be limiting, it also doesn't force those other platforms to have to support deserializing extremely complex TS types.

For example, I've had to work on a Java tool that converts a different styling language into the Mapbox styling language, and being able to relatively easily generate a strongly typed style spec was very nice. If the spec was in typescript I'm not even sure if there is a good library for parsing and reasoning about typescript definitions outside the typescript compiler itself.

[HarelM]

Thanks for the input, those are very good points to check. I'm not sure all of them are relevant, for example sdk version is for documentation only and not checked in run time as far as I know.
Ramge can be achieved with typesceipt, etc.
But default value for example is important and I'm not sure there's a good alternative.
I need to verify that the dependencies are actually checked and not there only for documentation reasons.
Generally speaking, we need to create a table representing pro, cons, features that are missing in each implementation and see what weight we give each line and formulate a decision.
Thanks a lot for taking the time to answer here in details!

[HarelM]

@westinrm just to share what I suspected: while the current json might be defining things related to dependencies and exclusion, there's no actual checks in the code, and the map is render as best effort from what I can tell.
Here's an example with the property you linked as a dependency requirement: icon-translate (requires icon-image):
The following code doesn't send any warning to the console as far as I can tell.
This might power the docs, but I personally think JsDocs can do a better job and reduce learning curve.
While I'm not saying typescript is the solution, as @smellyshovel also mentioned, I think we should consider simplifying the current state, as it doesn't live to what it should do on one hand, but is over complicated (at least from my point of view) on the other hand.

var map = new maplibregl.Map({
        container: 'map', // container id
        style: {
            'version': 8,
            'glyphs': "https://demotiles.maplibre.org/font/{fontstack}/{range}.pbf",
            'sources': {
                'raster-tiles': {
                    'type': 'raster',
                    'tiles': [
                        'https://stamen-tiles.a.ssl.fastly.net/watercolor/{z}/{x}/{y}.jpg'
                    ],
                    'tileSize': 256,
                    'attribution':
                        'Map tiles by <a target="_top" rel="noopener" href="http://stamen.com">Stamen Design</a>, under <a target="_top" rel="noopener" href="http://creativecommons.org/licenses/by/3.0">CC BY 3.0</a>. Data by <a target="_top" rel="noopener" href="http://openstreetmap.org">OpenStreetMap</a>, under <a target="_top" rel="noopener" href="http://creativecommons.org/licenses/by-sa/3.0">CC BY SA</a>'
                },
                'geo': {
                  'type': "geojson",
                  'data': {
                    'type': "Feature",
                    'geometry': {
                      'type': "Point",
                      'coordinates': [-74.5, 40]
                    },
                    'properties': {}
                  }
                }
            },
            'layers': [
                {
                    'id': 'simple-tiles',
                    'type': 'raster',
                    'source': 'raster-tiles',
                    'minzoom': 0,
                    'maxzoom': 22
                },
                {
                	'id': 'geo-l',
                  'type': 'symbol',
                  'source': 'geo',
                  'paint': {
                  	'icon-translate': [5, 5]
                  },
                  'layout': {
                     'text-field': 'hello!!'
                  }
                }
            ]
        },
        center: [-74.5, 40], // starting position
        zoom: 2 // starting zoom
    });

[westinrm]

I would argue that jsdoc is significantly less structured than the existing API version / dependency / expression capabilities / etc, and would be strictly worse for what should be a language agnostic specification, and also strictly worse for generating a helpful HTML docs page.

[rotu]

To my knowledge, the runtime tooling for type declaration (d.ts) is actually pretty poor. Typescript doesn’t provide any runtime representation of the type declarations.

[maxammann]

Schema definition

Using something like JSON Schema could definitely be worth it to move away from e.g. the v8.json.

The TypeScript type system is Turing complete. So it actually should be possible to perform arbitrary checks. Though, that would result in very very ugly TypeScript type definitions. JSON Schema is exactly made for that purpose so it should be preferred.

Schema Validation

Schema Validation should ideally happen at compile time of an application which uses maplibre (native or web). Schema validation must happen to a certain extent at runtime. The extent can vary though. It can be on a syntax level, and just let the application crash if a property is missing. Alternatively, we can so semantic checks at runtime which would catch any errors during the parsing stage of maplibre.

There is no reason why checks at compile time could not also be done at runtime. We just need to include the validator in the runtime (maybe only optionally).

TL;DR: There are different types of validation:

• Syntax vs. Semantically
• Runtime vs. Compile time

Styling Language

For maplibe-rs I actually have a much greater change in mind. I actually would love to move away from JSON as a language completely. After all the style specification is nothing more or less than a language.
Designing languages is hard. Though, I would love to see research be done on it as part of maplibre-rs.
I don't really want to dive deeper into this topic here, because we are in the gl-js repo.

[achou11]

totally agree with your thoughts on validation.

not to distract you but re: styling language, have you seen Felt's? Still JSON-based but maybe some interesting inspiration there :)

https://felt.com/blog/felt-style-language

[smellyshovel]

Thanks, short and on point.
Rises more questions than it answers though TBH.
I think we should comprise all the suggested ideas/proposals into a single table of some sort and let all the community decide what to do.
After all, I feel like (almost) everyone agrees that something must be done. It's only a matter now of finding exactly what.

[smellyshovel]

There is no reason why checks at compile time could not also be done at runtime.

But is there really a reason to duplicate these checks at runime?

[maxammann]

Yes, I already talked to them some time ago :)

[maxammann]

But is there really a reason to duplicate these checks at runime?

That depends. You absolutely have certain checks always. E.g. the syntactical checks are present in the web because we are parsing a JSON. That checks for correct syntax. Similarly, on native we have a JSON library which checks the syntax.

For semantics we have 2 options I guess:

• Validate all properties of the style (for example that a property is and integer and is in range (a..b)
• Do not validate the properties but expect that they are fine at runtime of the map rendered. This will cause crashes at an arbitrary point.

For each of the two options above we can do them at compile time, runtime or both. I would propose to provide an option to do the checks at compile time (e.g. a tool for linting the style). That same tool can also be used at runtime (for example when running in debug mode). We can allow to turn that off in production if there is a performance impact of the validation.

It would be great to write such a library in Rust/C++ which can easily be compiled to WebAssembly. Then we can just reuse the same library in the Web and the Native part of MapLibre.
*

[HarelM]

After some conversations in the TSC meetings and some googling it seems that JSON Schema is the right tool for the job, after all the style.json file is a json file and it makes sense to validate it using json schema, I think that some IDEs also support auto complete for json schema defined json files (I've seen this for angular I believe).
It might not be super mature, but I think it should do the trick.
It has validation in a lot of languages already which is good.
@louwers @ovivoda it would be great to have your input here (in continue to the TSC meeting) as to how this v8 part is used in native and how much effort would it take to replace it.
We already have some changes in the style-spec that need their native counterpart so it's worth looking into it from this aspect as well.

[louwers]

Should the style spec definition move to its own repo? This was a point of discussion during the last TSC meeting.

[davychxn]

I don't feel strongly about a separate repo, but I do feel it should be at least a separate workspace.

The specification versus implementation has gotten somewhat fuzzy in places - see #2054.

Are we expecting the code generator programs generate-style-code.ts and generate-style-spec.ts to be part of the style spec or the maplibre-gl-js repos? How about their output?

Hi rotu,

As Harel shared, make the Spec a NPM package. The scripts you mentioned can be separated to part of the tools/libs in the package.

In ML project, we can import the package and use as a module. How about this? I'm still a newbie. 😅

[rotu]

As Harel shared, make the Spec a NPM package. The scripts you mentioned can be separated to part of the tools/libs in the package.

Sorry, I'm still unclear. There is already a npm package @maplibre/maplibre-gl-style-spec.

This package:

• has a manifest (package.json) file with no build or test scripts
• contains the v8.json file
• contains the code generate-style-code.ts emits but not the script itself.
• does not contain either the code emitted by generate-style-spec.ts nor the script itself.

When you use the word "spec", that could be as specific as the v8.json file or as general as the code generation scripts and build tools surrounding that file.

[davychxn]

As Harel shared, make the Spec a NPM package. The scripts you mentioned can be separated to part of the tools/libs in the package.

Sorry, I'm still unclear. There is already a npm package @maplibre/maplibre-gl-style-spec.

This package:

• has a manifest (package.json) file with no build or test scripts
• contains the v8.json file
• contains the code generate-style-code.ts emits but not the script itself.
• does not contain either the code emitted by generate-style-spec.ts nor the script itself.

When you use the word "spec", that could be as specific as the v8.json file or as general as the code generation scripts and build tools surrounding that file.

I see, still a lot for me to learn. Thanks for sharing.

[acalcutt]

Theres also a few bin scripts included in the style-spec npm package we just got working again
https://github.com/maplibre/maplibre-gl-js/blob/main/src/style-spec/package.json#L25-L28
https://github.com/maplibre/maplibre-gl-js/tree/main/src/style-spec/bin

When I was testing building it I followed the steps in the workflow
https://github.com/maplibre/maplibre-gl-js/blob/main/.github/workflows/publish-style-spec.yml

I had worked with style-spec a bit because it gets used as a npm package in Tileserver-GL since I moved it to maplibre-native.

[HarelM]

If the separate package is a json-schema definition every implementation can use the relevant tools to extract the relevant code according to language.
We can decide to put the generated code as part of this repo and have this repo create the relevant npm/rust packages, or not, depends on how we decide. I don't think there's a right and wrong.
The fact that style-spec package is part of maplibre-gl-js feels weird to me...