Esri / esri-leaflet-vector

Display ArcGIS Online vector basemaps w/ Esri Leaflet
Apache License 2.0
56 stars 57 forks source link

`wrong listener type: undefined` console warning #210

Closed gavinr-maps closed 11 months ago

gavinr-maps commented 1 year ago

Describe the bug

When using Esri Leaflet Vector v4.0.0 and higher, there's a warning in the console when the basemap layer is added:

wrong listener type: undefined

image

Reproduction

  1. View the developer console on this JS bin that uses Esri Leaflet Vector v3.1.5: https://jsbin.com/yeyuwex/7/edit?html,output
    • expected: no warnings
    • actual: no warnings
  2. View the developer console on this JS bin that uses Esri Leaflet Vector v4.0.0: https://jsbin.com/yeyuwex/10/edit?html,output
    • expected: no warnings (same as above)
    • actual: wrong listener type: undefined image

Logs

No response

System Info

Leaflet version: 1.9.4 (latest)
Esri Leaflet version: 3.0.11  (latest)
Esri Leaflet Vector version: 4.0.0

Additional Information

No response

BrunoCaimar commented 1 year ago

The warning is generated because MaplibreGLLayer.js is returning an _resize function that does not have an implementation (undefined).

https://github.com/Esri/esri-leaflet-vector/blob/4fae9f23de9490ae98833b77e577d2a91ff3eacb/src/MaplibreGLLayer.js#L89

Not sure if the best option here is to implement _resize and call GL.resize or just call this._update that is calling _resize when needed.

Let me know what are your thoughts about it.

sheeley820 commented 1 year ago

@BrunoCaimar Good find. I was just investigating this.

sheeley820 commented 1 year ago

Would this work?

  _resize: function () {
    return this._glMap._resize;
  },

This does remove the warning for me, and I didn't see any issue with functionality. I could see in loading both contour-dev.html and custom-vtl-dev.html that _resize was called, but saw now issues in functionality or any additional errors.

I looked into just doing resize: this._glMap._resize and avoid defining the function at all, but when this code is executed glMap is not instantiated and it throws an undefined error. So implementing the _resize function is probably the best solution.

sheeley820 commented 1 year ago

I created a PR in the case that you find this solution sufficient.

BrunoCaimar commented 1 year ago

Would this work?

  _resize: function () {
    return this._glMap._resize;
  },

This does remove the warning for me, and I didn't see any issue with functionality. I could see in loading both contour-dev.html and custom-vtl-dev.html that _resize was called, but saw now issues in functionality or any additional errors.

I looked into just doing resize: this._glMap._resize and avoid defining the function at all, but when this code is executed glMap is not instantiated and it throws an undefined error. So implementing the _resize function is probably the best solution.

@sheeley820 I don't think this will work. If glMap._resize does not exist, it will throw an error in the same way. I believe that an implementation similar to the _update method will work (or maybe just call the _update on _resize).

sheeley820 commented 1 year ago

@BrunoCaimar Do you mean something like resize: this._update or

 _resize: function e {
    this._update(e);
  }

I have been using the implementation from my PR in my application and haven't seen any issues with it. Would it be enough to just place the null/undefined check in there?