Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade to the latest version of d3 5.x #343

Closed
alexcojocaru opened this issue Oct 17, 2020 · 2 comments
Closed

Upgrade to the latest version of d3 5.x #343

alexcojocaru opened this issue Oct 17, 2020 · 2 comments

Comments

@alexcojocaru
Copy link
Contributor

Using an older version of d3 (i.e. 3.x) in the brouter-web app makes it difficult to integrate leaflet plugins which depend on the latest d3 (i.e. 5.x atm). When trying to integrate the Leaflet.Heightgraph, as a replacement for the Leaflet.Elevation currently used to render the elevation profile of the route, and since Heightgraph depends on d3 5.x, I only had two options:

  1. Backport the Heightgraph to d3 3.x, which I have done without too much trouble; going forward with this option means that:
    1. someone would have to maintain the fork and release it whenever needed (Backporting to d3 v3 GIScience/Leaflet.Heightgraph#101)
    2. as more and more d3 5.x APIs, with no direct translation to d3 3.x APIs, are used by the Heightgraph (e.g. the color category), the backporting could get complex and the fork could diverge significantly
  2. Upgrade d3 to the latest 5.x in the brouter-web, so we could integrate the Heightgraph as it; an integration plugin (similar to the Elevation integration plugin) would be required, but I have written it already - it is simpler than the Elevation integration, thanks to the resize support in Heightgraph, as well as other APIs which I have already integrated into the project to make it easier to embed.
    This was my first choice and I took a stab at it, but it got pretty hairy and I fell back to the other option (backporting the Heightgraph to d3 3.x). FWIW this would be the cleanest option, and @nrenner suggested that I should start a discussion on this, so here we go...

On a side note, here's a screenshot of a route in brouter-web with the Heightgraph integrated and enabled:
screenshot

@nrenner
Copy link
Owner

nrenner commented Oct 29, 2020

The reason I wanted to discuss is to see if there is a way forward with option 2, as I am planning to add ES6 syntax support through Babel anyway - but no modules yet.

Option 3 is to use the already transpiled and compiled bundle, see PR #345. Probably the preferred option right now.

@alexcojocaru
Copy link
Contributor Author

Forgot to mention option #3 in the initial comment, thanks for doing that. I went with option #3 in the end, because #1 is an ugly hack, and #2 is not straightforward (at least for me).
Since for #3 I had to bring in the whole transpiled Heightgraph (i.e. Heightgraph + d3 + many other dependencies), that means some code duplication in the brouter-web bundle, which makes it bigger than it should be.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants