Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

map.getStyle() modification issue #4488

Merged
merged 8 commits into from
Aug 14, 2024

Conversation

gavinr-maps
Copy link
Contributor

@gavinr-maps gavinr-maps commented Aug 1, 2024

This change returns a clone of the object so that when calling .getStyle() the user cannot inadvertently modify the internally stored style.

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.
  • Link to related issues.
  • Include before/after visuals or gifs if this PR includes visual changes.
  • Write tests for all new functionality.
  • Document any changes to public APIs.
  • Post benchmark scores.
  • Add an entry to CHANGELOG.md under the ## main section.

@codecov-commenter
Copy link

codecov-commenter commented Aug 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 65.36%. Comparing base (26d61c1) to head (4bfae9f).
Report is 6 commits behind head on main.

❗ There is a different number of reports uploaded between BASE (26d61c1) and HEAD (4bfae9f). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (26d61c1) HEAD (4bfae9f)
9 8
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #4488       +/-   ##
===========================================
- Coverage   87.82%   65.36%   -22.46%     
===========================================
  Files         247      247               
  Lines       33492    33493        +1     
  Branches     2234     1780      -454     
===========================================
- Hits        29413    21892     -7521     
- Misses       3070    10758     +7688     
+ Partials     1009      843      -166     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@gavinr-maps gavinr-maps force-pushed the 4481-getstyle-modify-style-issue branch from bc98595 to db7bb1c Compare August 1, 2024 17:54
@gavinr-maps gavinr-maps force-pushed the 4481-getstyle-modify-style-issue branch from db7bb1c to fe7957b Compare August 1, 2024 18:01
@gavinr-maps gavinr-maps changed the title [in progress] map.getStyle() modification issue map.getStyle() modification issue Aug 1, 2024
@gavinr-maps gavinr-maps marked this pull request as ready for review August 1, 2024 18:39
src/style/style.ts Outdated Show resolved Hide resolved
@HarelM
Copy link
Collaborator

HarelM commented Aug 1, 2024

Thanks for taking the time to open this PR!
I've added a few minor comments.

@gavinr-maps
Copy link
Contributor Author

@HarelM thank you for the feedback and your time maintaining this project. I have addressed your feedback and is ready for your re-review when you have some time. Thanks.

Copy link
Collaborator

@HarelM HarelM left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good THANKS!

@HarelM HarelM enabled auto-merge (squash) August 14, 2024 19:44
@HarelM HarelM merged commit 7324d9a into maplibre:main Aug 14, 2024
15 checks passed
@gavinr-maps gavinr-maps deleted the 4481-getstyle-modify-style-issue branch August 14, 2024 20:27
alexandresoro pushed a commit to alexandresoro/ouca that referenced this pull request Sep 22, 2024
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [maplibre-gl](https://maplibre.org/) ([source](https://github.com/maplibre/maplibre-gl-js)) | dependencies | minor | [`4.5.2` -> `4.7.0`](https://renovatebot.com/diffs/npm/maplibre-gl/4.5.2/4.7.0) |

---

### Release Notes

<details>
<summary>maplibre/maplibre-gl-js (maplibre-gl)</summary>

### [`v4.7.0`](https://github.com/maplibre/maplibre-gl-js/blob/HEAD/CHANGELOG.md#470)

[Compare Source](maplibre/maplibre-gl-js@v4.6.0...v4.7.0)

##### ✨ Features and improvements

-   Support multiple layers in `map.on`, `map.once` and `map.off` methods ([#&#8203;4570](maplibre/maplibre-gl-js#4570))
-   Ensure GeoJSON cluster sources emit a console warning if `maxzoom` is less than or equal to `clusterMaxZoom` since in this case you may see unexpected results. ([#&#8203;4604](maplibre/maplibre-gl-js#4604))

##### 🐞 Bug fixes

-   Heatmap Fix for 3D terrain ([#&#8203;4571](maplibre/maplibre-gl-js#4571))
-   Fix Map#off to not remove listener with layer(s) registered with Map#once ([#&#8203;4592](maplibre/maplibre-gl-js#4592))
-   Improve types a bit for `addSource` and `getSource` ([#&#8203;4616](maplibre/maplibre-gl-js#4616))
-   Fix the color near the horizon when terrain is enabled without any sky ([#&#8203;4607](maplibre/maplibre-gl-js#4607))
-   Fix bug where `fitBounds` and `cameraForBounds` would not display across the 180th meridian (antimeridian)
-   Fix white flickering on map resize ([#&#8203;4158](maplibre/maplibre-gl-js#4158))
-   Fixed a performance regression related to symbol placement ([#&#8203;4599](maplibre/maplibre-gl-js#4599))
-   Fix a bug where cloning a Transform instance didn't include the `lngRange`. This caused a bug where
    using `transformCameraUpdate` caused the `maxBounds` to stop working just for east/west bounds. ([#&#8203;4625](maplibre/maplibre-gl-js#4625))

### [`v4.6.0`](https://github.com/maplibre/maplibre-gl-js/blob/HEAD/CHANGELOG.md#460)

[Compare Source](maplibre/maplibre-gl-js@v4.5.2...v4.6.0)

##### ✨ Features and improvements

-   Prefer local glyph rendering for all CJKV characters, not just those in the CJK Unified Ideographs, Hiragana, Katakana, and Hangul Syllables blocks. ([#&#8203;4560](maplibre/maplibre-gl-js#4560)))

##### 🐞 Bug fixes

-   Fix right-to-left layout of labels that contain characters in the Arabic Extended-B code block. ([#&#8203;4536](maplibre/maplibre-gl-js#4536))
-   Fix 3D map freezing when camera is adjusted against map bounds. ([#&#8203;4537](maplibre/maplibre-gl-js#4537))
-   Fix `getStyle()` to return a clone so the object cannot be internally changed ([#&#8203;4488](maplibre/maplibre-gl-js#4488))
-   Fix issues with setting sky to `undefined` ([#&#8203;4587](maplibre/maplibre-gl-js#4587)))

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOC40Ny4wIiwidXBkYXRlZEluVmVyIjoiMzguODYuMCIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiZGVwZW5kZW5jaWVzIl19-->

Reviewed-on: https://git.tristess.app/alexandresoro/ouca/pulls/52
Reviewed-by: Alexandre Soro <code@soro.dev>
Co-authored-by: renovate <renovate@git.tristess.app>
Co-committed-by: renovate <renovate@git.tristess.app>
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Modifying the results of map.getStyle() modifies the style
4 participants