Allow relative URL paths in styles

[xabbu42]

This uses the url of the style (or of the tilejson) as base url for relative urls. Before the referrer was used everywhere (that is the the url of the map application itself).

With this patches you can for example directly use https://basemaps.arcgis.com/arcgis/rest/services/World_Basemap_v2/VectorTileServer/resources/styles/ as style url and everything works.

This fixes #182.

Todo

• Adapt all other sources to use tileBaseUrl for tiles.
• How should the style baseUrl be passed around? Is expanding options correct?
• Should the baseUrl be included in serialize or not?
• Depending on the answers of the above questions, fix the two failing tests.
• Can wie use URL (https://developer.mozilla.org/en-US/docs/Web/API/URL/URL)?

Launch Checklist

• Confirm your changes do not include backports from Mapbox projects (unless with compliant license) - if you are not sure about this, please ask!
• Briefly describe the changes in this PR.
• Write tests for all new functionality.
• Document any changes to public APIs.
• Post benchmark scores.
• Manually test the debug page.
• Suggest a changelog category: "feature".
• Add an entry inside this element for inclusion in the maplibre-gl-js changelog: <changelog>Intepret urls in json files relative to the location of the json itself.</changelog>.

[xabbu42]

Any input to the approach I'm taking or to the open questions in the todos is very welcome.

[github-actions]

Bundle size report:

Size Change: +165 B
Total Size Before: 194 kB
Total Size After: 194 kB

Output file	Before	After	Change
maplibre-gl.js	185 kB	185 kB	+165 B
maplibre-gl.css	9.49 kB	9.49 kB	0 B

ℹ️ View Details

Source file	Before	After	Change
rollup/build/tsc/src/source/load_tilejson.js	280 B	339 B	+59 B
rollup/build/tsc/src/style/style.js	6.55 kB	6.59 kB	+47 B
rollup/build/tsc/src/util/request_manager.js	356 B	393 B	+37 B
rollup/build/tsc/src/source/vector_tile_source.js	1.11 kB	1.13 kB	+24 B
rollup/build/tsc/src/style/load_glyph_range.js	223 B	237 B	+14 B
rollup/build/tsc/src/render/glyph_manager.js	909 B	914 B	+5 B

[HarelM]

Thanks a lot for looking into this!!
It feels to me like the baseUrl is spread across the code a bit too much, I'm not sure if there's another solution to this problem, but adding a baseUrl in all kind of places and functions seems a bit too messy...
It would be great if we could simply add/define this in one place and use it only when needed instead of passing this as a parameter to some methods.
If this is the only solution then I guess we'll have to live with it.
The reason I'm saying this is that this is relatively, from my perspective, an edge case, and I would like to change the code as little as possible to support this.
Thanks for taking the time to try and solve this! Truly appreciated.

[xabbu42]

I agree about the spread out implementation. I was thinking about having some state in RequestManager instead, but this would make it necessary to have different RequestManagers in different places. The name and also the usage suggests to me at the moment that RequestManager should be a singleton. For instance a call to Map.setTransformRequest should affect all requests and therefore has to propagate everywhere.

Is there an accepted/non-hacky pattern so that I can have different RequestManager objects with different included baseUrls but which all refer to the same global _transformRequestFn? Would that be a better solution?

[HarelM]

Hmm... Is there a way to achieve this using transform request only without changing the code? It feels like this should be possible...

[xabbu42]

No, because the transform request no longer has the appropriate base url. it would need to guess based on the url itself and its ResourceType from where the url came which can never be completely correct and would be a hack.

Also I think this is the more useful behavior so it should imho be the default.

[HarelM]

I think it would be interesting to try and hack it - even if it's not 100% correct all the time it can provide a solution that doesn't require code changes for something that's not "mainstream".
Another approach from my point of view might be a plugin.
The more I look at this the less I feel comfortable to add this code.
But that might just be me... :-)

[xabbu42]

Just to be clear:

• the hack I had in mind was just maping the wrong urls to the correct ones with a hardcoded table. this only works for a specific style with a single source and I don't consider that a viable alternative. If you have any better solution in mind please provide some pointers on how to do that.
• I don't consider this pull request mergeable as is but a possible starting point for an implementation.
• Any implementation will be spread out a bit also as a consequence of the organization of the current code base (for example there is copy pasted code between different sources, and where code is reused its by functions which have to get all the necessary state as arguments).

what do you mean by plugin? I don't know of any plugin mechanism which could help here.

[HarelM]

maplibre plugin, for example the mapbox-draw plugin, mapbox-geocoder etc.
What I had in mind was a hack so that the request transform method will have a state so that if it gets called with a tilejson for example and this tilejson has some relative paths in it will replace them. It's more of an abstract concept at the moment but it might work, I don't know.
In any case, I think we both feel uncomfortable with the current changes. unfortunately, I don't have a better idea how to solve this...

[xabbu42]

• as far as I can see these plugins only use the public api of maplibre and do not change anything about the handling of styles or sources, I'd need something more concrete to understand how this could somehow be implemented as a plugin.
• transform request only gets an url and returns request parameters, it never sees the fetched tilejson
• I still want to get this pull request or something similar in a mergeable state. It will need some code changes throughout the code base to have the relevant state where necessary. If this is not acceptable I'd rather stop wasting my time now.

[xabbu42]

The cleanest way to implement this would imho be to replace RequestManager by a RequestContext which has no state of its own but pointers to the map,style and source in the context of which a request is made. The transform request function would be part of the map state and the different RequestContext objects could access it with their reference to the map object. But doing it this way will be a larger diff and touch more places as the minimalistic approach in this pull request.

[HarelM]

I think someone else besides me should review it and decide. I don't think this is a waste of time and I'm sorry you feel this way...

[xabbu42]

No hard feelings on my side, the investment so far wasn't that much. Its just that I have a hard time to understand if this approach would make it in if cleaned up a bit (like not abusing setUrl to also set an extra value) or if the changes are too invasive anyway and I should therefore not continue with it. So I'm waiting on more input on the best way forward to get something accepted.

[github-actions]

This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 30 days.

[xabbu42]

This should remain open.

[github-actions]

This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 30 days.

[wipfli]

Aha, a stale bot message! @xabbu42 any updates?

[xabbu42]

I'd need more input on the open question in the todos as well as on the best approach to pass baseUrl around to continue working on this. I'm still interested to get this in.

[github-actions]

This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 30 days.

[github-actions]

This PR was closed because it has been stalled for 30 days with no activity.

[KiwiKilian]

Really looking forward for this as ArcGIS Pro seems to output relative paths..

To the question auf URL usage, one possibility would be:

new URL(url, style).toString()

Where url is an absolute OR relative URL and style the URL of the used style. MDN states that an already absolute URL would not be tempered with:

If url is a relative URL, base is required, and will be used as the base URL. If url is an absolute URL, a given base will be ignored.