Handle invalid diagrams

[SamTV12345]

I am struggling to integrate this plugin into https://github.com/remarkjs/react-markdown . It is an async function but I have a synchronous components renderer function.

        <ReactMarkdown className="grid-none border-gray-100 border-2 rounded-2xl pl-4 pt-2 pb-2 pr-4 overflow-auto"
                       children={text}
                       components={{
                            code({node, inline, className, children, ...props}) {
                               const match = /language-(\w+)/.exec(className || '')
                               return !inline && match ? (
                                   <SyntaxHighlighter
                                       children={String(children).replace(/\n$/, '')}
                                       language={match[1]}
                                       PreTag="div"
                                       {...props}
                                   />
                               ) : (
                                   <code className={className} {...props}>
                                       {children}
                                   </code>
                               )
                           }
                       }}
                           remarkPlugins={[remarkMath, remarkGfm]}
            rehypePlugins={[rehypeKatex,rehypeRaw]}
        />

[remcohaszing]

Could you provide some information? Your provided code example doesn’t appear to use remark-mermaidjs at all. Also are you using a bundler? If so, which one?

Ideally I need a git repo with a small runnable example to reproduce the issue.

[SamTV12345]

Sure. Here is the full example: https://github.com/SamTV12345/stackedit-react/tree/feature/settingsmenu . I added it to the remark array of the editor but as soon as I type ```mermaid it throws an exception. So I changed to the components section but noticed then that the code is asynchronous.

For getting started with the project type npm install, npm start and visit localhost:8080/app . The respective file is the MarkdownViewer situated in src/components/MarkdownViewer.tsx

[remcohaszing]

Ah, I see. So you’re saying you wish to handle invalid diagrams?

For example:

```mermaid

[SamTV12345]

Yes. I need to ignore the input because the user types the mermaid code. So it is created step by step. Thus there will be always invalid mermaid code when entering valid mermaid code.

[remcohaszing]

That makes sense. What would be the desired behaviour? For additional context: the use case is a live editor, where user input is often invalid while typing.

• Should invalid diagrams be ignored?
• Should an error message be displayed instead? Should it a string or a hast / mdast node?
• Should it be configurable?
• How should it behave in Node.js?

I’m leaning towards something like this:

interface RemarkMermaidOptions {
  /**
   * What to do if an error occurs while processing a diagram.
   *
   * - `fallback`: Render the error fallback.
   * - `ignore`: Ignore the code block and keep it as-is.
   * - `throw`: Throw an error, which bubbles up the unified pipeline.
   *
   * @default 'throw'
   */
  onError?: 'fallback' | 'ignore' | 'throw'

  /**
   * A fallback node to render if a diagram is invalid and `onError` is set to `ignore`.
   *
   * @param diagran The invalid diagram code as a string.
   * @returns A hast node to render.
   */
  errorFallback?: (diagram: string) => hast.Element | hast.Text
}

I believe this is non-breaking, as currently invalid diagrams result in errors too.

@ChristianMurphy @wooorm I would love to hear your opinions too.

[SamTV12345]

From my point of view that looks fine. If I could add configuration like

useEffect(()=>remarkMaidJs.onError='ignore', 
[])

and then simply append the plugin to the remark array that would do the trick.

[ChristianMurphy]

Taking a step back, on:

the remark array of the editor but as soon as I type ```mermaid it throws an exception

Both mermaid and katex can throw errors on invalid user generated content. For user generated content consider wrapping the rendering in an ErrorBoundary https://reactjs.org/docs/error-boundaries.html In this case react-markdown, though the advise generally applies to any user generated content rendering.

Certainly open to discussing more fine grained error handling for known errors, like handling invalid formulas or diagrams, in an editing context.

---

Should invalid diagrams be ignored?

personal preference, in any semi-technical language, I'd want to surface syntax errors rather than ignoring them. I'd be cautious about ignoring, even as an option. I could easily see a bunch of questions/issues of "why doesn't my mermaid diagram appear?"

Should an error message be displayed instead?

I'd lean toward using https://github.com/vfile/vfile#vfilefailreason-position-origin to surface the error, and letting adopters choose how to display that.

Should it a string or a hast / mdast node?

Maybe, I'd lean against adding this until a clear need and presentation format is determined.

Should it be configurable?

Maybe, Similar to the previous, I'd default to YAGNI until the use/need becomes clearer.

How should it behave in Node.js?

Ideally the same across platforms. This seems like something which may not benefit from hard-coding platforms and framework specific implementation details (including browser and React). It can likely be handled through standard unified/JavaScript error handling which works across platforms.

I’m leaning towards something like this

Generally looks fine, I do have some hesitation on ignore noted above. If someone wanted to achieve the same effect they could use fallback with an empty component in errorFallback, I'd rather developers explicitly declare that. For errorFallback it might make more sense to pass the error from mermaid as the first option, and maybe clarify the diagram is rawDiagramSource.

I believe this is non-breaking, as currently invalid diagrams result in errors too

Agreed, if wanted this could probably be semver minor.

[SamTV12345]

Concerning Katex its respective plugin remarkMath catches the error, marks the text red and sets a title for the user when hovering over it. Maybe that would also be an option for you.

The error boundary was a good hint. Unfortunately once an error occurs it never changes the output again and I don't know how to display that error for the user.

[remcohaszing]

For user generated content consider wrapping the rendering in an ErrorBoundary https://reactjs.org/docs/error-boundaries.html In this case react-markdown, though the advise generally applies to any user generated content rendering.

I don’t think using an error boundary for this is desirable. The mermaid diagram is only a small part of the markdown document. I think it would be valuable to render the rest of the document if only the diagram is invalid.

Should invalid diagrams be ignored?

personal preference, in any semi-technical language, I'd want to surface syntax errors rather than ignoring them. I'd be cautious about ignoring, even as an option. I could easily see a bunch of questions/issues of "why doesn't my mermaid diagram appear?"

The term ignore is ambiguous here. By ignoring it, I meant to not replace the node, so the code block stays as-is. I don’t think removing it is a good idea either.

I'd lean toward using https://github.com/vfile/vfile#vfilefailreason-position-origin to surface the error, and letting adopters choose how to display that.

I think this is a better approach than the current approach of not catching errors at all.

Should it be configurable?

Maybe, Similar to the previous, I'd default to YAGNI until the use/need becomes clearer.

I think this issue shows we do need it. Incorrect syntax appears while the user is typing. This situation requires a more nuanced way to handle errors than just throwing. However, when rendering the diagram from static content, I think throwing an error is desirable behaviour.

I’ll go with the solution I proposed, but use a different name for ignore. Perhaps keep? as-is?

FWIW Markdown Preview Mermaid Support for VSCode does remove invalid diagrams, but as @ChristianMurphy pointed out, the user could set an empty hast node instead.

[remcohaszing]

I implemented this in #14. I didn’t even need the onError property to support all scenarios.