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

removeLayer method throws an error #49

Open
ttsvetko opened this issue Apr 26, 2017 · 6 comments
Open

removeLayer method throws an error #49

ttsvetko opened this issue Apr 26, 2017 · 6 comments
Labels

Comments

@ttsvetko
Copy link

Here it the code-fragment for 'removeLayer':

 removeLayer: function (layer) {
    var id = L.Util.stamp(layer);
    var _layer = this._getLayer(id);
    if (_layer) {
      delete this.layers[this.layers.indexOf(_layer)]; // `this.layers` should be replaced with `this._layers`
    }
    this._update();
    return this;
  }

Instead of this.layers should be this._layers i guess.

@ismyrnow
Copy link
Owner

ismyrnow commented May 2, 2017

You're totally right about that. Would someone mind opening a pull request with the fix?

@ismyrnow ismyrnow added the bug label May 2, 2017
@ttsvetko
Copy link
Author

ttsvetko commented May 2, 2017

I can do later

@ForgottenLords
Copy link

ForgottenLords commented May 13, 2017

Calling delete on the array index is not sufficient, as it is later iterated over; delete removes the index from the array, causing the deleted index's value to be 'undefined' when expecting an object. Instead, you must splice the item from the array to leave the indexes continuous.

The current pull request #50 is insufficient.
I had included a fix in my pull request #46 (an unrelated feature request).

ismyrnow added a commit that referenced this issue Oct 5, 2017
Fixes issue #49 - removeLayer method doesn't work
@karlitos
Copy link

Please, accept the PR to fix the issue caused by using delete on array.

@newmanw
Copy link

newmanw commented Feb 19, 2018

Possible to merge this pull request and release a new version to npm?

@ChrisSoper1
Copy link

It looks like the pull request has been accepted, but the NPM package has not been updated

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

No branches or pull requests

6 participants