[RFC] Split markdown rendering to a separate package.

[ditman]

Description

This PR separates the markdown-it dependency from the lit and angular rendererers, by allowing users to inject their own markdown renderer.

Warning

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 new package is 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 that can be reused across all web packages. This is just 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.

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

Fixes

• Fixes Figure out how to resolve Markdown dependency in OSS and google repository #475
• Supersedes/closes [RFC] Split markdown rendering to a separate package. #595

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.

[jacobsimionato]

Hey thanks for iterating - this seems a lot lighter! It looks like angular support is gone, but ideally it will be possible to use this same shared interface/package for Angular?

[ditman]

/gemini review

[gemini-code-assist]

Code Review

This pull request effectively refactors the markdown rendering logic into a new shared package, @a2ui/markdown-it-shared. This is a positive step towards better code organization and reusability. The dependency injection approach for providing the markdown renderer in both Angular and Lit frameworks is well-executed.

However, I have identified a critical security vulnerability due to an incomplete sanitizer implementation in the new shared package. Additionally, there are a couple of bugs in the new package: one in the HTML highlighting logic and another in a build script, which could break the publishing process. My review includes specific comments and suggestions to address these issues.

[gemini-code-assist]

The highlight function for HTML code blocks is using iframe.innerHTML, which will return an empty string. This is likely not the intended behavior. If the goal is to display a sandboxed preview of the HTML, you should probably be returning the iframe.outerHTML.

Additionally, using document.createElement will cause this code to fail in non-browser environments (e.g., during server-side rendering or in tests running in Node.js). Consider adding a check for the environment.

Suggested change

	case 'html': {
	const iframe = document.createElement('iframe');
	iframe.classList.add('html-view');
	iframe.srcdoc = str;
	iframe.sandbox.add('');
	return iframe.innerHTML;
	}
	case 'html': {
	if (typeof document === 'undefined') {
	return str; // Or escape it
	}
	const iframe = document.createElement('iframe');
	iframe.classList.add('html-view');
	iframe.srcdoc = str;
	iframe.sandbox.add('');
	return iframe.outerHTML;
	}

[ditman]

This is in fact logic that probably should disappear. I don't think that an html tagged highlight is a good signal to attempt to render an iframe :/

[ditman]

/gemini review

[gemini-code-assist]

Code Review

This pull request is a great step towards improving the maintainability and consistency of markdown rendering by centralizing the logic into a new @a2ui/markdown-it-shared package. The use of dependency injection for the renderer in both Angular and Lit frameworks is well-executed. I've identified a couple of critical issues in the new shared markdown renderer that could lead to incorrect rendering and potential security vulnerabilities. Additionally, there are a few medium-severity suggestions to improve code clarity and adhere to best practices for library development. Overall, this is a valuable refactoring, and addressing these points will make it even more robust.

[ditman]

/gemini review

[gemini-code-assist]

Code Review

This pull request refactors the markdown rendering logic by decoupling the markdown-it dependency from the lit and angular renderers and moving it into a new shared package, @a2ui/markdown-it-shared. This is a great architectural improvement that makes the renderers more modular and allows users to inject their own markdown implementation. The changes are well-implemented, including updates to the sample applications to use the new provider/context patterns. My review includes a few minor suggestions to improve code clarity and maintainability.

[jacobsimionato]

LGTM. I wonder if the package should be called a2ui-markdown-it or something? Not sure if 'shared' is needed in there?