Upgrade LeafletJS to 1.x

[bhaskarvk]

The issue

Now that LeafletJS 1.x has been out and stable for a while, it's time to think about upgrading to it. However it's not as simple and as straight forward as it sounds, so I'm creating this issue to capture all information related to the upgrade.

Initial Assessment

The first order of thing is to adapt any existing code to use 1.x API, and the good news is that not much has changed, so most of the code can work as is. The only two things I am aware of are

• L.circle now accepts radius as an option (like L.circleMarker) rather than a second argument
• Labels are not supported by the Leaflet.label plugin anymore, rather they are incorporated in the core dist. and renamed to L.Tooltip

These are both minor changes can can be done only on the JS side w/o having to rewrite the R API.

Now for the hard parts.

The hardest part is going to be upgrading the plugins used. As plugins are individual contributions, it is up to the plugin author to migrate to 1.x and unfortunately not all have done so. We need to review this on a case by case basis. Starting with Proj4Leaflet which is critical in supporting custom projections. Proj4Leaflet hasn't been fully migrated to 1.x, but a branch does exists claiming to support 1.x. The problem is that the maintainer of Proj4Leaflet has not found spare time to work on it in a while and the branch is > 6 months old.

Next are some other plugins we use in the package such as AwesomeMarkers, Measure, Minimap, Terminator, omnivore, leaflet-providers, Easybutton, marker cluster, location filter (for crosstalk).
The repos of all these plugins needs to be examined and we need to determine what efforts will require for each of them.

Lastly we have the leaflet sup-packages, starting with leaflet.extras (by your's truly), and mapview. These two use a lot of leaflet plugins and they too must be examined for the level of efforts required.

The benefits

• 0.7 branch is no longer being developed or fixed.
• 1.x branch has a lot of performance and bug fixes.
• 1.x has support for some really cool stuff like vector tiles, webgl and more.
• Always pays off to stay current.

Action Plan

• I can take a first stab at porting just the leaflet package (along with any plugins it uses) to 1.x
• For preventing any compatibility issues I will call this package leaflet2 (leaflet2 which is based on LeafletJS 1.x, I know how ironic that sounds).
• Once this is in a usable condition, I'll begin to work on leaflet.extras and more.

@jcheng5 @hadley @timelyportfolio @tim-salabim @mdsumner Please provide your inputs on this. This is quite critical in keeping the package up to date and viable for the coming years.

One thing you guys can help out immediately is discussing whether this needs to be a new package (leaflet2), or simply continue with the same name. I don't think compatibility is going to be an issue for users of just this packge, but for sub-package it could lead to problems if we retain the same name. e.g. some plugins in mapview / leaflet.extras may not have been ported to 1.x, and ergo won't work with the latest leaflet package (if we were to retain the same name).

[tim-salabim]

This came around on R-pkg-devel today

https://mailman.stat.ethz.ch/pipermail/r-package-devel/2017q1/001256.html

Quote @eddelbuettel :

"AFAIK that practice of package inflation is outlawed by CRAN Policy now:

 * Changes to CRAN packages causing significant disruption to other
   packages must be agreed with the CRAN maintainers well in advance of
   any publicity. Introduction of packages providing back-compatibility
   versions of already available packages is not allowed.

R itself has mechanisms for this: .Deprecated was already mentioned.

Many other packages "warn now", phase in new code and then offer a toggle
(maybe via options()) to get the old behaviour.

As package authors, we have a "contract" with our users. Sure, we can break
it willy-nilly (and the informal reputation mechanism may make us pay for
this) or we can try to hold the contract and accomodate.

It's programming. There is always another layer we can insert."

[jcheng5]

Let's not decide on any solutions until we know how bad the breakage is. I'm going to spend some time on Leaflet starting next week, and can start with Leaflet 1.0 testing.

I don't feel comfortable though releasing 1.1 of this package to CRAN until we at least have some confidence that we're not going to have to drop any of the recently introduced features (I don't think it's likely, but better to know).

[bhaskarvk]

FWIW I do think that eventually everything will work with 1.x and the situation is not terrible. I agree with your concerns regarding pushing the new stuff in the package to CRAN.

Would you have a few mins to discuss this over a call?

[bhaskarvk]

Woohoo, Proj4Leaflet just published 1.0 support kartena/Proj4Leaflet#126

[timelyportfolio]

I'd like to help in this effort. Should we create a project https://github.com/rstudio/leaflet/projects under this Github repo to track progress?

[bhaskarvk]

I have already started a bit of work on 1.0. But would love to cooperate. @timelyportfolio For leaflet we're already using waffle.io https://waffle.io/rstudio/leaflet.
Personally I like waffle coz you can track multiple repos as opposed to GH projects which I tied to the repo.

[timelyportfolio]

Do we have a working dev branch with > 1.0? I'd love to start tackling remaining items especially regarding the core R leaflet API.

[bhaskarvk]

Actually I am swamped with personal stuff + work stuff. Don't let me hold you back.

[timelyportfolio]

So, anybody out there listening, just to confirm no one else is working or has worked on this problem? I just want to make sure that in a world of scarce resources that I am not duplicating efforts. Thanks!

[jcheng5]

I'm not working on it either yet.

[tim-salabim]

Is there an expected timeline? And how detrimental will this update likely be for existing leaflet functionality?

[timelyportfolio]

wish me luck :) https://github.com/timelyportfolio/leaflet/tree/v1.0

[timelyportfolio]

Don't expect this to be easy, but R and JS tests all pass. Now, for the hard slog...

[bhaskarvk]

I won't be too encouraged with the tests passing. They are fairly rudimentary ;)

From my notes here're a couple of things to watch out for.

• L.circle now accepts radius as an option (like L.circleMarker) rather than a second argument
• Labels are now Tooltips and part of core JS lib, but no need to change R API. Just delete the leaflet-label JS plugin and change all L.label to L.tooltip.

Some big plugins you'll need to watch out for

• Marker Cluster
• Proj4Leaflet (This one got 1.x compatibility only recently)

[timelyportfolio]

Thanks for the headstart. Hopefully I can add some more tests that will help in future since the cadence for leaflet is now much quicker. I will try to log all I find in https://github.com/timelyportfolio/leaflet/blob/v1.0/inst/errors/errors.Rmd.