fix mermaidAPI.parse() behavior to match documentation, add tests to ensure behavior matches docs

[timmaffett]

This PR fixes the behavior of mermaidAPI.parse() to match to documentation here

It now returns true (instead of the parse object !?) if the diagram has valid syntax.
If there is no mermaid.parseError handler defined then it will still throw an error as before, however, if there is a mermaid.parseError handler defined then the mermaid.parseError handler will be called and then false will be returned.

The behavior of requiring a parseError() handler to be defined is that so all existing tests (prior to this PR) still pass (ie. requiring a throw when parse() is called on invalid input).

The documentation in newHandler.md for parseError is also corrected to match reality.

I had added tests to mermaidAPI.spec.js to test all the new code and that the behavior matches to documented behavior.

Within mermaid I have added a setParseErrorHandler() method that can be used instead of mermaid.parseError = function()...
The reason for this is that I am using mermaid within a Dart interop wrapper, and it is not allowed to add a new member variable to an existing javascript object, and we start with mermaid NOT having any parseError member defined.

Within the mermaid export object definition I check for a undefined value of mermaidAPI before using it. The reason for this is that mermaidAPI is sometimes NOT defined and this was causing an error when mermaid.js was required within mermaidAPI.js/mermaidAPI.spec.js (where mermaidAPI was not yet defined).

The only behavior changed by this PR is that parse() now returns true when the input is valid, and if there is a mermaid.parseError handler defined then parse() will now return false instead of throwing an exception.

I verified that all existing (and new) test pass.

The existing behavior described in the existing documentation makes sense, so I simply made the code match that 'reality'.

[sidharthv96]

I'm a bit conflicted on whether the documentation behaviour should be the correct one or not.

The current behaviour somewhat fits the cognitive flow, mermaid.parse parses the text, or it throws an error if it fails.

Giving the ability to set a separate error handler will lead to more pathways to failures, race conditions, etc. (e.g.: Multiple error handlers being set in different places). Either the error handler should be passed as part of the parse call (but this would open up the possibility of callback hell), or the parse function should throw an error (current behaviour).

If the function was named isValid then returning true/false would be more appropriate.
We should ideally be renaming parse to validate [and remove the parseError handler altogether].

mermaid.validate(code) => void if success, throws error if failure

Would this have to be released as a major version change?
Not throwing the error does break backward compatibility.

[sidharthv96]

@knsv @ashishjain0512 ?

[timmaffett]

My question here was because there are some places in the code that the parseError() method is called as
mermaid.parseError()

and a couple places where it is called as
global.mermaid.parseError()

These two should be identical, but it is more of a stylistic question for the mermaid code base.

[timmaffett]

Would this have to be released as a major version change? Not throwing the error does break backward compatibility.

To be clear the error is always still thrown, except when a mermaid.parseError handler has been set, in which case it is the user's choice whether or not to throw the error again within their parseError() handler.
All existing tests that test for errors being thrown still work correctly.

The documented behavior is the most useful, as sure we could change the docs, or the name, that that seems way more 'breaking'. We could add an additional new validate() method that works as you describe if that is needed, but the parse() function would do everything the validate() did and you could ignore the return value.

The problem for me is that throwing the exception does not help me (I can't detect it), I am using mermaid from a Dart<->JS interop wrapper.. Being able to set the parseError() callback you already have been able to do is much more versatile.
Now returning the true/false seems to be a minor bug fix (as it was always returning the parser object before...)
When the error is always thrown I have no way to detect it, or to fix it with a catch. This prevents the ability to use the parse() method at all to validate diagram source.

The current behavior somewhat fits the cognitive flow, mermaid.parse parses the text, or it throws an error if it fails.

This remains the case unless you set a parseErrorI() handler, in which case the user gets to choose to re-throw or now.
The only different is that true is now returned when the parse() method detects no error.

Giving the ability to set a separate error handler will lead to more pathways to failures, race conditions, etc. (e.g.: Multiple error handlers being set in different places). Either the error handler should be passed as part of the parse call (but this would open up the possibility of callback hell), or the parse function should throw an error (current behavior).

The existing functionality has already had the ability to set mermaid.parseError, this change just ensures that parse() itself also calls it if it is set. If it is not set the error is re-thrown an all existing users will see no difference.

If the function was named isValid then returning true/false would be more appropriate. We should ideally be renaming parse to validate [and remove the parseError handler altogether].

mermaid.validate(code) => void if success, throws error if failure

[sidharthv96]

I was referring to the documented behaviour and current implementation, this PR is absolutely fine.

[knsv]

Thanks for looking at this @timmaffett and thanks @sidharthv96 for your review. I will merge. Agree that it should be consistent in how parseError is called. I think it looks better to use mermaid.parseError instead of global.mermaid.parseError even though I suppose they would both work fine.