tbo47 / dagre-es

The dagrejs library exported as ES modules.
Other
32 stars 14 forks source link

Why do these graphs cause errors? #27

Closed randolchance closed 1 month ago

randolchance commented 1 year ago

Very apologies if this has been asked before somewhere. I did look for a similar issue.

I'm trying to figure out why some seemingly basic edges cause errors.

// Set the nodes
g.setNode('a', {
  label: 'A'
});
g.setNode('b', {
  label: 'B'
});
g.setNode('c', {
  label: 'C'
});
g.setNode('d', {
  label: 'D'
});
// Set the parents
g.setParent('b', 'a');
g.setParent('c', 'b');

// These edges cause errors:
//g.setEdge('a', 'b');
//g.setEdge('a', 'c');
//g.setEdge('a', 'd');
//g.setEdge('b', 'c');
//g.setEdge('a', 'a');
//g.setEdge('b', 'b');

// These edges don't cause errors
//g.setEdge('c', 'd');
//g.setEdge('d', 'c');
//g.setEdge('c', 'c');
//g.setEdge('d', 'd');

Here is a jsfiddle to demonstrate, just uncomment an edge that causes an error.

There is a package called dagre-cluster-fix on npm, but it did not seem to solve anything. I thought I'd better reach out to the community before I got too deep in trying to discern what dagre-cluster-fix might be doing to try to solve the errors, if indeed that is the error it is designed to solve; the npm page is pretty useless for explaining its function/purpose so I don't yet know.

Any guidance, advice, dagre-related theory, deeper documentation, is very much appreciated! If this is acknowledged as an issue, if it were desirable, I'd be willing to learn the theory behind dagre to try to contribute and solve this issue.

Thanks for your time!

tbo47 commented 1 year ago

I'm not sure you will get an answer to your question. The project is not maintained anymore. I'm only doing light maintenance but I don't have a deep knowledge of the code.

aloisklink commented 1 year ago

Hi @randolchance,

you might be better off posting your issue in the original project, see https://github.com/dagrejs/graphlib

This dagre-es repo is pretty much only an ESM fork, and I don't think anybody here actually knows how the code works.

The original @dagrejs project is starting to get maintained again, so you can try submitting a fix there! If it gets merged upstream (and reviewed by somebody that actually understands the code!), I'd be happy for the same fix to be applied to this repo too.

In the long-term, the original @dagrejs projects will hopefully support ESM, and we can close down this fork, but it might take a while!