mermaid-js / mermaid

Generation of diagrams like flowcharts or sequence diagrams from text in a similar manner as markdown
https://mermaid.js.org
MIT License
72.08k stars 6.55k forks source link

`copy` function uses `Graph.prototype.edges` where it requires `Graph.prototype.nodeEdges` #3296

Open johnbender opened 2 years ago

johnbender commented 2 years ago

Description For state diagrams with edges pointing to clusters the output will be a parser error.

To Reproduce

Attempt to render the following state diagram:

stateDiagram-v2
    state Top {
        state Child1 {
          A --> B
        }
        state Child2 {
          C --> GrandChild
          state GrandChild {
            GreatGrandChild1 --> GreatGrandChild2
            state GreatGrandChild1 {
              E --> F
            }
            state GreatGrandChild2 {
              G --> H
            }
          }
        }
    }

You can see the issue in the live editor here

Steps to reproduce the behavior:

  1. Go to the link above
  2. See the "parser" error

Expected behavior

It should render the following diagram:

image

Instead it gives a parser error.

Code Sample The problem seems to be that when the graph is being rendered, the addClustersAndEdges calls extractor which in turn copies the contents of cluster nodes using copy. That function seemingly intends to copy edges on a per node basis here:

https://github.com/mermaid-js/mermaid/blob/develop/src/dagre-wrapper/mermaid-graphlib.js#L97

But the correct method to call seems to be Graph.prototype.nodeEdges ?

Desktop (please complete the following information):

Additional context Add any other context about the problem here.

knsv commented 2 years ago

Thanks for your analysis. It will make it easier to fix this.

mishugana commented 2 years ago

Would love to try and solve this

randolchance commented 1 year ago

I came across this open issue while looking into #4644, which also uses the copy function found here. Changing Graph.prototype.edges to Graph.prototype.nodeEdges (see here as suggested above does allow the diagram to render.)

I don't know if that's all there is to this solution, but that change does make a positive difference, as far as I can tell.

I can't tell if I'm not running the tests right or they're not completing properly. When I run pnpm test my console ends with (and no other errors before it):

=============

WARNING: You are currently running a version of TypeScript which is not officially supported by @typescript-eslint/typescript-estree.

You may find that it works just fine, or you may not.

SUPPORTED TYPESCRIPT VERSIONS: >=3.3.1 <5.1.0

YOUR TYPESCRIPT VERSION: 5.1.3

Please only submit bug reports when using the officially supported version.

=============
Checking formatting...
[warn] demos\flowchart-test.html
[warn] packages\mermaid-zenuml\README.md
[warn] Code style issues found in 2 files. Forgot to run Prettier?
 ELIFECYCLE  Command failed with exit code 1.
 ELIFECYCLE  Test failed. See above for more details.

   When I run pnpm e2e some tests definitely appear to be failing, but I have no frame of reference to know if any of this is normal.