Skip to content

Change undefined argument handling in Layer[Group]/hasLayer() - #6999

Merged
mourner merged 1 commit into
Leaflet:masterfrom
johnd0e:haslayer-notallow-undefined
Nov 5, 2021
Merged

mourner merged 1 commit into
Leaflet:masterfrom
johnd0e:haslayer-notallow-undefined

Conversation

@johnd0e

@johnd0e johnd0e commented Feb 18, 2020 •

Copy link
Copy Markdown
Collaborator

undefined/null/false are not considered as special case anymore,
layer argument is always required, as per docs.

Previous excessive tolerance may lead to real errors get masked.

I've tracked the issue down to it's origin: #1282, but found mentioned there reasons weak.

@johnd0e
johnd0e force-pushed the haslayer-notallow-undefined branch 2 times, most recently from b90cab6 to 576032c Compare February 18, 2020 14:57
@johnd0e
johnd0e marked this pull request as ready for review February 18, 2020 14:59
@johnd0e

johnd0e commented Feb 18, 2020

Copy link
Copy Markdown
Collaborator Author

May be better leave corresponding tests, if it's possible to express exception as expected result.

@IvanSanchez IvanSanchez left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. I'll leave this unmerged for some time, to see if any other maintainer has any objections to the change in behaviour.

@mourner

mourner commented Nov 5, 2021

Copy link
Copy Markdown
Member

Looks there are still some conflicts?

…/hasLayer()

undefined/null/false are not considered as special case anymore,
`layer` argument is always required, as per docs.

Previous excessive tolerance may lead to real errors get masked.
@johnd0e
johnd0e force-pushed the haslayer-notallow-undefined branch from c7e2bbb to 71e1438 Compare November 5, 2021 12:01
@johnd0e

johnd0e commented Nov 5, 2021

Copy link
Copy Markdown
Collaborator Author

Looks there are still some conflicts?

Fixed!

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.

3 participants