Skip to content

Use upstream d3-sankey #7689

Closed
Closed

Description

@adamreeve

Plotly.js currently uses a forked version of d3-sankey: https://github.com/plotly/d3-sankey

This appears to have been originally forked due to the upstream maintainer not being responsive to a PR: d3/d3-sankey#19 (comment)

However, the Plotly fork is now out of date with upstream d3-sankey and missing some features, notably the ability to control the order of nodes, which has been requested in #4373.

The Plotly fork could be updated, but it appears that all the features that were added to the fork have now been added upstream or reverted, so it might be simplest to switch to using the upstream d3-sankey library directly.

Fork features:

plotly.js similarly uses a forked version of d3-sankey-cirular: https://github.com/plotly/d3-sankey-circular. This has been archived and is also behind its upstream library, which also has support for setting nodeSort.

Do you have any thoughts on this @monfera, @antoinerg or @etpinard?

Activity

  1. camdecoster commented on Jan 21, 2026

    @camdecoster
    Contributor

    Thanks for investigating this. Have you tried swapping the libraries out? If so, did things just "work"? I'm trying to understand how much work will be involved in making this change.

  2. adamreeve commented on Jan 21, 2026

    @adamreeve
    ContributorAuthor

    Yes I tested this quite a while ago and also got the nodeSort feature working, and I seem to remember that everything just worked. I will make a PR with this and see what happens.

  3. adamreeve commented on Jan 22, 2026

    @adamreeve
    ContributorAuthor

    I've opened a draft PR at #7696. This seemed to mostly work but there's a regression with the large padding test that will be a problem: https://github.com/plotly/plotly.js/pull/7696/files#r2715057996.

    I guess the upstream fix in d3/d3-sankey@0fa75d8 didn't actually address the problem correctly, or maybe this regressed in more recent versions.

  4. adamreeve commented on Apr 8, 2026

    @adamreeve
    ContributorAuthor

    Due to upstream d3-sankey not properly fixing the large node padding issue, I've instead made a PR against the plotly fork to update it: plotly/d3-sankey#12

  5. camdecoster commented on Jun 8, 2026

    @camdecoster
    Contributor

    @adamreeve do you see value in updating the plotly/d3-sankey-circular to the latest version of the base repo?

  6. adamreeve commented on Jun 8, 2026

    @adamreeve
    ContributorAuthor

    Hmm good question, the plotly fork of d3-sankey-circular doesn't contain any changes that aren't in the base repo, except for the rebranding, but there are some changes in the base repo that haven't been released to npm. Eg. the change to add nodeSort (tomshanley/d3-sankey-circular#37) hasn't been released, but it looks like that might also have introduced a bug based on the comment on that PR, so we'd need to fix that.

    So it's probably worth unarchiving that repo and updating it to get nodeSort support for circular graphs, as well as integrating the other unreleased improvements.

  7. added this to the v4.0.0 milestone on Jun 10, 2026
  8. camdecoster commented on Jun 12, 2026

    @camdecoster
    Contributor

    I dug into it a bit and it doesn't really seem worth the effort to update d3-sankey-circular. Some of the changes from 0.34.0 actually make rendering look worse. Let's just plan on upgrading d3-sankey.

  9. changed the title [-]Use upstream d3-sankey and d3-sankey-circular[/-] [+]Use upstream d3-sankey[/+] on Aug 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions