Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DeadEnd should be exposed at least as a opaque type #56

Open
turboMaCk opened this issue Sep 30, 2020 · 3 comments
Open

DeadEnd should be exposed at least as a opaque type #56

turboMaCk opened this issue Sep 30, 2020 · 3 comments

Comments

@turboMaCk
Copy link

Function parse exposes DeadEnd type to the user API. However this type is not exposed which means one can't write type annotations for function to convert these error cases.

Two solutions:

  • automatically call deadEndsToString in user API since there is not much else user can do about this type anyway
  • expose DeadEnd type as opaque type (without constructor) so it can be used on type level (in type annotations)
@dillonkearns
Copy link
Owner

Thanks for the report, that's an excellent point.

I think the right answer here is to hide DeadEnd as an internal detail. Markdown parsers aren't actually supposed to fail at all, they always fallback to something. For example, [this is treated as plain text because it's an unclosed link](https://example.com/. That shouldn't give a parser error (and doesn't in this implementation).

The only place I'm aware of that will cause a parser error right now is HTML parsing. I haven't decided what the behavior should be here, because I do think it's reasonable to treat HTML in a special way since it has a special meaning in this library. MDX does something similar, it will actually fail with invalid HTML (you can see this if you remove a closing HTML tag in the example on this page: https://mdxjs.com/).

So one option could be to expose a specific error type where the only kind of error would be specific to invalid HTML, since that's the only possible error (or at least that's the intention, and anything else would be a bug). I think it's worth revisiting whether it's worth considering having a fallback for HTML errors as well, since that's more inline with the markdown philosophy. Then we could parse to a type that has warnings in cases where there is ambiguous markup, but it would always succeed with fallbacks, and you could choose to treat those warnings as errors (for example, in a build where you want to find out about possible issues), or ignore the warnings (for example, when presenting something to a user that's already stored in a DB so you just want to do your best to present something).

Long answer to a simple issue, but I'd like to do some research on this direction. It might make sense to ship something in the meantime with one of the simpler solutions (like exposing an error type that represents only HTML parsing errors), then iterate on a design with warnings where it never fails but instead uses fallbacks.

@turboMaCk
Copy link
Author

I think your points makes sense. The option with collecting warning sounds like the most robust way to go as it would give simple and close to spec defaults as well as enable more advanced custom use-cases.

When it comes to error correcting code I think in case of Elm (and any other VDom based rendering) it might be safest not to try to be overly smart and just fallback to text node quickly to avoid potentially bad behavior in something like live updating markdown render where there would be a risk of single key stroke suddenly producing large numbers of nodes or something. I think in these live coding situations it's mostly expected to see raw markup until it becomes valid which might be also the simplest way to implement.

Regarding issue itself I don't think you need to worry about this too much. It was not causing any big problems when using the library, it just felt like it would be good to point out as it might inform some of the future design decision.

@dillonkearns
Copy link
Owner

Great points, thanks for the feedback! It's good to keep that use case in mind of quick live updates while editing markdown with a dev server. You're right that having simple plain text in those cases is helpful and probably a good enough reason to lean towards the side of having things like invalid HTML be a warning rather than an unrecoverable error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants