search

yaph / d3-geomap

A library for creating geographical maps based on D3.js
https://d3-geomap.github.io/
MIT License
133 stars 45 forks source link

No distributable provided for import/require #49

Closed dpraul closed 5 years ago

dpraul commented 5 years ago

Usually, a library delivered over npm provides a built version that another module can directly consume with import or require. This is usually done by pointing main in package.json to a file within the source bundle. If I'm understanding #44, this is the same problem they are facing - although jsDelivr is not a standard key in package.json so is not the proper solution.

To fix this, either (a) the distributable needs to be included in the source bundle uploaded to npm, (b) the package needs to be built by import/require instead of concatenation, or - ideally - (c) both!

(a) is the easiest and requires the least restructuring, and the simplicity of this library means (b) probably isn't too far off as well. If you're looking for assistance, happy to make a PR (although it may seem drastic)

yaph commented 5 years ago

Hey thanks for offering your help with this! Can you give me a few more details about the changes solution b would involve? For example which additional dependencies would come with it?

dpraul commented 5 years ago

Certainly! The only dependency you'd need to add is some sort of module bundler. I saw you were using gulp before with concatenation, but the most popular right now is webpack. Their getting started guide is a fairly straightforward introduction.

After that you should restructure your files to use import and export to share code. That should be a fairly simple exercise since you have so few files. Webpack should be able to pickup all the files being used and create a single output distributable, which others can directly consume. It can even integrate with babel (choose the "Webpack" options in the "Build Systems" section).

The only potential gotcha is making sure you don't bundle your dependencies into the same distributable since your module is a library, which you can read about here: https://webpack.js.org/guides/author-libraries/. If you externalize your outside dependencies, people consuming your package can load the dependencies once for each library that needs them - for example, we use billboard.js for charts, and we use your library for maps - both of which depend on d3. If you bundle d3, we're locked to the version within your distributable, so best to externalize it so we can decide how it's bundled.

Let me know if you have any other questions!

yaph commented 5 years ago

Thanks again! Your explanation is very helpful and webpack looks good to me. I'll give it a try and will certainly get back to you if I have questions.

yaph commented 5 years ago

I've made some progress on this in the https://github.com/yaph/d3-geomap/tree/webpack branch. I changed the API from d3.geomap to geomap.basemap() and d3.geomap.choropleth() to geomap.choropleth(). I couldn't figure out a way to keep it like before. Personally, I don't mind that change, but I am interested in your opinion as a user.

Also, I'm not sure whether I made all the required changes. It would be awesome if you could check out the webpack branch to see whether it actually solves the issue. A PR is very welcome.

dpraul commented 5 years ago

Hi! Sorry for dropping off the map there for a while, had to re-prioritize things on our push off looking at this.

I had the chance to take a look at this last week, and have opened a PR with some changes at https://github.com/yaph/d3-geomap/pull/51. They seem pretty drastic (particularly, moving entirely from webpack to rollup), but most of them are fairly straightforward. Have a look and let me know what you think!

yaph commented 5 years ago

Hey thanks a lot for the PR! I hope I get to try and review it till the end of this week.

I've looked into rollup too, because d3 is using it. I have no personal preference for rollup or webpack because I haven't really used either of them. I hope we'll get this resolved soon. Thanks again for your effort!

yaph commented 5 years ago

I just created release 3.0.0. So I'll close this issue. If something is not working as expected, please let me know. Thanks again for your help in making this happen!