search

nextzen / nextzen.js

Javascript SDK for Mapzen products
ISC License
85 stars 43 forks source link

Bump up to Leaflet 1.2 #417

Closed hanbyul-here closed 7 years ago

hanbyul-here commented 7 years ago

Leaflet is now 1.1 !

Main concern for this change is that Leaflet is moving towards to deprecating L.Mixin.Events. So It throws warning messages to components using L.mixin.Events. Currently, MapControl (of mapzen.js) , Leaflet GeocoderMapzen, TangramLayer (of mapzen.js), Leaflet Routing Machine are using L.mixin. So there are quite a lot of warning messages thrown with 1.1. However, if we deprecate the usage of L.mixin, it wouldn't work with Leaflet v0.7 even when user is using standalone version.

I think it would be better to embrace those warning messages for now (not sure how long we should lo this though), and start following Leaflet's recommendation to use L.Evented instead of L.Mixin (at least for components that we have control over)

louh commented 7 years ago

Is geocoder also throwing warnings? It's not supposed to... I tried to take care of it here:

https://github.com/mapzen/leaflet-geocoder/blob/master/src/core.js#L51-L54

  includes: L.Evented ? L.Evented.prototype : L.Mixin.Events,

It uses L.Evented where available, but if it's not present, falls back to L.Mixin.

louh commented 7 years ago

Also, not sure if you saw this already or if some of the regressions they were talking about were impacting 1.1 adoption: http://leafletjs.com/2017/08/08/leaflet-1.2.0.html

hanbyul-here commented 7 years ago

@louh I must have been confused with control-geocoder. This is awesome, I will take this approach to other components! :D