Problem adding geoJson a second time

[RobbieTheWagner]

I am using PruneCluster with Leaflet and I have custom logic happening to attempt to add and remove polygons when clusters are created. It mostly works, but I have an issue when I leave the page, and come back. I need all the markers that were put on the map previously, to be added again. This seems to work fine for markers without a geoJson layer, but when I try to add back a geoJson layer I get Uncaught TypeError: Cannot read property 'min' of undefined which is being triggered by this in Leaflet:

    _getBitCode: function (/*Point*/ p, bounds) {
        var code = 0;

        if (p.x < bounds.min.x) { // left
            code |= 1;
        } else if (p.x > bounds.max.x) { // right
            code |= 2;
        }
        if (p.y < bounds.min.y) { // bottom
            code |= 4;
        } else if (p.y > bounds.max.y) { // top
            code |= 8;
        }

        return code;
    },

My custom code to add shapes, when the marker is not clustered looks like this:

    pruneCluster.PrepareLeafletMarker = function (leafletMarker, data) {
      if (data.isShape) {
        map.addLayer(data.geoJsonLayer);
      }
      leafletMarker.on('click', function (e) {
        GenerateRrosePopup(e, data.popupContent);
        if(data.isShape) {
          map.fitBounds(data.boundsForFit);
        }
      });
      leafletMarker.setIcon(data.icon);
    };

map.addLayer(data.geoJsonLayer) is what causes the issue. I'm not sure why though because my geoJson object looks as I expect it to.

Why would this same code work for initially adding the geoJson layer, but then fail when I come back to the page and try to add the points and shapes I previously added?

[mourner]

Could you replicate the same issue in a minimal pure-Leaflet JSFiddle test case (without plugins)? It's pretty hard to figure the issue out without it.

[RobbieTheWagner]

I can try. I will work on it now. It may be difficult to reproduce though.

[RobbieTheWagner]

I was able to reproduce it. The geoJson is drawn when the map loads, then there is a button to click which creates a new map and adds the geoJson again. It yields the same Uncaught TypeError: Cannot read property 'min' of undefined as I was getting in my app.

Here is the link: http://jsfiddle.net/F9kqq/30/

[RobbieTheWagner]

Please confirm when you have checked out the fiddle and that you see the same error being logged.

[mourner]

Yes, I'm getting the error. Is it the same if you use the latest stable, 0.7.3?

[RobbieTheWagner]

Yes, it's still the same. I updated the fiddle to use 0.7.3 http://jsfiddle.net/F9kqq/31/ . There was no change though

[mourner]

Could you update to the uncompressed Leaflet, leaflet-src.js?

[RobbieTheWagner]

Sure thing, here it is updated with leaflet-src http://jsfiddle.net/F9kqq/32/

[mourner]

Is it reproducible in master version?

[RobbieTheWagner]

I updated it to the version you requested. Isn't it unlikely master would fix it? I can update it to master if I need to.

[RobbieTheWagner]

Do I need to update it to master? I can if you would like, but I don't think it will change. Are the examples I provided not sufficient to track down the error?

[RobbieTheWagner]

Please let me know what else I need to provide to help debug this problem. I cannot ship my map update until this functionality works.

[mourner]

@rwwagner90 as a temporary workaround, could you try removing the map HTML node, create a new one dynamically and readd it to the DOM after doing map.remove()?

[RobbieTheWagner]

@mourner I'm not quite following what you're saying. Could you elaborate? Is the issue that the map is missing bounds when it is recreated for some reason? I'm not even really sure what the underlying issue is.

[mourner]

@rwwagner90 I'm still not sure what the problem is, but if it's triggered by doing map.remove and adding a layer on a re-initialized map, that initializing a map on a new DOM element would probably fix the issue since there's no way the first map creation would affect the other.

[RobbieTheWagner]

@mourner we are using Ember and the map is in a component. The component and all views and everything are destroyed, then we come back to the page, create a brand new map component, and the associated DOM elements, so that would not help. It seems to me that adding geoJson to the map gives it bounds, then when I come back and try to add the same geoJson to a new map, without any geoJson, it's looking for the bounds, and there are none.

[mourner]

Could you recreate the geoJson object than?

[RobbieTheWagner]

I could, it's just going to add more complexity. I will have to loop through all of my primitives and create new geoJson every time. I should be able to add the object to another map. It works with markers.

