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

Can't render mindmap mermaid diagrams #9032

Closed
6 of 7 tasks
jaimergp opened this issue Jun 1, 2023 · 18 comments · Fixed by #9305
Closed
6 of 7 tasks

Can't render mindmap mermaid diagrams #9032

jaimergp opened this issue Jun 1, 2023 · 18 comments · Fixed by #9305
Labels
bug An error in the Docusaurus core causing instability or issues with its execution
Milestone

Comments

@jaimergp
Copy link

jaimergp commented Jun 1, 2023

Have you read the Contributing Guidelines on issues?

Prerequisites

  • I'm using the latest version of Docusaurus.
  • I have tried the npm run clear or yarn clear command.
  • I have tried rm -rf node_modules yarn.lock package-lock.json and re-installing packages.
  • I have tried creating a repro with https://new.docusaurus.io.
  • I have read the console error message carefully (if applicable).

Description

It looks like the mindmap Mermaid diagram doesn't work with the current Docusaurus integrations. Other diagrams do work as expected.

Reproducible demo

https://github.com/Quansight-Labs/cf-infra-docs

Steps to reproduce

I followed the instructions for Mermaid diagrams and modified my Docusaurus configuration accordingly (see this PR).

I would have expected the rendering to work out of the box after those changes, but it doesn't.

Expected behavior

Get the same diagram as available in mermaid.live

Actual behavior

You first see this warning:

image

Clicking on Try Again gets you to a rendered page with a faulty diagram:

image

Locally, you see this error message:

