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 v7 #1267

Open
kachkaev opened this issue Jul 7, 2021 · 8 comments
Open

d3 v7 #1267

kachkaev opened this issue Jul 7, 2021 · 8 comments

Comments

@kachkaev
Copy link
Contributor

kachkaev commented Jul 7, 2021

Hi folks πŸ‘‹

There is a new major version of d3, which was released in June 2021:
https://github.com/d3/d3/releases/tag/v7.0.0

A number of auxiliary d3 libraries got upgraded too, e.g. d3-array: v2 β†’ v3 etc.

I work on a project that uses a few d3 libraries directly and also via visx. Thus, upgrading direct dependencies to the latest versions produces avoidable duplicates.

What is visx’s strategy on d3 dependencies? When would be a good time for you to upgrade them? Happy to help with a PR if it is needed and if time allows πŸ™‚

@williaster
Copy link
Collaborator

Hey @kachkaev thanks for opening this πŸ™

We're definitely open to updating the d3 deps. From a quick glance it doesn't look like there are too many crazy breaking changes. We don't have the whole d3 package installed anywhere, so I think it'd probably make sense to update d3-* dependencies on a per-visx package basis. I think for TS to be happy we'd hope to keep the d3 types in sync as well, it looks like there was a 7.0.0 release for those as well.

I'm not sure I have the bandwidth to do this currently but am happy to review PR(s) if others are interested πŸ‘€

@azemetre
Copy link

azemetre commented Aug 8, 2021

If no one else is taking this, I don't mind working on it.

I'll tag this issue in an open PR once I have it complete.

edit:

after updating the packages there were some issues with the current node engine not supporting the newer d3 libs using ESM, bumping it the node version to 12.22.4 resolved this issue.

But I ran into some problems with running visx-demo. next.js does not seem to like any of the D3 libraries (shape, scale, format, etc) using ESM:

Error [ERR_REQUIRE_ESM]: Must use import to load ES Module: /Users/azemetre/github/visx/node_modules/d3-shape/src/index.js
require() of ES modules is not supported.
require() of /Users/azemetre/github/visx/node_modules/d3-shape/src/index.js from /Users/azemetre/github/visx/packages/visx-shape/lib/util/D3ShapeFactories.js is an ES module file as it is a .js file whose nearest parent package.json contains "type": "module" which defines all .js files in that package scope as ES modules.
Instead rename index.js to end in .cjs, change the requiring code to use import(), or remove "type": "module" from /Users/azemetre/github/visx/node_modules/d3-shape/package.json.

Kinda don't know the best approach to solve this, any suggestions? This is the last hurdle. It's able to build and tests pass with newer D3 libs but don't know how to handle next.js ESM woes.

edit2: tried to jump ahead and update react to v17 and was also trying out nextjs v11.0.2-canary.27
(supports esm).

Only issues I had with using the canary build of nextjs was improper react installations (had multiple local copies, had issues properly building; totally on my end, don't have the time to properly resolve). Might be something to consider if it requires minimal breaking changes (had to tweak the next.config.js to allow webpack v4).

Might be better to revisit this issue after these two PRs are merged in tho:

#1268

#1301

@make-github-pseudonymous-again

Kinda don't know the best approach to solve this, any suggestions? This is the last hurdle. It's able to build and tests pass with newer D3 libs but don't know how to handle next.js ESM woes.

I believe the alternatives are:

  1. Have d3 provide a .cjs file either in package.json#main or package.json#exports so that it can be picked up by node when using require.
  2. Use await import(...) (not top-level) instead of require where it breaks, this may not be compatible with the existing synchronous API, or be easy to produce from TypeScript sources. Not sure what's the support for this in NodeJS 12.x either.
  3. Only provide ESM output for @visx libraries.

IMHO, the only reasonable alternative is 1.

@azemetre
Copy link

Kinda don't know the best approach to solve this, any suggestions? This is the last hurdle. It's able to build and tests pass with newer D3 libs but don't know how to handle next.js ESM woes.

I believe the alternatives are:

  1. Have d3 provide a .cjs file either in package.json#main or package.json#exports so that it can be picked up by node when using require.
  2. Use await import(...) (not top-level) instead of require where it breaks, this may not be compatible with the existing synchronous API, or be easy to produce from TypeScript sources. Not sure what's the support for this in NodeJS 12.x either.
  3. Only provide ESM output for @visx libraries.

IMHO, the only reasonable alternative is 1.

Awesome, I can try out again this weekend and report back.

@becca-bailey
Copy link

becca-bailey commented Feb 14, 2023

Hey! πŸ‘‹
I have seen this before and I thought I would share some things we learned from upgrading Victory (Formidable's React + SVG charting library) to the latest version of d3.

As you have figured out here, the latest version of d3 no longer supports CommonJS, which is required for node.js environments. Since d3 itself is unlikely to change this (we also went that route), we created a script to vendor our d3 dependencies to export both CommonJS and ESM, and updated our imports to use our vendored d3 packages rather than importing directly from node_modules/d3. My colleague at the time wrote a really helpful blog post that goes into more detail about this approach. https://formidable.com/blog/2022/victory-esm/

@williaster If we wanted to take a similar approach here, I'm happy to take a stab at it! Let me know what you think.

@williaster
Copy link
Collaborator

hey @becca-bailey thanks for chiming in – I've read that post and it was great πŸ™ in our 3.0.0 release we updated a few d3 deps to the latest to fix some security vulnerabilities, and now we're in a semi-broken state with regard to supporting CSJ 😞

we've discussed 3 options in #1637, the first I wanted to try was updating all of our problematic d3-* imports to import() to work in both csj/esm worlds – but it's unclear if that will work for all consumers. we haven't discussed making our own vendor packages which could be interesting. @becca-bailey how much work do you think it'd be to make @visx/vendor with cjs/esm exports which we can then use for all of our other imports? happy to review or collab on a PR to explore. (feel free to chime in on the other issue thread as well)

@kaiyoma
Copy link

kaiyoma commented May 17, 2024

Will this ever be addressed? I'm trying to upgrade to D3 7.x in my project and running into TypeScript errors because visx pulls in an ancient version of d3-shape (1.x, 5 years old).

@williaster
Copy link
Collaborator

@kaiyoma we now support d3's esm-only packages via @visx/vendor. so updating to the latest d3 packages should be doable. if you or anyone is interested in an updating a given package you can follow the instructions here #1761 (comment)

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

No branches or pull requests

6 participants