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

d3 v4 and TypeScipt #344

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

SamuelMarks
Copy link

I've done some work to upgrade to the latest d3; and also rewrote in TypeScript.

@syntagmatic ; not sure what your plans are for this library.

@syntagmatic
Copy link
Owner

I would go ahead and use your fork as the main development branch. I don't have time to maintain/evolve this version, so there's definitely room for a better implementation to take over. Typescript is a good idea, I just don't want to add another layer here for anyone who's been using the library for a while.

@SamuelMarks
Copy link
Author

Hmm, well @syntagmatic the main idea with TypeScript is that you just add type annotations and it'll still compile down to pure JavaScript.

So people won't need to change a line of their code to use it as a library, and for those developing they get type hints.

This is especially useful in this case, where many d3 functions have been obsoleted; the compiler just tells me:

Error:(784, 8) TS2339: Property rebind does not exist on type 'typeof "parallel-coordinates/node_modules/@types/d3/index"'.

Regardless would you be able to resolve my queries when I have questions about why things don't work?

E.g.: what is $V at (and 3 other places):

centroids.push($V([x, y]));
and what is the k meant to be at:
__.dimensions[d].yscale.domain(d3.extent(__.data, function(d) { return +d[k]; }));

@syntagmatic
Copy link
Owner

I'm not sure what the $V references, it may have been related to a vector library.

That k appears to be a bug. I doubt that block would run without errors.

@SamuelMarks
Copy link
Author

SamuelMarks commented Jan 27, 2018

Hmm, alright. Okay @syntagmatic I have finished rewriting d3.parcoords so it compiles in TypeScript.

Now down to the d3 v3 → v4 upgrade:
screen shot 2018-01-27 at 4 23 33 pm


Can you assist with the d3 v3 → v4 upgrade?

@SamuelMarks
Copy link
Author

Getting closer:
screen shot 2018-01-27 at 7 25 17 pm

Added little TODO helpers with a candidate rewrite when it wasn't a 1:1 replacement in implementation (e.g.: flattened namespace or rename):

  1. axis = d3.axis().orient('left').ticks(5);
  2. xscale.rangePoints([0, w()], 1);
  3. brush.y(__.dimensions[axis].yscale), possibly this is how
  4. xscale.rangePoints([0, w()], 1);

Then just:

  1. Replacing functor (see official upgrade guide)
  2. Replacing rebind (see official upgrade guide)

Not sure about multibrush, but it's kind of optional anyway so we can put it to one side for now.

@timpelser
Copy link

@SamuelMarks Pretty sure $V comes from sylvester: http://sylvester.jcoglan.com/

@SamuelMarks
Copy link
Author

Oh good, it has types: https://www.npmjs.com/package/@types/sylvester

@nonoesp
Copy link

nonoesp commented Jun 27, 2018

Great work. Thanks for doing this @SamuelMarks!

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.

4 participants