-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Improve some nullable map types #13328
Conversation
📦 Preview the examples and docs from this branch here: https://deploy-preview-13328--ol-site.netlify.app/. |
43e3a5b
to
be6dac8
Compare
@@ -121,7 +121,7 @@ class Control extends BaseObject { | |||
} | |||
this.listenerKeys.length = 0; | |||
this.map_ = map; | |||
if (this.map_) { | |||
if (map) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better TS type inference inside the if
be6dac8
to
15de4bb
Compare
7f3dc0b
to
7b6afcd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @EvertEt
return /** @type {import("./PluggableMap.js").default|undefined} */ ( | ||
this.get(Property.MAP) | ||
return /** @type {import("./PluggableMap.js").default|null} */ ( | ||
this.get(Property.MAP) || null |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although this looks like a breaking change, it actually only fixes the inconsistency that when previouslyMap#removeOverlay()
was called, a subsequent call of Overlay#getMap()
returned null
, but when called after Overlay
creation, it returned undefined
.
Closes #13326
I left out the interactions for now, they could probably also be improved.