this._chart is undefined when calling clear() and redraw() before loading data

[velocat]

Subject of the issue

Dont work function clear()

Your environment

• leaflet-elevation: 1.7.5 & latest release

Steps to reproduce

Previously, I used this feature to switch between charts for tracks containing multiple segments (otherwise they are displayed all in one chart).

var map = new L.Map('map');
var controlElevation = L.control.elevation(options).addTo(map);
function toggleElevation(set){
  if(controlElevation){
    controlElevation.clear();
    controlElevation.redraw();
    controlElevation.addData(track.toGeoJSON());
  } 
}

All this worked with version 1.5.3

Now I tried to upgrade to the latest version and get this error:

https://github.com/Raruto/leaflet-elevation/blob/ac803fd023718e097a71f5121501a6a5043946d2/src/distance.js#L43

[Raruto]

Hi velocat,

Previously, I used this feature to switch between charts for tracks containing multiple segments (otherwise they are displayed all in one chart).

in the meantime can you make a simple pull request that adds here a new example with your old working code? (version 1.5.3)

Have a nice day, Raruto

[velocat]

The error can be seen in your own examples: https://raruto.github.io/leaflet-elevation/examples/leaflet-elevation_toggable-tracks.html (Google chrome, Edge and Opera show this, Firefox failed to catch this error.)

Even if the error does not appear on the first load, reloading the page results in this. It seems to me that the reason lies in the fact that the data has not yet been uploaded, for example, like mine, where they are uploaded later, or as in your example, if the data has not been calculated yet, then the chart has not yet been built and it is impossible to access it.

[mainzelM]

Same here: I tried to call clear() before load() to ensure the map is empty and got this error.

[Raruto]

Same here: I tried to call clear() before load() to ensure the map is empty and got this error.

Hi mainzeIM, first make sure that the chart is initialized:

https://github.com/Raruto/leaflet-elevation/blob/ac803fd023718e097a71f5121501a6a5043946d2/examples/leaflet-elevation_toggable-tracks.html#L84

and binded to the map (this just an alias for addTo):

https://github.com/Raruto/leaflet-elevation/blob/ac803fd023718e097a71f5121501a6a5043946d2/examples/leaflet-elevation_toggable-tracks.html#L90

after that it might work as follows:

https://github.com/Raruto/leaflet-elevation/blob/ac803fd023718e097a71f5121501a6a5043946d2/examples/leaflet-elevation_toggable-tracks.html#L154-L157

---

in the meantime can you make a simple pull request that adds here a new example with your old working code? (version 1.5.3)

The error can be seen in your own examples: https://raruto.github.io/leaflet-elevation/examples/leaflet-elevation_toggable-tracks.html Even if the error does not appear on the first load, reloading the page results in this.

I tried as you say but it doesn't happen in a predictable and testable way (when I refresh the page it happens much more rarely than you do). When you have some time can you send a complete example? (so at least we partially address this issue https://github.com/Raruto/leaflet-elevation/issues/56 too ...)

It seems to me that the reason lies in the fact that the data has not yet been uploaded, for example, like mine, where they are uploaded later, or as in your example, if the data has not been calculated yet, then the chart has not yet been built and it is impossible to access it.

Certainly before this trigger there is no check on the existence of the this._chart variable:

https://github.com/Raruto/leaflet-elevation/blob/ac803fd023718e097a71f5121501a6a5043946d2/src/distance.js#L38-L46

which theoretically should be initialized here:

https://github.com/Raruto/leaflet-elevation/blob/669251774062c25b1d148426237177bd4cf74fbe/src/control.js#L558-L602

but of course it's possible that that trigger is invoked somewhere else first:

https://github.com/Raruto/leaflet-elevation/blob/669251774062c25b1d148426237177bd4cf74fbe/src/control.js#L925

---

Anyway, i think the problem you report in here is due to the d3js library being loaded dynamically (as it is not included manually):

