Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Fix typing of Map.getLayer() and Style.getLayer() #2969

Merged
merged 2 commits into from
Aug 10, 2023

Conversation

RoystonS
Copy link
Contributor

@RoystonS RoystonS commented Aug 10, 2023

Launch Checklist

Fixes #2968

  • 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. Define return type of getSource as possibly undefined #724 made the equivalent change to getSource in December 2021.
  • Include before/after visuals or gifs if this PR includes visual changes. N/A
  • Write tests for all new functionality. This includes typing changes only.
  • Document any changes to public APIs. This change actually makes the advertised API match the documented APIs.
  • Post benchmark scores. This does not affect generated code at all, so can't affect performance.
  • Add an entry to CHANGELOG.md under the ## main section.

Changes

This PR changes the return types of:

  • Map.getLayer() from returning StyleLayer to StyleLayer | undefined
  • Style.getLayer() from returning StyleLayer to StyleLayer | undefined

Those declared return types are changing to match the existing documented behaviour, and brings them in line with the existing similar Map.getSource() and Style.getSource() methods.

Notes

  • Does this constitute a (breaking) public API change?

Yes and no. It's making the TypeScript-advertised type match the already-documented and implemented behaviour. It is possible that this could cause existing code to fail to compile if it's calling getLayer() and is compiled with a strict type-checking mode that will flag up that the return type is not being checked for undefined.

See PR #724 for earlier work which made the same change to the getSource() methods.

@codecov-commenter
Copy link

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (deb1276) 75.01% compared to head (427fbb6) 75.01%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2969   +/-   ##
=======================================
  Coverage   75.01%   75.01%           
=======================================
  Files         240      240           
  Lines       19110    19110           
  Branches     4311     4311           
=======================================
  Hits        14336    14336           
  Misses       4774     4774           
Files Changed Coverage Δ
src/style/style.ts 83.78% <100.00%> (ø)
src/ui/map.ts 82.92% <100.00%> (ø)

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

@HarelM HarelM enabled auto-merge (squash) August 10, 2023 12:58
@HarelM HarelM disabled auto-merge August 10, 2023 12:58
@HarelM HarelM enabled auto-merge (squash) August 10, 2023 12:59
@HarelM HarelM merged commit e8bf0bd into maplibre:main Aug 10, 2023
@RoystonS RoystonS deleted the pr/fix-getlayer-return-types branch August 10, 2023 18:45
@RoystonS
Copy link
Contributor Author

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The declared type of Map.getLayer() doesn't match the documentation or behaviour
3 participants