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

feat: build standard d3-module for d3v4+ & update examples #51

Merged
merged 2 commits into from
Jun 4, 2019

Conversation

dpraul
Copy link
Contributor

@dpraul dpraul commented May 6, 2019

With the release of version 4, d3 moved over to a modular approach, wherein everything is a plugin that extends off the main d3 module. In doing so, they setup a standard approach to building d3 modules. I originally recommended webpack for bundling source code, but the rest of the d3 universe uses rollup with a standard configuration (example for d3-shape).

Summarized, a standard d3-module for v4+ entails:

  • using the standard rollup config
  • filename outputs to d3-geomap (instead of d3.geomap)
  • requiring individual packages instead of d3's main package

For convenience, I've added the following:

  • use rollup-plugin-babel to transpile ES6
  • use rollup-plugin-postcss to compile sass
  • serve examples using rollup-plugin-serve
  • remove dependency on d3-geo-projection (user can load if needed - the default projection is available in d3-geo)
  • wrap CSS styles in d3-geomap class to not styles to global DOM, e.g. <text>

I've made several other small changes around the edges that I am happy to explain if you're wondering why they were made.

Below are the breaking changes. Do note, the API does not need to be changed, but I think making each of the exports distinct calls really clears up usage.

BREAKING CHANGES:

  • files outputs are now:
    • dist/d3-geomap.js and dist/d3-geomap.min.js
    • dist/d3-geomap.css and dist/d3-geomap.min.css
  • API is now d3.geomap(), d3.choropleth(), and d3.colorbrewer()
  • d3-geomap class must be added to map elements for default styling

dpraul added 2 commits May 1, 2019 18:14
[A standard d3-module for v4+](https://bost.ocks.org/mike/d3-plugin/):
 - using rollup
 - filename outputs to d3-geomap (instead of d3.geomap)
 - requiring individual packages instead of d3's main package

For convenience:
 - use rollup-plugin-babel to transpile ES6
 - use rollup-plugin-postcss to compile sass
 - serve examples using rollup-plugin-serve
 - remove dependency on `d3-geo-projection` (user can load if needed)
 - wrap CSS styles in `d3-geomap` class to not apply global styles

BREAKING CHANGES:
 - files outputs are now:
   - `dist/d3-geomap.js` and `dist/d3-geomap.min.js`
   - `dist/d3-geomap.css` and `dist/d3-geomap.min.css`
 - API is now `d3.geomap()`, `d3.choropleth()`, and `d3.colorbrewer()`
 - `d3-geomap` class must be added to map elements for default styling
@yaph
Copy link
Owner

yaph commented May 10, 2019

Big thanks for this PR @dpraul! I really appreciate your help and the effort you put into explaining and documenting your changes. They all look really good to me.

One additional change I want to make before the 3.0.0 release is to remove the colorbrewer component and use d3-scale-chromatic instead, i. e. colorbrewer.OrRd[9] would be replaced with d3.schemeOrRd[9] or an appropriate alias in choropleth.js. Since version 3 of d3-geomap will introduce breaking changes anyway, I think it's a good opportunity to remove this code. Please let me know if you think this is a bad idea.

If you are fine with this and want to update your PR with the removed code that would be awesome. Otherwise I can make the change after merging the PR. Provided you have no convincing arguments against it.

In any case I'm really happy to make progress with this and about your substantial contribution to this project.

@yaph yaph merged commit de22395 into yaph:webpack Jun 4, 2019
@dpraul
Copy link
Contributor Author

dpraul commented Jun 4, 2019

Whew I certainly let this fall away again, didn't I?

All is well and fine that you merged it, though, as my suggestion was to be that you do that extra migration in a separate PR, just for clearer git history, before making the 3.0.0 release.

I'll do a better job of keeping up with this from here on - we're not far from integrating this package into production, so it's in our best interest for me to do so, anyway 😄

@yaph
Copy link
Owner

yaph commented Jun 5, 2019

Hey no problem. Not sure what you refer to by "extra migration" though. Do you mean my intended removal of colorbrewer? In any case, I haven't made any changes except for adding .github/FUNDING.yml from the master branch, which was not really necessary I guess.

@dpraul
Copy link
Contributor Author

dpraul commented Jun 5, 2019

Yes, I was referring to the removal of colorbrewer when I said the "extra migration" - just me trying to keep pull requests closer to single-purpose. Are you still looking for assistance in removing colorbrewer?

@yaph
Copy link
Owner

yaph commented Jun 5, 2019

I'd be happy if you send me another PR.

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

Successfully merging this pull request may close these issues.

2 participants