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

Migrate flowchart from dagre to elkjs #1969

Closed
redexp opened this issue Mar 30, 2021 · 17 comments
Closed

Migrate flowchart from dagre to elkjs #1969

redexp opened this issue Mar 30, 2021 · 17 comments
Assignees
Labels
Retained Nonperishable Status: Pending Is not to be executed as it currently is Type: Enhancement New feature or request

Comments

@redexp
Copy link

redexp commented Mar 30, 2021

Hello, I was playing around with flowchart and really big diagram and find out that dagre has a lot issues with blocks positioning (like some blocks that has only one edge to other block in some subgroup unexpectedly far from that group, like across hole diagram) and with edges drawing which goes through other blocks.

I found good solution - elkjs it lot more configurable, smarter and can draw edges around a blocks

@redexp redexp added Status: Triage Needs to be verified, categorized, etc Type: Enhancement New feature or request labels Mar 30, 2021
@jgreywolf jgreywolf added Retained Nonperishable Status: Pending Is not to be executed as it currently is and removed Status: Triage Needs to be verified, categorized, etc labels Apr 28, 2021
@knsv knsv self-assigned this Jul 5, 2021
@mestaritonttu
Copy link

Dagre itself was recently declared unmaintained. In its issue tracker, people recommend elkjs or springy as alternatives.

@knsv
Copy link
Collaborator

knsv commented Oct 28, 2021

This has been accentuated as dagre is not longer actively maintained. One problem with elkjs is its size though...
image

Mermaid is already a bit on the heavy side with:
image

We should be looking at lazy loading as well as investigating if tree-shaking works with elkjs. Have not seen any better options though.

@mestaritonttu
Copy link

Looking at the elkjs issue tracker, modularisation is definitely on their radar, but far from trivial due to the tooling used and other factors.

@knsv
Copy link
Collaborator

knsv commented Nov 3, 2021

I think we should start with lazy loading in mermaid, this opens up for use of larger libraries

@richardtallent
Copy link

I'm having an issue with a site built with Vite, looks like it stems from this line in dagre's "greedy-fas" file:

var Graph = require("./graphlib").Graph;

This results in "Uncaught TypeError: Cannot read properties of undefined (reading 'Graph')".

Could be related to this issue. If so, I'd be willing to load the extra 100KB if it means I can keep using Mermaid. For now, I've had to remove it from my build, so my online docs have no fancy diagrams.

@Mizux
Copy link

Mizux commented Mar 21, 2022

FYI dagre is depreacted and lack of maintainers.

Important!
This project does not have a maintainer or active project members. There won’t be any support or attention to pull requests. Please do not contact previous maintainers unless you are qualified and have the resources to make a serious commitment to fully take over ownership of the project.

ref: https://github.com/dagrejs/dagre

Does mermaid dev will continue to maintain it ? (or you should drop this deps and find an alternative...)

@mestaritonttu
Copy link

In May 2022, a dagre contributor merged a couple of PRs from a new developer (build-related stuff). No announcement or undeprecation notice yet, but at least that is counter to "there won’t be any support or attention to pull requests" :)

elkjs is also still active and releases are being tagged.

@abitrolly
Copy link

https://github.com/eclipse/sprotty might provide an alternative.

@ghost
Copy link

ghost commented Oct 4, 2022

We should do this sooner rather than later considering it's a high severity ReDoS

image

FYI current workaround is to add the following to package.json

  "resolutions": {
    "d3-color": "^3.1.0"
  }

@KartikSoneji
Copy link

Another issue with depending on dagre-d3 is it needs a browser env to run.
This means a fully headless cli is not possible, and a chrome or chromium instance will always be launched in the background, which is very resource intensive.

Migrating away from dagre might solve this issue.

@hermanussen
Copy link

It appears elkjs is being used from version 9.4.0 and up. However, since elkjs has an Eclipse Public License 2.0 (which is a copyleft license), this forces us (and any commercial closed source users) to use an older version of mermaid-js.

Is there no alternative?

@abitrolly
Copy link

Is there no alternative?

The alternative is to fork dagre and/or sponsor its development.

@alinamx
Copy link

alinamx commented May 3, 2023

It appears elkjs is being used from version 9.4.0 and up. However, since elkjs has an Eclipse Public License 2.0 (which is a copyleft license), this forces us (and any commercial closed source users) to use an older version of mermaid-js.

Is there no alternative?

I am facing the same issue right now.

@knsv
Copy link
Collaborator

knsv commented Jun 26, 2023

We could move elkjs out to be an external diagram. This would make it possible for a site integrator to add elkjs by registering it when mermaid is initialized.

I am no legal expert but though, perhaps naively, that the copyleft license only would apply if you go in and make changes in the elkjs code. And that you then need to make those changes available to the original project.

@hermanussen
Copy link

I'm also not a legal expert. But I think if you put the usage of the Elk library into a separate npm package, that would solve the issue.

In order to avoid the Copyleft license, you would just install the main package. And not the Elk "plugin".

Of course, that would mean that the main package must be usable and useful without the plugin. I'm not familiar enough with mermaid-js to know if that would be possible or not.

@ted-marozzi
Copy link
Contributor

We could move elkjs out to be an external diagram. This would make it possible for a site integrator to add elkjs by registering it when mermaid is initialized.

I am no legal expert but though, perhaps naively, that the copyleft license only would apply if you go in and make changes in the elkjs code. And that you then need to make those changes available to the original project.

This seems like the consensus here. I am not a legal expert but from what I can gather we can use elk in commercial software but any modifications to elk must be made public.

@abitrolly
Copy link

abitrolly commented Jul 26, 2023

Looks like dagre is alive again. https://www.npmjs.com/package/@dagrejs/dagre?activeTab=versions Not sure if the maintainers found the funding, or there is another reason.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Retained Nonperishable Status: Pending Is not to be executed as it currently is Type: Enhancement New feature or request
Projects
None yet
Development

No branches or pull requests