https://github.com/Raruto/leaflet-elevation/blob/ac803fd023718e097a71f5121501a6a5043946d2/examples/leaflet-elevation_toggable-tracks.html#L19-L28

since the execution of this piece of code (which essentially includes that library) is asynchronous, to solve we might need to add some sort of promise or a callback in client code before calling any other function (e.g. load() or clear()):

https://github.com/Raruto/leaflet-elevation/blob/669251774062c25b1d148426237177bd4cf74fbe/src/control.js#L330-L357

---

Brainings aside, Raruto

[velocat]

I found such a solution for myself.: change this: https://github.com/Raruto/leaflet-elevation/blob/ac803fd023718e097a71f5121501a6a5043946d2/src/control.js#L922-L923 into:

 _updateChart: function() { 
    if (/*!this._data.length ||*/ !this._chart || !this._container) return; 

This allows to clear the data and not continue if the chart does not exist yet :) Called from clear function: https://github.com/Raruto/leaflet-elevation/blob/ac803fd023718e097a71f5121501a6a5043946d2/src/control.js#L57-L69

[Raruto]

Hi velocat,

I found such a solution for myself: change this: https://github.com/Raruto/leaflet-elevation/blob/ac803fd023718e097a71f5121501a6a5043946d2/src/control.js#L922-L923 into:

