maplibre-gl-js
maplibre-gl-js copied to clipboard
Remove support for old filter expression syntax.
There is quite some code supporting the old filter expressions (https://maplibre.org/maplibre-gl-js-docs/style-spec/other/#other-filter), which have been deprecated for years. Support for them should be dropped in 2.x
I would recommend posing this in the slack channel to get this more visible and get more input. Removing support for these kind of filters might prove problematic, much like my attempt to remove the css mapbox prefix...
I'm afraid I don't see the value in removing the support for the deprecated expressions.
IMO, it is inflicting pain on customers/users without providing any added value to them. I also do not see how removing the code will have a significant impact on Maplibre, such that could justify this pain.
Moreover, https://maplibre.org/maplibre-gl-js-docs/style-spec/other/#other-filter, mentioned above, clearly says:
filters defined with this syntax will continue to work
without any mention of an End-of-Life process.
If these are indeed deprecated, then it should log a warning. A formal deprecation policy (which doesn't exist yet?) could say, "deprecated items will produce a warning, and indicate that they may be removed in a major version".
The value in removing this, it's easy to accidentally use the old syntax, and then attempting to mix the two produces a confusing error.
map.setFilter(
'countries-fill',
['any', ['==','CONTINENT','Europe'], ['==',['get','CONTINENT'],'Africa']],
)
Error: layers.countries-fill.filter[2][1]: string expected, array found
All the Maplibre styles I'm aware of are using the old style, including:
- Maplibre demo map (example) @petr-pokorny-1
- All 7 Maptiler styles @klokan
- The OpenStreetMap Americana Style (example) @ZeLonewolf
- Swisstopo style and swiss-map (example) @wipfli
- Both Israel Hiking Map styles (example) @zstadler
Implementing this PR will introduce a backward incompatibility that will require them and many other Maplibre users to either avoid updating their Maplibre version or receive unpleasant error messages, learn the new syntax, and convert the styles to the new syntax.
On the other hand, the above mentioned drawback is an unclear error message for code that never worked that will be seen by developers that didn't read the deprecation note:
Expression syntax and the deprecated syntax below cannot be mixed in a single filter definition.
If the wording of the error message is a significant problem, a separate issue can be opened for improving it. Unlike the change suggester here, fixing the error message will have no negative impact on existing loyal users of Maplibre.
I had no idea we were using an "old" syntax, and it would be helpful to show a few examples of old and new.
Ad mixing old and new filters: this could be improved by checking not only the first condition, but rather, whether everything could be a old filter.
In that case the above example would be a (valid) new filter, although a rather useless one: ['==','CONTINENT','Europe'] is always false.
I've labeled it as need more info as I think there's more input needed here. I'll take it to the steering committee meeting to see what they say about it.
I would like to see a strategy where deprecated features first throw a warning (in the console) indicating that the functionality will be removed in a future version.
I don't think it is a good idea to deprecate these. Hundreds of map styles are using it...
Are there tools automatically converting this syntax to the new expressions?
The code itself does the conversion/migration, I'm sure it can be easily extracted to a cli tool if needed.
Migration can probably be done automatically with https://github.com/maplibre/maplibre-gl-js/blob/main/src/style-spec/README.md#gl-style-migrate
Edit: https://github.com/maplibre/maplibre-style-spec/blob/main/README.md#gl-style-migrate
Americana's gl-js code is generated programmatically, so I'm not sure that can be converted automatically?
@ZeLonewolf Right, you would have to look at the generated style and then update the source by hand.
I started in https://github.com/ZeLonewolf/openstreetmap-americana/pull/527, need to handle the exponentials next.
I don't think removing the support for the original filter syntax is a good idea
This "deprecation" message is only a preference statement from Mapbox, as it clearly says
filters defined with this syntax will continue to work
Let's not look at the term "deprecation" out of this specific context.
This thread still has no reference to potential benefits of removing the support for the original filter syntax.
The old expressions really seem to be used a lot, also by public services like maptiler.com. So we probably shouldn't remove the old filter expressions, but rather make sure that the check for old/new is improved, as mentioned above (https://github.com/maplibre/maplibre-gl-js/issues/1385#issuecomment-1195337056)
Technical steering committee that was held today decided not to remove these filters. So I'm closing this issue as the discussion is mainly around removing the "old" filters. Feel free to open a new issues with the exact scenario that is problematic so we can track that differently. I would say that checking the style can be done with other tools maybe? even the style-spec cli (although I think it uses the same code), but again, this can be tracked elsewhere...