Change style-spec version to v108, increment on future changes.

tmcw commented 1 year ago

Motivation

Right now, maplibre-gl-js contains incompatible additions to style spec v8, like the change to icon-padding. This is an issue for a few reasons:

It's not compatible with Mapbox GL JS, but also Mapbox GL JS (and the rest of that ecosystem) can't tell that before validation.
It's also not compatible with older versions of Maplibre.

The intent of version numbers is to indicate compatibility, and currently Maplibre isn't really using them for that purpose: styles with different levels of compatibility are all version: 8.

Right now the compatibility story is bad.

Maplibre is not compatible with Mapbox styles, because of the mapbox:// removal.
Mapbox is not compatible with Maplibre styles, because of the additions like icon-padding
New maplibre styles are not compatible with earlier versions of Maplibre GL JS because of the additions.

Design Alternatives

The idea of 108 is that we wouldn't want the version to be 9 because then it'll conflict with Mapbox's 9 version when they launch it eventually. So the "Maplibre flavor" versions and the "Mapbox flavor" versions need a distinguishing factor, which could be that Maplibre versions are >= 100. Or there could be an additional key in the style spec like engine: "maplibre".

Implementation

This will be a pretty invasive change, especially because versions of Maplibre that added style capabilities without incrementing the version number have already shipped. But I think it's pretty necessary to update the version key in the future, and the best time to start is now.

What is important about the version semantics is that, by indicating different versions and flavors of the style spec, then it would be possible to port "new" libre styles to old ones, and port "mapbox" flavor styles to "libre", and vice versa. Without an explicit indication of compatibility (a version number that identifies the compatibility requirements), none of this is cleanly possible.

HarelM commented 1 year ago

Style spec package was updated, maplibre,gl-js package was updated. There's no point in changing the version from 8 to a different numbet as latest code supports latest changes and there were no breaking changes, just additions. We are also not planning on introducing breaking changes in the future, hopefully.

tmcw commented 1 year ago

I'm not sure if our definitions of what is a breaking change might be different. I mean a breaking change to be an addition to the style spec that is not backwards-compatible: a style that says "version: 8" but does not work with all of the tools tested that already work with "version: 8".

The icon-padding change is not backward-compatible.

Here's an example of 1.52.0 failing to handle a style with icon-padding: https://codesandbox.io/s/maplibre-152-9xx0zs?file=/index.html
And here's an example of 2.4.0 handling that style: https://codesandbox.io/s/maplibre-240-4tvf4o

The issue here is that we end up with three different kinds of styles (mapbox, maplibre <=2.2.0, and maplibre >2.2.0) that all have the same information about what engines they're compatible with ("version: 8") but all have different actual ranges of compatibility. There are styles that only work with Maplibre 2.x, but no signal of that fact. I think this is a pretty difficult dynamic that is not good for the ecosystem because it means that, given "a GL style" there's no guarantees what it'll work with. The intent of the version system, and how Mapbox has been maintaining it afaict, is to help at least make it clear what is compatible with what.

HarelM commented 1 year ago

That's simply incorrect. Mapbox v2 and maplibre v2 intruduced terrain. In both cases the version of the style-spec was not changed (it's still 8) as the version rules did not change. Every property in the spec has the relevant version it supports (both native, and web). There's also no alignment in the specs, so you can't really change from mapbox to maplibre without fully checking that everything works. You also already started a discussion here: #1826 so I'm not sure what's the reason to split the conversation. Closing this issue.

tmcw commented 1 year ago

I was opening this issue because in the discussion you seemed pretty annoyed that I was complaining but not contributing. I'm happy to contribute and this was one of the solutions discussed in the other thread. My intent here is just to try and make sure that specs aren't fractured, and it'd be nice to have strong interoperability in maps. Anyway, I'm seeing that you're pretty annoyed, so sorry about that. I wanted to experiment with Maplibre in my product and these style changes have been problematic, hence the questions.

HarelM commented 1 year ago

I've outlined there what are the possible directions to continue. If you want to contribute I suggest opening a PR and not opening an issue... And I also suggest to better study the history and implantation in order to open meaningful PRs. What I'm missing here is a problem statement and not a solution. Changing a version is a solution not a problem statement. Saying you want to avoid fracturing the style spec is still not a problem definition, it's wishful thinking. Did you open the same issue to mapbox as well? Did you contact then in order to have their perspective on this?

Bottom line, if your product wants to use an outdated version of any tool mapbox or maplibre with a up-to-date spec then you'd need to downgrade the style based on the version you want to support, creating a product that is forward compatible is a waist of energy from my point of view. I would advise to simply upgrade to latest version. If you want to switch between products (mapbox/maplibre) I advise to create a plugin, much like what happened with mapbox scheme. It's not a part of this library, and as I said, I'll be surprised if the style-spec would not diverge. But good luck! 😀

wipfli commented 1 year ago

I think Mapbox chose to freeze the style spec version years ago and they decided to basically never introduce breaking changes to the style. I like this approach a lot and find that we should do the same, i.e., not break the style spec.

Additions like padding arrays, terrain, and viewport-glyph are not breaking changes. They just don't exist in older versions of MapLibre GL JS.

Note that compatibility with Mapbox GL JS v1 is the goal of MapLibre GL JS v1. Afterwards, I don't expect us to follow the new API of Mapbox.

tmcw commented 1 year ago

Okay, fair enough, it's your project. I was hoping that at least there'd be some indication of compatible engines in spec documents so that tooling and the ecosystem could distinguish between maplibre styles and mapbox styles. But I'm feeling some antagonism in this thread, so 🤷

wipfli commented 1 year ago

So yeah, it is a bit strange that the style has a version v8, @tmcw, since this number is basically useless. But I think we should continue like we did so far and use the version of the style spec package.json...

wipfli commented 1 year ago

The compatibilty information is probably is v8.json. It is safe to assume that everything marked with "js": "2.*.*" does not work in Mapbox and vice versa.

wipfli commented 1 year ago

Here is an example: https://github.com/maplibre/maplibre-gl-js/blob/1a4d81daea5e4f21e8c8069dabf04acd7c9ea9dc/src/style-spec/reference/v8.json#L1196

maplibre / maplibre-gl-js