_updateChart: function() { 
 if (/*!this._data.length ||*/ !this._chart || !this._container) return; 

It seems reasonable to me, if you think it is ok feel free to send a pull request.

Raruto

[mainzelM]

Same here: I tried to call clear() before load() to ensure the map is empty and got this error.

Hi mainzeIM, first make sure that the chart is initialized:

Hi Raruto

Thanks for your extensive answer. I think I followed the examples but my problem is, that I call clear() before load() (because I do clear(); load() upon an event and at that time, I don't know, whether load() has been called at least once). I can also reproduce this by adding controlElevation.clear() right before https://github.com/Raruto/leaflet-elevation/blob/ac803fd023718e097a71f5121501a6a5043946d2/examples/leaflet-elevation_geojson-data.html#L99

I'm now working around that by maintaining a flag that indicates whether I've called load() at least once and call clear() only if the flag is false:

if (this.isLoaded) {
  this.controlElevation.clear();
}
this.controlElevation.load(url);
this.isLoaded = true;

This seems to work in a sense that the error does no longer appear and the track itself is removed from the map. However, the name of the track remains in the layers list (top-right control on the map), and if I deselect and then again select a track there, it re-appears. So maybe clear() is not what I'm actually looking for, but I haven't found a proper way to remove a track added via controlElevation.load(url) (where url points to a GeoJson file). Is there a way to do so?

[velocat]

Well, my other solution, which seems more logical to me, is simply to delay the execution of the function while the D3 script is waiting to load. Although it would be more correct to make a Promise, but I have no thoughts how to implement it.

      /*
       * Reset data and display
       */
      clear: function clear() {
        if (this._marker) this._marker.remove();
        if (this._chart) this._clearChart();
        if (this._layers) this._clearLayers();
        if (this._markers) this._clearMarkers();
        this._data = [];
        this.track_info = {};

        this._fireEvt("eledata_clear");
    setTimeout(this._updateChart, 1000);
        //this._updateChart();
      },

The problem here is that we don't know exactly the response time from unpkg where the script is located. And if the time exceeds a second, as I set, the error will repeat

I will most likely refuse lazy loading of scripts altogether, firstly because with such loading scripts are not cached by the browser, which means they will be loaded every time. And also toGeojson I have my own, written for my needs, because in the original there are a lot of things that are not needed in it. And I download the "Distance markers" previously earlier for my own needs.

[Raruto]

the name of the track remains in the layers list (top-right control on the map), and if I deselect and then again select a track there, it re-appears. So maybe clear() is not what I'm actually looking for,

@mainzelM yes, maybe you are missing this? https://leafletjs.com/reference#control-layers-removelayer

https://github.com/Raruto/leaflet-elevation/blob/ac803fd023718e097a71f5121501a6a5043946d2/examples/leaflet-elevation_geojson-data.html#L96

---

@velocat @mainzelM do you confirm that doing as below is resolved?

// based on: https://github.com/Raruto/leaflet-elevation/issues/171#issuecomment-1030607096

// Save a reference of default "_updateChart" function (for later use)
let elevationProto = L.Control.Elevation.prototype;
let _updateChart = elevationProto._updateChart

// Override default "L.Control.Elevation" behaviour.
L.Control.Elevation.include({
   _updateChart: function(){
    if(!this._chart) return;
      _updateChart.apply(this);
   }
});

...

// initialize everything as usual
var controlElevation = L.control.elevation(opts.elevationControl.options);

// trigger the bug ?
controlElevation.clear();
controlElevation.redraw();
controlElevation.load(opts.elevationControl.url);

---

I will most likely refuse lazy loading of scripts altogether, firstly because with such loading scripts are not cached by the browser, which means they will be loaded every time. And also toGeojson I have my own, written for my needs, because in the original there are a lot of things that are not needed in it. And I download the "Distance markers" previously earlier for my own needs.

@velocat everyone tries the best to please the search engines... (maybe you might not-like: dynamic imports)

Raruto

[velocat]

@velocat @mainzelM , do you confirm that doing as below is resolved?

Yes, there is no error in this case, this is the out-of-the-box option that I suggested at: https://github.com/Raruto/leaflet-elevation/issues/171#issuecomment-1030607096

This happens only at the first launch, so this is the most logical solution. In the future, when the chart is already built, there is no such error. As you said, this is due to the time of script loading, which confirms my experiment with deferred time.

[mainzelM]

yes, maybe you are missing this? https://leafletjs.com/reference#control-layers-removelayer

I tried removeLayer on the layers for which previously eledata_loaded has been fired but that didn't change anything. But never mind, I've switched to creating a new map, I think that better matches my scenario.

do you confirm that doing as below is resolved?

Well, I think it should be possible to call clear() right away, even if load() hasn't been called before, but that's a minor point. You can close this issue, thanks a lot for your support and of course for providing this excellent extension!

[velocat]

Although, the problem comes out now in another place...

I'll explain a little how it happens for me: The file is not taken ready, but a fileloader loader is used, after that I process the received geojson myself - I check if the points have heights, if not, then I get this height for the point using an ajax request (I raised my own height server). At each iteration of the point formation cycle, I redraw the height graphs, respectively clearing it of old data and drawing it anew.

Previously, everything worked fine, but now, having solved the problem above with files from the server, and having reached the download by the user, I get another error, but still related to the lack of a chart.

I want to note that I would not bother you by staying on a more minor version, but the problem is also displayed in your examples, for example here: https://raruto.github.io/leaflet-elevation/examples/leaflet-elevation_geojson-group .

The html error is reproduced periodically in order to receive it more often - you can open the developer console, it slightly increases the execution and loading time, and in this state of the browser, the error is almost constant.

PS It seems to me that it is very difficult to solve all these problems in this case. And so far I have only one thought - to create a chart not in "onAdd", but during initialization, then when adding data, it will already exist. And you can even add a dummy to it - some null data, to be sure.

[Raruto]

https://github.com/Raruto/leaflet-elevation/issues/171#issuecomment-1030607096

Patch released in version 1.7.6

[Raruto]

Although, the problem comes out now in another place...

I did some tests with the latest version and it looks fine to me, anyway let me know if it comes back

PS It seems to me that it is very difficult to solve all these problems in this case. And so far I have only one thought - to create a chart not in "onAdd", but during initialization, then when adding data, it will already exist. And you can even add a dummy to it - some null data, to be sure.

if it works well for you, I see nothing wrong with it 😄

Regards, Raruto