[RobbieTheWagner]

If this is an issue you do not want to look into fixing, then I will find a workaround, but markers, polygons, geoJson, whatever should all be able to be created and added to multiple different maps, in my opinion.

[RobbieTheWagner]

In my opinion, this is something that should be fixed in Leaflet, rather than applying a workaround. Do you disagree?

[mourner]

Probably, but it's not an immediate priority. Pull requests welcome.

[RobbieTheWagner]

Okay. I will take a look at the Leaflet source, but I don't have any knowledge of it. Could you point me to sections of the code relevant to my problem?

[RobbieTheWagner]

So I just built leaflet from source on my machine, and used that built code in the same JSFiddle example, and the problem appears to be gone now. Not sure what changed in the master version, but I'll try updating my leaflet version to master in my codebase, and hopefully it will fix the issue.

[RobbieTheWagner]

Unfortunately, using the master version I built, completely breaks my actual app now.

[mourner]

What code version did you use to make a custom build?

[RobbieTheWagner]

I just forked master and ran jake. The leaflet-src output says 0.8-dev. It appears to be handling L differently, which may be causing my issues.

[RobbieTheWagner]

I'm getting this error: Uncaught TypeError: Cannot read property 'call' of undefined In this code:

whenReady: function (callback, context) {
        if (this._loaded) {
            callback.call(context || this, {target: this});
        } else {
            this.on('load', callback, context);
        }
        return this;
    },

[RobbieTheWagner]

After investigating further, it appears the addLayer refactor may be the culprit.

[RobbieTheWagner]

The issue stems from:

this.whenReady(layer._layerAdd, layer);

layer._layerAdd is undefined. Why are we relying on the layer to have its own _layerAdd method? Shouldn't adding layers be a map thing? It was before, I believe.

We are using the heatmap plugin for leaflet, and the layer created by that plugin, and any other non-updated plugins, will not have the _layerAdd method, correct?

[mourner]

The master version has a lot of breaking changes (see the changelog). Now all custom layers are required to inherit from L.Layer class.

[RobbieTheWagner]

Why was that decided upon? Was there something wrong with the old way? I guess we'll have to wait for all of the plugins we use to make new versions, seems inefficient.

[RobbieTheWagner]

Actually, the first plugin we are having issues with is your Leaflet.heat plugin. There is already an issue open for support: https://github.com/Leaflet/Leaflet.heat/issues/17. Would this take long to switch to support 0.8? It would be very helpful to me and at least a few other people :+1:

[mourner]

Yes, there was a lot wrong with the old way. Yes, plugins will need to catch up, but it's a very easy process.

[RobbieTheWagner]

Alright, when is 0.8 expected to be released?

[mourner]

Planning to make a beta release during the next few weeks. It will be actually 1.0.

[RobbieTheWagner]

Okay, awesome. Once it's released, and all plugins we use are updated, my problem may go away. If you can fix the Leaflet.heat issue, I can see what else is broken still.

[codeofsumit]

I have the same issue.

After re-adding geoJSON layer to a map and calling fitBounds, i get: Cannot read property 'min' of undefined also triggered by _getBitCode.

Might this be related?

[codeofsumit]

Alright we're using the beta version now to have this resolved. Kinda scary to use it in a huge production app.

[a1an77]

I just hit a similar problem when updating to 1.0beta2 and it took quite a bit to figure out plugin compatibility as the root source. It would be very appreciated to catch the error and give advice on the possible cause when introducing backward incompatibility points. It will save developers quite a bit of debugging time.

[pedrovsra]

I'm getting this problem while using the version 1.03. I'm trying to print a map using the browser print, working with an empty map. When I add my layers, it doesn't work anymore and I get the "Uncaught TypeError: Cannot read property 'call' of undefined" error. I tried to disable canvas and messagebox but no success

[perliedman]

@pedrovsra if you believe this is a bug, I suggest you open a new issue. Also, please first make sure the plugins you use are compatible with 1.0, as you can see from comments above, old plugins could be the source of this issue.

It would be great if you could provide a running example of the problem, for example using http://playground-leaflet.rhcloud.com/?html,output - this makes it a lot easier to find, understand and debug the problem.

With the information we have from your report now, it's more or less impossible for us to tell if this is a bug or not.