ERROR
Diagram is a promise. Use renderAsync.
    at Object.render (webpack-internal:///./node_modules/mermaid/dist/mermaid-ae477ddf.js:29923:13)
    at eval (webpack-internal:///./node_modules/@docusaurus/theme-mermaid/lib/client/index.js:44:58)
    at mountMemo (webpack-internal:///./node_modules/react-dom/cjs/react-dom.development.js:15846:19)
    at Object.useMemo (webpack-internal:///./node_modules/react-dom/cjs/react-dom.development.js:16219:16)
    at useMemo (webpack-internal:///./node_modules/react/cjs/react.development.js:1532:21)
    at useMermaidSvg (webpack-internal:///./node_modules/@docusaurus/theme-mermaid/lib/client/index.js:21:168)
    at MermaidDiagram (webpack-internal:///./node_modules/@docusaurus/theme-mermaid/lib/theme/Mermaid/index.js:14:139)
    at renderWithHooks (webpack-internal:///./node_modules/react-dom/cjs/react-dom.development.js:14985:18)
    at mountIndeterminateComponent (webpack-internal:///./node_modules/react-dom/cjs/react-dom.development.js:17811:13)
    at beginWork (webpack-internal:///./node_modules/react-dom/cjs/react-dom.development.js:19049:16)
ERROR
Diagram mindmap already registered.
    at registerDiagram (webpack-internal:///./node_modules/mermaid/dist/mermaid-ae477ddf.js:7474:11)
    at eval (webpack-internal:///./node_modules/mermaid/dist/mermaid-ae477ddf.js:29705:7)

It sounds like this discussion, which led to this commit. I assume something similar is needed here? I am not an expert in anything JavaScript, sorry 😬

Your environment

Self-service

  • I'd be willing to fix this bug myself.
@jaimergp jaimergp added bug An error in the Docusaurus core causing instability or issues with its execution status: needs triage This issue has not been triaged by maintainers labels Jun 1, 2023
@Josh-Cena
Copy link
Collaborator

Josh-Cena commented Jun 7, 2023

We use mermaid.render. Looks like we may need to use renderAsync. Not sure if this brings more issues elsewhere. Curious why this diagram needs to be async at all...

@Josh-Cena Josh-Cena added status: needs more information There is not enough information to take action on the issue. and removed status: needs triage This issue has not been triaged by maintainers labels Jun 7, 2023
@slorber
Copy link
Collaborator

slorber commented Jun 8, 2023

9.4 mentions mindmap needs renderAsync: https://github.com/mermaid-js/mermaid/releases/tag/v9.4.0

With v10.0 it seems that now Mermaid render always returns a promise (breaking chance) and renderAsync is deprecated or removed:

I guess we should upgrade to Mermaid v10 and handle the promise

While upgrading we should probably also prevent concurrent mermaid rendering issues, cf my comment here: #8357 (comment)

@slorber slorber removed the status: needs more information There is not enough information to take action on the issue. label Jun 8, 2023
@sharky98
Copy link

FYI, in my simple use case, this patch seems to do the trick. Basically, simply using useState() + useEffect() instead of useMemo() since it seems that useMemo() does not play nice with async.

diff --git a/lib/client/index.js b/lib/client/index.js
index 1c121a7adf027f62e1d57510f9d86eee62d313ea..9c0d384f2facfd36e50839c0c03d264243b4bded 100644
--- a/lib/client/index.js
+++ b/lib/client/index.js
@@ -4,7 +4,7 @@
  * This source code is licensed under the MIT license found in the
  * LICENSE file in the root directory of this source tree.
  */
-import { useMemo } from 'react';
+import { useEffect, useMemo, useState } from 'react';
 import { useColorMode, useThemeConfig } from '@docusaurus/theme-common';
 import mermaid from 'mermaid';
 // Stable className to allow users to easily target with CSS
@@ -26,7 +26,8 @@ export function useMermaidSvg(txt, mermaidConfigParam) {
      */
     const defaultMermaidConfig = useMermaidConfig();
     const mermaidConfig = mermaidConfigParam ?? defaultMermaidConfig;
-    return useMemo(() => {
+    const [svgResult, setSvgResult] = useState('');
+    useEffect(() => {
         /*
         Mermaid API is really weird :s
         It is a big mutable singleton with multiple config levels
@@ -55,6 +56,11 @@ export function useMermaidSvg(txt, mermaidConfigParam) {
         Not even documented: mermaid.render returns the svg string
         Using the documented form is un-necessary
          */
-        return mermaid.render(mermaidId, txt);
+        mermaid.render(mermaidId, txt)
+          .then((result) => {
+            setSvgResult(result.svg);
+          });
     }, [txt, mermaidConfig]);
+    
+    return svgResult;
 }
diff --git a/src/client/index.ts b/src/client/index.ts
index f8302085c44155f693aa2c68f4a6d109c5228e4b..3e069146167e4a504e30e337e19ad076fa57a0fa 100644
--- a/src/client/index.ts
+++ b/src/client/index.ts
@@ -5,7 +5,7 @@
  * LICENSE file in the root directory of this source tree.
  */
 
-import {useMemo} from 'react';
+import {useEffect,useMemo,useState } from 'react';
 import {useColorMode, useThemeConfig} from '@docusaurus/theme-common';
 import mermaid, {type MermaidConfig} from 'mermaid';
 import type {ThemeConfig} from '@docusaurus/theme-mermaid';
@@ -40,8 +40,9 @@ export function useMermaidSvg(
    */
   const defaultMermaidConfig = useMermaidConfig();
   const mermaidConfig = mermaidConfigParam ?? defaultMermaidConfig;
+  const [svgResult, setSvgResult] = useState('');
 
-  return useMemo(() => {
+  useEffect(() => {
     /*
     Mermaid API is really weird :s
     It is a big mutable singleton with multiple config levels
@@ -72,6 +73,11 @@ export function useMermaidSvg(
     Not even documented: mermaid.render returns the svg string
     Using the documented form is un-necessary
      */
-    return mermaid.render(mermaidId, txt);
+    mermaid.render(mermaidId, txt)
+      .then((result) => {
+        setSvgResult(result.svg);
+      });
   }, [txt, mermaidConfig]);
+  
+  return svgResult;
 }

@slorber
Copy link
Collaborator

slorber commented Jun 14, 2023

Thanks @sharky98, that's a good solution yes, and indeed useMemo is not meant to read promises.

A lightweight solution for that is my little package react-async-hook, but I'm hesitant to add it to Docusaurus just for this use-case.

I assume you also upgraded to v10 right?

@sharky98
Copy link

I assume you also upgraded to v10 right?

Yes, I did add an overrides with pnpm to upgrade to mermaid@10.2.3. So far so good, I have added multiple diagrams per page. I also added a way for each node to be a link1.

The only issue I have (and I don't care in my use case), is that when loading a page, I have an error message while waiting for mermaid to build and provide the diagram to the component.

Footnotes

  1. by ejecting the Mermaid component and parsing the resulting SVG to create links where needed since this is not possible with mermaid mindmap.

@Acr0most
Copy link

Acr0most commented Jul 5, 2023

It was kind of frustrating, maybe kind of fun, but more than weird to figure out how this bug occurs.
After some hours of debugging and a lot (really a lot) of coffee i found this crazy little part of code in the dev tools:

image

if we check line +29871 there is a simple condition, checking if the word "then" occurs in our rendered diagram. This could work of course for some specific error cases. But if your diagram also includes some captions or text like "if you can see this diagram, then its working" or "Rothenburg" this line of code kills the render process and we are facing this issue.

I will check the mermaid code asap or create an issue to solve this crazy brainfuck.

As a small workaround remove all occurrences of "then" in your diagram definition. ;)

@Josh-Cena
Copy link
Collaborator

@Acr0most I don't think it has anything to do with what text your diagram contains. It's because a mindmap diagram is a promise, so it has a then property, and you can't render a promise synchronously.

@Acr0most
Copy link

Acr0most commented Jul 5, 2023

@Josh-Cena For my diagram I'm facing exactly this issue und removing this text works. Think the error message doesn't match the reason behind.

edit: you're right.. think changing some lines in diagram removed cache or it worked for some seconds.. weird.

@mcclowes
Copy link

I have the same issue for the timeline diagram

@slorber slorber modified the milestones: Upcoming, 3.0 Aug 17, 2023
@hrumhurum
Copy link

The same issue with quadrantChart diagram. After upgrading mermaid package to 10.3.1, text [object Promise] is rendered instead of a diagram. The page crashes without a package upgrade.

@slorber
Copy link
Collaborator

slorber commented Sep 11, 2023

As part of the Mermaid v10 upgrade that I'd like to do for Docusaurus v3, we'll also get new features like Markdown support, requested here: #9296

@slorber
Copy link
Collaborator

slorber commented Sep 14, 2023

New Mermaid 10.4 features will be supported as part of Docusaurus v3 thanks to this PR: #9305

CleanShot 2023-09-14 at 17 08 01@2x CleanShot 2023-09-14 at 17 08 14@2x CleanShot 2023-09-14 at 17 08 26@2x

@slorber slorber added pr: new feature This PR adds a new API or behavior. and removed pr: new feature This PR adds a new API or behavior. labels Sep 14, 2023
@Olshansk
Copy link

@slorber I noticed that timeline is still unsupported. I'm planning to open up a new issues for it, but wanted to check if that's expected behaviour (or if I'm missing something) first?

@slorber
Copy link
Collaborator

slorber commented Feb 20, 2024

@Olshansk I don't use Mermaid much myself, so I don't really know.

Is this a new Mermaid component? Can you show us a repro of a timeline not working?

@Olshansk
Copy link

Is this a new Mermaid component?

It was introduced in mermaid/releases/tag/v10.0.1 on Mar 1, 2023.

Can you show us a repro of a timeline not working?

Rendered example: https://dev.poktroll.com/protocol/claim_and_proof_lifecycle#session-end
Sour Code: https://github.com/pokt-network/poktroll/blob/main/docusaurus/docs/protocol/claim_and_proof_lifecycle.md

Note: There is a screenshot next to the syntax as a workaround. I'm inserting a screenshot for reference.

Screenshot 2024-02-20 at 2 16 49 PM

@slorber
Copy link
Collaborator

slorber commented Feb 21, 2024

@Olshansk you shared your prod site source code, this is not a minimal repro.

Here's a minimal repro proof that Docusaurus can support Mermaid timelines:
https://stackblitz.com/edit/github-b3imwp?file=src%2Fpages%2Findex.md,docusaurus.config.ts

CleanShot 2024-02-21 at 10 22 20

If your site doesn't, then maybe you did something wrong, but I can't study it in depth to help you so a minimal repro is always preferable than a link to a large open source production site.

@slorber
Copy link
Collaborator

slorber commented Feb 21, 2024

```Mermaid

You shouldn't use an uppercase character here @Olshansk

@Olshansk
Copy link

@slorber

I read show us a repro as show us a repo so I apoligize for the complexity 🤦‍♂️

That being said, thank you for actually looking into it because that small typo was indeed the issue and now everything is 👌

Thanks again!! 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An error in the Docusaurus core causing instability or issues with its execution
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants