MemberJunction / MemberJunction/MJ

Migrate @memberjunction/ng-markdown to render from markdown-core AST (follow-up to #2617)

Open
#2,665 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
TSQL
Stars
29
Forks
6
Avg merge
2d 1h
Merged PRs (30d)
323

Description

## Background

PR #2617 introduces `@memberjunction/markdown-core` — a framework-agnostic markdown engine extracted from `@memberjunction/ng-markdown`. The core owns the `marked` configuration and the MemberJunction custom extensions (SVG code blocks, collapsible headings, GitHub alerts, heading IDs, smartypants) and can emit **either**:

- an **HTML string** (`MarkdownEngine.parseToHtml`) — the path `ng-markdown` uses today, or
- a **token tree / AST** (`MarkdownEngine.parseToTokens`) — used by the new React Native renderer in the mobile app (which has no DOM).

To keep that PR low-risk, **`ng-markdown` was deliberately left on the HTML-string + DOM post-processing path** (`[innerHTML]` binding, then DOM passes that inject copy buttons, wire collapsible toggles, render mermaid). Web rendering is therefore byte-for-byte unchanged.

## Why migrate the web path to the AST eventually

The HTML-string approach is the historical design and has real drawbacks the AST path removes:

- **Two sources of truth.** The collapsible-headings extension emits HTML comment markers and a postprocess hook re-parses the string into nested `

`s; the SVG extension emits raw HTML; copy buttons and mermaid are injected by walking the rendered DOM. Rendering logic is split between marked extensions and imperative DOM code in the component/service.
- **Sanitizer friction.** The component bypasses Angular's sanitizer for SVG/HTML passthrough and re-implements JS stripping; `unwrapMiscodedHtml` exists purely to undo marked miscoding indented HTML as code blocks.
- **Divergence risk.** Web (HTML) and mobile (AST) now render from the same config but **different pipelines**. Over time they can drift. A single AST-driven renderer per platform keeps behavior aligned.

Migrating `ng-markdown` to render directly from `parseToTokens` (Angular components walking the token tree → elements, instead of `[innerHTML]` + DOM surgery) would give one source of truth, drop the DOM-fixup code, and make web/mobile parity structural rather than coincidental.

## Scope of the migration (future work)

- [ ] Define/finalize the normalized AST contract in `markdown-core` that both renderers consume (heading nesting for collapsibles, code-block tokens with language, `svgCodeBlock` tokens, alert tokens, table/list/inline tokens).
- [ ] Build an Angular token renderer in `ng-markdown` that renders the AST to Angular views (replacing the `[innerHTML]` binding).
- [ ] Re-implement the three interactive behaviors against the AST instead of post-render DOM: copy buttons, collapsible toggles, mermaid (mermaid still needs a DOM/browser render step, but driven from the token, not by scraping `

`).

- [ ] Decide the fate of `enableHtml` passthrough + `unwrapMiscodedHtml` under an AST renderer (likely a dedicated raw-HTML token rather than the current heuristic unwrap).
- [ ] Remove now-dead DOM helpers once the renderer no longer needs them.

## Regression tests required before flipping the web path

The whole point of staying on the HTML path for now is zero web regression. Before migrating, we need a regression suite (golden-output and/or component tests) covering at minimum:

- [ ] **Headings & TOC**: heading IDs (with/without `headingIdPrefix`), `getHeadingList()` output, anchor scrolling.
- [ ] **Collapsible headings**: correct nesting (parent collapse hides descendants), `startLevel`, `defaultExpanded`, `autoExpandLevels`, expand/collapse-all, expand-to-heading.
- [ ] **Code blocks**: Prism highlighting per language, copy-to-clipboard button (success/error states), language label formatting.
- [ ] **SVG code blocks**: `svgCodeBlock` rendering + `sanitizeSvgContent` (scripts, `on*` handlers, `javascript:` URLs, `foreignObject`, external `` stripped).
- [ ] **GitHub alerts** (`[!NOTE]`/`[!TIP]`/`[!IMPORTANT]`/`[!WARNING]`/`[!CAUTION]`).
- [ ] **Smartypants** typography (curly quotes, en/em dashes, ellipsis).
- [ ] **HTML passthrough**: `enableHtml` true/false, indented-HTML normalization, `unwrapMiscodedHtml` behavior, JS stripping when `enableJavaScript` is false.
- [ ] **Mermaid**: diagram render + theme (`auto`/light/dark), error handling.
- [ ] **Tables, task lists, blockquotes, nested lists, inline formatting** baseline GFM coverage.
- [ ] **Theme switching**: `data-theme` change re-renders mermaid when `mermaidTheme: 'auto'`.

Capture current (HTML-path) output as the golden baseline first, then assert the AST renderer matches it.

## Acceptance criteria

- `ng-markdown` renders from `markdown-core`'s token AST with the regression suite green and no visual diffs in MJ Explorer.
- Web and React Native renderers share the same AST contract from `markdown-core`.
- DOM post-processing for copy buttons / collapsibles is removed (or reduced to genuinely DOM-only concerns like mermaid's render step).

---

Tracking issue for follow-up to PR #2617. Not blocking that PR — the mobile app uses the AST path today; this is about bringing the web component onto it later.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.