Parse method does not return a boolean

[ginger-tek]

Describe the bug
The mermaid.parse() method is supposed to return a boolean when passed a string of mermaid markdown, but it is instead returning undefined for valid markup, and an Uncaught error on invalid markup.

To Reproduce
mermaid.parse('graph TD\n') = returns undefined
mermaid.parse('graph TD\n{') = returns Uncaught {str: "Parse error on line 1...

Expected behavior
Per this section of the current documentation,

"The function mermaid.parse(txt) takes a text string as an argument and returns true if the text is syntactically correct and false if it is not."

mermaid.parse('graph TD\n') = should return true
mermaid.parse('graph TD\n{') = should return false

Screenshots

Desktop (please complete the following information):

• OS: Windows 10 Pro
• Browser: Chrome
• Version: 77.0.3865.120

[klemmchr]

Right, the docs are not matching the implementation. There is no sign of returning a boolean. It would make sense to return a boolean in the first place, however the thrown error contains the information about the parser error.

If you would change the method in a way that it returns a bool and doesn't throw an exception, this would be a breaking change in the api which should be avoided. Therefore I don't see the point in returning a boolean. This is more like a flaw in the docs.

[ginger-tek]

I see what you mean, but having the ability to return both boolean and error info would be great. Maybe the return value can be an object instead? That way you can still easily implement simple checking, but still get detailed error info upon invalid markup.

The object would probably be something like this, with a nullable error property :

{
    result: [boolean]
    error: [object] or [null]
}

For example:

var result = mermaid.parse(str);
if(!result.result) { // false
    // invalid markup
    // result object --> { result: false, error: 'Uncaught: ...' }
    var error = result.error;
}

if(result.result) { // true
    // valid markup
    // result object --> { result: true, error: null }
}

[klemmchr]

Still the parse function would throw an exception (because removing that would be a breaking change). Regarding execution flow, it would stop if you don't catch the exception. And if you catch it, you automatically know that the parser failed. Imo that's a suitable solution that would also fit the code you posted (if you change it a bit).

[ginger-tek]

Hmm, then maybe this method shouldn't really be sold as a validation check, or the docs should say that you need to use a try catch to return what you want to return. That, or just make a separate method called validate() implementing a try catch around parse() specifically for validation checking only, which would return something similar to what I wrote above

[klemmchr]

You're right, I also consider that to be a flaw in the docs. I like the idea of a separate method, that could be handy. Will implement that.

[jgreywolf]

@klemmchr Do you have any update on this? Are you still able to work on a solution?

[klemmchr]

Not really, I don't really have the spare time to help out here and I'm heavily out of sync with the codebase.

[NeilCuzon]

Will this method be deprecated, until it is fixed? In the meantime, I will be adjusting the docs to and providing an alternative method of validation and also link to this issue for reference.

If this issue does get corrected please update here:
https://github.com/mermaid-js/mermaid/edit/develop/docs/usage.md.

[sidharthv96]

The upcoming parse in v10(#4108) will support both use cases.

/**
 * Parse the text and validate the syntax.
 * @param text - The mermaid diagram definition.
 * @param parseOptions - Options for parsing.
 * @returns true if the diagram is valid, false otherwise if parseOptions.suppressErrors is true.
 * @throws Error if the diagram is invalid and parseOptions.suppressErrors is false.
 */

async function parse(text: string, parseOptions?: ParseOptions): Promise<boolean | void> {

[silverwind]

I tried using await mermaid.parse(source) in v10.0.2 but it still was returning undefined when diagram was valid, so I'm relying purely on the rejection to detect parse errors.

[silverwind]

Actually I think the return value of parse doesn't need to matter because if there is a parse error, I would like to know what it is, so I think it's fine for it to always just throw the actual error and let the return value be undefined.