Break type of mermaid.render in v10

[sidharthv96]

Describe the bug

Context: With the upcoming v10, mermaidAPI.render is async.

Sid:
The main problem is that mermaid can't be run concurrently.
So if someone calls render multiple times in a loop (or in succession) without await, everything falls apart. (As witnessed in E2E tests)
When we provide the option for callback, people might not use await and only pass the callback. Or just leave everything as is and simply upgrade to v10.
If anyone is upgrading to v10, it needs to be clear that their code will break if they don't use await.
The issue is that the breaks might not be obvious or consistent. There might not be any errors, either.
But if we break the function signature, then it's explicit. They know when upgrading to v10 that something needs to be done wherever they are calling render.

Knsv:
That makes sense. I could see how a clear call in a diagram could mess things up.
Then perhaps we also change init to run and handle unknown diagram as an error diagram with that as a message, instead of a flowchart

[slorber]

Hi

Just to clarify something, are you saying this will also not work as expected?

const [d1, d2] = await Promise.all([
  renderDiagram1(),
  renderDiagram2(),
])

Because even with async/await, intuitively it's code I would use to optimize performances.

[kiejo]

Hi,

I just wanted to give some feedback as I ran into this issue.

The main problem is that mermaid can't be run concurrently.
So if someone calls render multiple times in a loop (or in succession) without await, everything falls apart. (As witnessed in E2E tests)

Based on the documentation and method signature, it was not clear to me that render can't be run concurrently. The fact that render returns a Promise does not indicate to me that I can't have several render calls running at the same time. I think this comment shows a good example of how easy it is to run into this problem when writing idiomatic code that uses async/await.

I see two approaches that would have helped me:

1. Return a rejected Promise when calling render while another render call is still in progress. This would make it clear that this is not supported. It could use a descriptive error message, e.g. "Performing concurrent renders is not supported.".
2. An even nicer solution could be to ensure that all render calls are performed sequentially at the library level even when being called concurrently by the API consumer. It could internally use a render queue and process pending calls sequentially.

I ended up wrapping the Mermaid render call with a helper function like this:

function makeSequential(asyncFn) {
  const queue = []
  let inProgress = false

  async function processQueue() {
    if (inProgress)
      return

    inProgress = true
    while (queue.length > 0) {
      const { args, resolve, reject } = queue.shift()
      try {
        const result = await asyncFn(...args)
        resolve(result)
      } catch (error) {
        reject(error)
      }
    }

    inProgress = false
  }

  return function (...args) {
    return new Promise((resolve, reject) => {
      queue.push({ args, resolve, reject })
      processQueue()
    })
  }
}

const renderMermaid = makeSequential((mermaidAPI, id, text) => mermaidAPI.render(id, text))

I don't know the internals of Mermaid, but I imagine something similar could be used internally in the library to completely hide this complexity from the API consumer. With this approach the consumer would not need to know that render should not be called concurrently and things would simply work as expected.