feat: expose original style URL via map.getStyleUrl()#7110
feat: expose original style URL via map.getStyleUrl()#7110giswqs wants to merge 6 commits intomaplibre:mainfrom
Conversation
Add a new `getStyleUrl()` method to the Map class that returns the URL used to load the current style, or null if the style was loaded from a JSON object. This enables plugin developers to: - Distinguish basemap layers from dynamically added layers - Build layer control plugins that group basemap layers - Implement style management tools - Serialize map state with the original style URL Closes maplibre#7109
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7110 +/- ##
==========================================
+ Coverage 92.66% 92.68% +0.02%
==========================================
Files 289 289
Lines 24061 24073 +12
Branches 5094 5098 +4
==========================================
+ Hits 22297 22313 +16
+ Misses 1764 1760 -4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Thanks for taking the time to open this PR. |
- Add setStyleUrl() public method to Style class for URL management - Add optional sourceUrl parameter to loadJSON() for diff fallback - Update Map class to use setStyleUrl() instead of direct property access - Add _updateStyleWithSourceUrl() helper to pass URL through loadJSON This ensures all _styleUrl property updates happen within the Style class, addressing code review feedback about encapsulation.
| } | ||
| } | ||
|
|
||
| private _updateStyleWithSourceUrl(style: StyleSpecification, options?: StyleSwapOptions & StyleOptions, sourceUrl?: string) { |
There was a problem hiding this comment.
This looks like a bug fix...? Is it related to this PR? I'm not sure I like this waiting approach... Is there an alternative?
| * | ||
| * @param url - The style URL, or null if the style is not loaded from a URL. | ||
| */ | ||
| setStyleUrl(url: string | null) { |
There was a problem hiding this comment.
Is it possible not to expose this method and handle updating the style in the relevant place without exposing this?
This API is misleading as it looks like you can set the style URL and have the style updated using this method, but that's not the case...
| }); | ||
|
|
||
| describe('getStyleUrl', () => { | ||
| test('returns null when style is loaded from JSON object', async () => { |
There was a problem hiding this comment.
These looks like duplicated tests, I would only keep tests here that are relevant to the changes of the map.ts class as part of this PR.
Seems like setting a styleURL and getting it back is enough as other scenarios are covered by the style.test.ts file (except the added _updateStyleWithSourceUrl logic, which I already reviewed and left my comments on).
- Add sourceUrl parameter to setState() so _styleUrl is updated internally - Remove public setStyleUrl() method from Style class (was misleading API) - Remove _updateStyleWithSourceUrl() helper, use _updateStyle with sourceUrl param - Simplify _diffStyle() by letting setState handle _styleUrl updates - Reduce duplicate tests in map_style.test.ts as suggested by maintainer - Add tests for setState with sourceUrl parameter
|
Thanks for the detailed feedback! I've addressed all the concerns in the latest commit:
All tests pass locally. Let me know if there's anything else that needs adjustment! |
| } | ||
|
|
||
| _updateDiff(style: StyleSpecification, options?: StyleSwapOptions & StyleOptions) { | ||
| _updateDiff(style: StyleSpecification, options?: StyleSwapOptions & StyleOptions, sourceUrl?: string) { |
There was a problem hiding this comment.
Would it make sense to add this to the options object?
| * ``` | ||
| */ | ||
| getStyleUrl(): string | null { | ||
| if (this.style) { |
|
It is better to answer in the relevant thread and link to the relevant commit, this way I can track which review comment we are talking about... |
Summary
map.getStyleUrl()method that returns the URL used to load the current stylenullif the style was loaded from a JSON objectMotivation
Closes #7109
When a map is initialized with a style URL, that URL is not accessible later. The
map.getStyle()method returns the current style object which includes all layers - both from the original basemap and any layers added dynamically. There's no way to distinguish between them.This feature enables plugin developers to:
API
Test plan
Style.getStyleUrl()instyle.test.tsMap.getStyleUrl()inmap_style.test.ts