Fix(#4140): Async bug in mermaid.run

[sidharthv96]

📑 Summary

Fixes diagram not registered error when using mermaid.render with startonload: true.

Resolves #4140

📏 Design Decisions

When startOnLoad was true, run calls mermaidAPI.render directly.
So, if a mermaid.render call was made at the same time, there will be issues during runtime.

Now, run calls the same mermaid.render so is part of the executionQueue.

📋 Tasks

Make sure you

• 📖 have read the contribution guidelines
• 💻 have added unit/e2e tests (if appropriate)
• 📓 have added documentation (if appropriate)
• 🔖 targeted develop branch

[sidharthv96]

@knsv @aloisklink, This issue will only happen if both startOnLoad:true and mermaid.render is used.

We could avoid this by replacing the exported mermaidAPI.render with mermaid.render in mermaid.ts.
Should we do it? It won't break anything as far as I'm aware.

diff --git a/packages/mermaid/src/mermaid.ts b/packages/mermaid/src/mermaid.ts
index db72aeed..cdc86903 100644
--- a/packages/mermaid/src/mermaid.ts
+++ b/packages/mermaid/src/mermaid.ts
@@ -403,7 +403,10 @@ const mermaid: {
   setParseErrorHandler: typeof setParseErrorHandler;
 } = {
   startOnLoad: true,
-  mermaidAPI,
+  mermaidAPI: {
+    ...mermaidAPI,
+    render
+  },
   parse,
   render,
   init,

[aloisklink]

We could avoid this by replacing the exported mermaidAPI.render with mermaid.render in mermaid.ts.
Should we do it? It won't break anything as far as I'm aware.

Whatever we decide, we should do the same thing with mermaid.parse and mermaidAPI.parse.

Is it safe to call mermaidAPI.render on two different elements?:

// would this be fine, or would we get race-conditions
// by running this in parallel
Promise.all([
  mermaidAPI.render(xx1, text1, element1),
  mermaidAPI.render(xx2, text2, element2),
]);

If it's safe, we should probably keep mermaidAPI.render(), but stick a warning in the documentation that it isn't safe to run multiple times with the same element.

If it's not safe, then I guess we can replace mermaidAPI.render() with the current mermaid.render() (although I would slightly prefer moving the executionQueue.push() code into src/mermaidAPI.ts, and/or renaming the old function to renderNotThreadSafe()).

[AlbinoGeek]

Man, I can't even tell the difference between mermaid and mermaid.mermaidAPI anymore, as the methods seem to duplicate functionality, have similiar options, and even behave identically after 10.0.0 made the primary render async.

Maybe it's not strictly related to this PR, but man, why not just deprecate one in favor of the other? Maybe they still have differences I'm not privy to? If so, they should be documnted, yeah?

[sidharthv96]

mermaidAPI.render is not "threadasyncsafe". So we can't call it with Promise.all.

mermaid.render basically enqueues calls to mermaidAPI.render.

---

Maybe it's not strictly related to this PR, but man, why not just deprecate one in favor of the other? Maybe they still have differences I'm not privy to? If so, they should be documnted, yeah?

I believe the original plan was to separate mermaidAPI as the entrypoint for libraries that manually rendered diagrams, and mermaid as the entrypoint to drop into webpages. But that plan seemed to have been dropped to keep things simple.

Currently, mermaidAPI is only kept not to break backwards compatability and mermaid is the preferred way of accessing everything.

[knsv]

Thanks

[AlbinoGeek]

Thank you!