[controversial] Breaking API proposal

[jonasfj]

In #585 I and later @srawlins sort of agreed that exposing *Syntax objects is probably a bad idea. We could document that, so consumers don't have to come to the same realization, or we could change the public API.

This is a controversial proposal, feel free to shoot it down and close the issue.
It's certainly possible that some users might be hard to migrate.

Proposed public API

• Export a List<Node> markdown(String text, {options...}) function
  • Options consist of booleans and Resolver.
  • Options can only enable/disable features.
  • Users cannot inject a custom parser or custom syntax into the mix.
• Mark everything with @Deprecated('Use the markdown() function')
  • Except Node, Element, Text and Resolver.
  • Add an extension RenderHtml on List<Node> { String renderHtml(options...) {...} }

Next major version break

• Stop exporting all the deprecated things
• Annotate Node with sealed class
• Annotate Text and Element with final class

This would prevent users from trying to make custom *Syntax classes, reorder Syntax classes, and get all sorts of weird behavior.

It would also allow us to refactor the implementation and add new features without breaking existing users.
Maybe, we keep all the internals as they are, but exposing them to the public seems a bit scary.

[jonasfj]

@isoos how breaking would this be for the way we use markdown on pub.dev?

[isoos]

@isoos how breaking would this be for the way we use markdown on pub.dev?

Most of our markdown-internals are used only in https://github.com/dart-lang/pub-dev/blob/master/app/lib/shared/markdown.dart

It seems to me that proposed changes won't break us much. We need to observe and create Node/Element classes, but otherwise we don't do subclassing of Nodes. Note however, that we are using a fix like this at the moment:

    // alert syntax disabled as it is broken in 7.2.1
    m.ExtensionSet.gitHubWeb.blockSyntaxes
        .where((s) => s is! m.AlertBlockSyntax),

so maybe some kind of configuration object that disables some of the syntax could be helpful.

[srawlins]

Hmm, interesting. We could. I don't really know what problem this solves. (I don't know if there are any users that are confused, or breaking changes we want to make, or that we've ever been hampered by releasing a breaking change with a major version bump.)

Occasionally users have remarked in issues that they have implemented a syntax themselves, so I think the extensibility is in use.

[jonasfj]

I don't really know what problem this solves.

I think this is a totally fair point! Also why it's a controversial proposal :D

It would allow us to implement AlertSyntax as a post-processing step on the List<Node> structure, before we return it to the user. Or we could implement AlertSyntax inside BlockquoteSyntax instead of artificially having to split it into a separate AlertSyntax parser.

Occasionally users have remarked in issues that they have implemented a syntax themselves, so I think the extensibility is in use.

Is that a good thing? We're offering A LOT functionality for sure. But it's largely undocumented, and unclear what can be expected to work.

The argument would be that: Promising a small set of stable features is better than offering a lot of functionality for which we don't intend to fix bugs (or even define behavior).

Or if a certain constellation of syntaxes causes an infinite-loop, crash or stops working?

Less is more.

• Most users want: markdown -> html.
• Advanced users might want to enable/disable features and resolve links.
• Super advanced users might want to post-process the generated nodes
  (to do: table for contents, dartpad embedding, mermaid diagrams, etc)

Should anyone really make custom syntaxes? Wouldn't post-processing be better, like github does for:

• Alert syntax (which is a post-processing step for blockquotes, or,
• mermaid diagrams (which is a post-processing step for code-blocks).