d4
d4 copied to clipboard
Refactor as node module
Hello again! Implementation-wise this is not too different from #14, but I'm starting to realize that the implications reach far beyond browserify. One amazing and under-used feature of d3 is its ability to run on node, which means you can pre-render charts on the server. Would be excellent if d4 could do the same.
I don't think there's anything major blocking it except code structure and the dependency on a browser global, as discussed in the other issue, as d3 is fully node-compatible. Thoughts @heavysixer ?
Hey @nsonnad as far as I understand your point I think you are entirely correct. It may be just reconfiguring the way d4 exposes itself as a variable. I assume we can follow whatever steps d3 does since it obviously works within node.
I guess we'll want to support all of: UMD, bower, CommonJS (ie node), webpack. It looks like in-progress d3 version 4.0 is using ES6 and a custom version of bundler, while d3 3.x uses Makefile targets. There is also browserify's standalone build option, which might be the best option.
+1 for a CommonJS compatible version
A very quick attempt at this and found some issues to work out. I haven't been able to run the tests since that rebuilds the library and I've simply made changes to the compiled d4.js file.
- I got rid of all intermediate anonymous functions and made one large one
- It relies on a global d3 reference, so in its environment check, it should load
var d3 = require('d3')
if it's in CommonJs etc. - For some reason I get the following error on
tspan
.
if (tspan.node().getComputedTextLength() > width - Math.abs(x)) {
^
TypeError: tspan.node(...).getComputedTextLength is not a function
Logging tspan
gives
[ { _events: {},
_childNodesList: null,
_ownerDocument: [Object],
_childrenList: null,
_version: 1,
_memoizedQueries: {},
_readonly: false,
_namespaceURI: 'http://www.w3.org/2000/svg',
_prefix: null,
_localName: 'tspan',
_attributes: [Object],
_settingCssText: false,
_style: [Object],
_attached: true,
_attachedToDocument: true,
__data__: 700 },
parentNode: { _events: {},
_childNodesList: null,
_ownerDocument: [Object],
_childrenList: null,
_version: 2,
_memoizedQueries: {},
_readonly: false,
_namespaceURI: 'http://www.w3.org/1999/xhtml',
_prefix: null,
_localName: 'html',
_attributes: {},
_settingCssText: false,
_style: [Object],
_attached: true,
_attachedToDocument: true } ] ]
Commenting out that if statement seems to get the library working. I'll try and find time to investigate more and get the tests working to see about any unintended consequences. If someone else wants to jump on too that'd be great.
Thanks for the progress @mhkeller. Let me see if I can help suss things out. I think that @nsonnad was working on this too.
@mhkeller is your work in a fork i can see?
Here you go. The diff is ugly since the version I had pulled from npm was 8.10 but I forked a more recent version. I was gonna redo but I couldn't find the exact tag in the GitHub releases so figured the diff would be messy in any event.
To help, I've added inline comments documenting the bulleted changes described above: https://github.com/mhkeller/d4/commit/d16c91c488e8b93af8df90b3fb0f26685a90de10