Skip to content
This repository has been archived by the owner on Mar 17, 2020. It is now read-only.

Add better support for es6 modules. #192

Closed
wants to merge 1 commit into from

Conversation

GordonSmith
Copy link

Signed-off-by: Gordon Smith [email protected]

@GordonSmith
Copy link
Author

FYI - The main issue that hit me, was that simply including d3-tip into my project forced the entire d3 library to be included (adding 600k to my project).

This code uses the typescript compiler to convert any es6 code to es5 (babel could have been an alternative).

I am not happy with the iife name "d3-tip", but that can be configured in the webpack config (I think its the only breaking change on what was there previously).

@@ -12,6 +12,7 @@
"docs"
],
"dependencies": {
"d3": "^4.0.0",
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only needed by bar.html demo

@@ -51,7 +51,7 @@

</div>
<script type="text/javascript">
var tip = d3.tip()
var tip = window["d3-tip"].tip() // Not sure this is an acceptable approach...
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not 100% ok with this - but am not sure what the best d3 v4 approach is.

@vdh
Copy link
Collaborator

vdh commented Mar 8, 2017

There's already been an update to d3 v4 modules (d3-collection & d3-selection) in the latest master branch, it just hasn't been put into a release yet.

@GordonSmith
Copy link
Author

GordonSmith commented Mar 9, 2017

@vdh Yes - I did notice that after I added all the build stuff! BUT your approach pulls in the entire selection and collection libs and doesn't allow for tree shaking on those (a small saving).

To that end I added the "module" or "jsnext" section in your package.json, which allows consumers of the project to use es6 and tree shake your project.

There are also a a few fixes in there (rootElement access), which you may want to throw an eye at - and the build process may be of interest, but feel free to close this PR if you wish.

If you like I could convert the entire project to TypeScript, which tends to be an interesting exercise as typically it will show a bunch of potential issues (even if your not interested in pulling it)!

Like I say I won't be offended if you wish to close, just trying to give back a little.

@vdh
Copy link
Collaborator

vdh commented Mar 13, 2017

@GordonSmith I'm just a collaborator, not the project owner, and unfortunately not one with a lot of free time.

I'm a big fan of ES6, but it's more important to get unit testing added first (#165), so that any conversions or additional features don't break any existing functionality. There's already been some rootElement-related issues cropping up (#175) trying to merge in updates from other contributors into an alpha build.

@stale
Copy link

stale bot commented May 1, 2018

Hey there! It looks like this issue has been automatically marked as stale because it has not had recent activity. To help the maintainers stay focused, it will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label May 1, 2018
@caged
Copy link
Owner

caged commented May 4, 2018

The latest release now uses modern import/export constructs.

@caged caged closed this May 4, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants