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 exception when calling LayerGroup/hasLayer() with wrong layerId #6998

Merged
merged 1 commit into from
Feb 18, 2020

Conversation

johnd0e
Copy link
Collaborator

@johnd0e johnd0e commented Feb 18, 2020

Now false returned, as per docs
fix #6472

@johnd0e
Copy link
Collaborator Author

johnd0e commented Feb 18, 2020

Note:
Currently undefined/false/null are tolerated as layer value.

There is even special code in tests to ensure that:

describe("#hasLayer", function () {
it("returns false when passed undefined, null, or false", function () {
var lg = L.layerGroup();
expect(lg.hasLayer(undefined)).to.equal(false);
expect(lg.hasLayer(null)).to.equal(false);
expect(lg.hasLayer(false)).to.equal(false);
});
});

In my opinion it has little sense but can lead to bugs.
Could anyone explain source reason?
Edit: #6999

Copy link
Member

@IvanSanchez IvanSanchez left a comment

Choose a reason for hiding this comment

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

LGTM

@IvanSanchez
Copy link
Member

Note:
Currently undefined/false/null are tolerated as layer value.
[...]
In my opinion it has little sense but can lead to bugs.
Could anyone explain source reason?

That dates back to... wow, #4. My guess is that at some point, the meaning (and documentation) of hasLayer() changed without anybody noticing.

Copy link
Member

@IvanSanchez IvanSanchez left a comment

Choose a reason for hiding this comment

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

The travis CI tests are failing with

  77:7   error  Identifier 'layer_id' is not in camel case  camelcase
  78:10  error  Identifier 'layer_id' is not in camel case  camelcase

@johnd0e johnd0e changed the title Fix exception when calling LayerGroup/hasLayer() with wrong layer_id Fix exception when calling LayerGroup/hasLayer() with wrong layerId Feb 18, 2020
@johnd0e
Copy link
Collaborator Author

johnd0e commented Feb 18, 2020

The travis CI tests are failing with

Fixed

@IvanSanchez IvanSanchez self-requested a review February 18, 2020 14:02
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.

Uncaught TypeError: Cannot create property '_leaflet_id' on number '1'
2 participants