Add support for MapTiler SDK #199
Conversation
|
@petr-hajek Thanks for this! We'll be taking a look at this soon. |
|
It's my first PR into this project and for full transparency, MapTiler is my client so I am open to any feedback. Thanks a lot. Anyway, this is a great tool! |
There was a problem hiding this comment.
Looking good here!
I left a long explainer for this, but basically I think we may need to switch the order of our || conditional here to be style || url as opposed to how it is now.
We'll also want to QA that change and make sure there's no weird side effects.
cc @ebrelsford
| nextUrl.host === 'api.maptiler.com' && | ||
| !nextUrl.searchParams.has('key') | ||
| ) { | ||
| nextUrl.searchParams.append('key', maptilerApiKey); |
There was a problem hiding this comment.
I'm seeing Maptiler URLs fail in other renderers (Mapbox and Maplibre).
I think it's because here you're following the same pattern we use for a mapbox URL and as I think on this, I wonder if we need a change in this PR for Maptiler and a followup for Mapbox URLs.
Basically we haven't run into this with Mapbox URLs (with the mapbox protocol mapbox://) because we've considered them only valid via the Mapbox renderer despite us normalizing the URL here. This is because they typically also reference additional Mapbox protocol URLs for sprites and glyphs so we didn't consider this a bug (You'll see Mapbox styles using the protocol fail in Maplibre for example).
Maptiler (correct me if I'm wrong) is different in that it does not use a custom protocol inside the style and instead creates a valid url with the key present when requesting the style url with that key:
"glyphs": "[https://api.maptiler.com/fonts/{fontstack}/{range}.pbf?key=fBwWvQI476rYQ6q9LrJf](https://api.maptiler.com/fonts/%7Bfontstack%7D/%7Brange%7D.pbf?key=fBwWvQI476rYQ6q9LrJf)"
So basically, we should be able to request the style with the new maptilerApiKey param while rendering with Maplibre, but we fail here because it looks at the URL (without accounting for the API key) before the style requested from the normalized the URL.
tl;dr
- I think this line should privilege
styleoverurl(I don't think there's a side effect of this?) like this:map.setStyle(style || url); - As a followup, we should consider normalizing the URLs using the Mapbox protocol within a stylesheet to allow it to be rendered outside of the Mapbox renderer.
Let me know if this does not sound accurate!
|
Sorry for not responding. I was loaded with other things, but I'll try get back to this soon, thanks for the feedback. |
Description
As a user of MapTiler SDK & MapTiler maps I'd like an easy way how to compare also MapTiler maps.
With this PR I can add MapTiler API key to the config (similar to MapBox). I can also use MapTiler SDK as a renderer (it's almost the same like MapLibre).
I removed the duplicate code in Map.svelte
QA steps
Probably some basic regression of MapLibre & MapBox but should not be affected
Author checklist
Create the PR
After approval
maininto your branch, resolve any conflictsmain