-
-
Notifications
You must be signed in to change notification settings - Fork 74
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
Adjust geometry-type
expression to return Point
, LineString
or Polygon
?
#965
Comments
In the TSC Meeting of February 2024 this change was discussed. We thought that most styles used In retrospect, with styles from all MapLibre sponsors I tried breaking in subtle (or not so subtle) ways, that does not seem to be the case. I would be in favor of reverting this change, cutting another release, coming up with another solution for 5.1.0. |
First of all, I'm sorry for entering the room just before celebration of the amazing release of 5.0.0. Adding Globe view is indeed a huge accomplishment, and I didn't want to ruin the mood! I'm just feeling that this voting back then probably didn't represent the belief of the wider MapLibre ecosystem and community. Actually, it probably didn't represent even the core team's view, had it been clear that AWS maps, Facebook maps, etc. would all will be broken with one option. I propose one solution:
I made a PR for the fix, by doing an interactive rebase: #966 |
About Version <=20 turns it into single types: 21, 22 turns it into double types. In conclusion, anyone who uses the linting tools is 100% sure not to have |
Versions 21 and 22 were released late October 2024 in order to support the geometry expression change of maplibre v5. |
This is what I wrote: "gl-style-migrate removes it" What is wrong about it? It removes the "$type" from the styles, both old and new versions. So no one can have "$type" in a stylesheet if they used gl-style-migrate. |
Hi MapLibre team, I am coming from the Amazon Location Service team. We have had multiple customers raising this rendering issue with AWS Maps using MapLibre v5.x. Is there a ETA on the fix? Could we rollback the backward incompatible change? |
@philipko100 Just to clarify, the breaking change was a conscious decision instead of a bug. So technically there is no "fix". From semantic versioning perspective, v5.x is a major version bump. Including incompatible API in a new major version is a common practice. On the other hand, I am sorry to hear that this change has impacted your customers. To help the community understand the impact and decide the next steps, could you please provide more information without sharing any sensitive or confidential data? It will be also great if you guys can make a suggestion/pull request from your perspective. |
Hi @hy9be, thanks for the context. I understand that a rollback is not the preferred way to proceed as this is a major version bump. The issue was that our style descriptors that customers are using no longer became compatible with MapLibre GL v5 due to the geometry-type polygon change. For now, we are exploring the option to update our style descriptors to become comptaible with gl v5. I assume that updating our style descriptors to be compatible with geometry-type polgon of gl v5 will still keep the compatibility of v4 and v3. Could you kindly confirm this is the case? |
Is the style specification covered by the semantic versioning guarantee of MapLibre GL JS or the maplibre-gl-style-spec package? Where does that leave MapLibre Native? Since there’s been some speculation: the original Mapbox GL team decided in 2018 to freeze the style specification at v8 and avoid any breaking changes to the specification for the foreseeable future. A lot of good ideas for breaking changes were collected in mapbox/mapbox-gl-js#6584 for a breaking v9 release someday in the distant, unimaginable future. In the meantime, new layer properties would be purely additive, and any obsolete properties or combinations would simply be ignored rather than explicitly deprecated or removed. The closest thing to a breaking change has been the removal of the undocumented Before the fork, Mapbox also noticed that There were many reasons for these decisions, but the one that’s probably most relevant to MapLibre is compatibility with older styles. By analogy, the major version of a Web browser or HTML rendering library has little bearing on whether the browser can still view HTML 3.2 authored many years ago in FrontPage. gl-style-migrate is just one way to upgrade a style from legacy functions and filters, and of course there’s no guarantee it has been upgraded anyways. |
I understand @hyperknot's predicament, even though I don't use the lint tools and pretty much relied on existing styles and only modified them as needed. Regardless of the outcome, we need some clarity on future usage of $type, e.g.
[Disclaimer: these points are what I understood from the slack comments and may not be entirely correct] |
To avoid further disruption, keep backward compatibility with styles that use this expression and give people a smoother transition path to MapLibre GL JS v5, we decided to reverted this change to match the old behavior. I hope that this is an acceptable outcome for everyone, especially to those that contributed to the past discussion and development of #519. Let's open a new issue so we can tackle the aims of that PR (which was reverted) in a backward-compatible way. I shared an idea to create end-to-end tests that use real styles in the MapLibre GL JS repo, which I think will be helpful to gauge the impact of changes on real styles in the future. |
Historically, in both MapLibre GL JS up to v4 and MapLibre Native, contrary to what the style specification specifies, the
geometry-type
expression returnedPoint
,LineString
orPolygon
. This was reported in maplibre/maplibre-gl-js#3516 as a bug, fixed in #519 and released as a breaking change in MapLibre GL JS 5.0.0.We had a discussion about this topic on Slack and during today's TSC Meeting. I am creating this issue, because I feel we haven't reached a consensus on this topic yet. To add some more context to the discussion I tried the styles of some MapLibre sponsors. Most have issues.
Jawg Streets
AWS Open
Radar Maps
Facebook Light
Obligatory XKCD
The text was updated successfully, but these errors were encountered: