[RFC] Split markdown rendering to a separate package.

[ditman]

Important

This PR is too big to land in one go, but luckily we can land this in stages quite easily by landing the markdown-it-* packages first, and then the modifications to the renderers and samples. That also gives us some time to figure out how to publish the markdown-it-* packages automatically without the pressure of them being "depended on", or needed within G3.

Description

This PR separates the markdown-it dependency from the lit renderer, by allowing users to @provide a markdown renderer of their own.

BREAKING CHANGE: By default, now incoming markdown is rendered as a pre element, and users must inject a markdown renderer of their choosing for markdown to be rendered (see below).

In order to keep the "batteries" somewhat included in the sdk, a couple of new packages are introduced:

• @a2ui/markdown-it-shared: the pre-configured markdown-it instance for all web renderers. This allows us to have a single configured markdown renderer with markdown-it and dompurify (TBD) that can be reused across all packages. This is pretty much a markdown string -> html string converter, but if we want to add plugins to markdown-it later, all packages get the new output at once.
• @a2ui/markdown-it-lit: the bridge package between the lit renderer and markdown-it-shared. This is what people will use when using the lit renderer to @provide the markdown renderer. They can also use this package as a jumping off point to create their own markdown renderer, if they prefer anything else other than markdown-it.
• @a2ui/markdown-it-angular: the bridge package between the angular renderer and markdown-it-shared. This package provides a MarkdownItMarkdownRenderer class that can provide: MarkdownRenderer.

The lit samples, and the angular restaurant sample are updated to inject the new markdown renderer.

Fixes

• Fixes Figure out how to resolve Markdown dependency in OSS and google repository #475

Tests

• Added some unit tests to all new packages.

Pre-launch Checklist

• I signed the CLA.
• I read the Contributors Guide.
• I read the Style Guide.
• I have added updates to the CHANGELOG.
• I updated/added relevant documentation.
• My code changes (if any) have tests.

If you need help, consider asking for advice on the discussion board.

[ditman]

I posted some comments to guide through the PR, and maybe start some discussions. PTAL!

[ditman]

I don't understand why am I getting these .JSON updates, is my branch too old? Is this being updated off of master, instead of the contents of my branch?

Updating off of master sounds dangerous.

[ditman]

This is the gist of the "integration" for Lit users, I think it turned out fairly simple, but still it's a breaking change with the previous version of the package, unfortunately.

[ditman]

Renaming UI.Context.themeContext to UI.Context.theme is also a breaking change, but IMO it improves legibility a little bit (especially now that there's more than one inhabitant in the UI.Context namespace.

[ditman]

(Rebasing to resolve merge conflicts)

[ditman]

I had to tweak the build script task a little bit to ensure the new packages I added as path dependencies for the lit renderer are being built.

[yuantian-agentspace]

I didn't go through all the code changes. Overall, I agree with the approach.

nit: I see the default is noop. is it possible to make the default renderer capable to handle very basic markdown? So that it can be used out of box without a markdown provider?

[ditman]

nit: I see the default is noop. is it possible to make the default renderer capable to handle very basic markdown? So that it can be used out of box without a markdown provider?

@yuantian-agentspace I've made the NoopMarkdownRenderer a very very very simple MinimalMarkdownRenderer (you'll be familiar with it :P). It also warns programmers the first time it's used, pointing them to the more fully-featured one.

[ditman]

I've added the Angular integration that allows for DI, and removed the markdown-it dependency from the Angular renderer as well.

[jacobsimionato]

This looks super neat!

I think my main concern here is just that it adds a lot of boilerplate.

Another option here might be for us to focus on separating the standard catalog from the central renderer, and also breaking down catalogs into freestanding component implementations which are composed together into a catalog.

That way, Google internal customers could import the code and just fork the "Text" Component implementation, and the BasicCatalog definition, substituting it with a different implementation of Text, with a different Markdown dependency. I think that might be neater overall rather than us needing to formalize the concept of a Markdown renderer in this way.

It's a bit of a hack, because we'd still import all the code and then just have BUILD rule that excludes the sources with the unwanted dependency, but maybe simpler overall?

All that said, I'm still happy to land this, and maybe it's still beneficial. I don't understand the markdown rendering landscape across the industry and whether people have strong opinions or specific needs which make this injection approach valuable!

[jacobsimionato]

I wonder if it makes sense for us to not include this, just so that it's less code to maintain. I think that having a partial implementation of Markdown could ultimately be frustrating for developers, and it shouldn't be too hard to wire up a real markdown renderer.