From 50468f9c90fbea41c6a7e6cd5da4d203864b67bd Mon Sep 17 00:00:00 2001 From: pravit-amp <43916793+pravit-amp@users.noreply.github.com> Date: Tue, 25 Aug 2026 07:14:47 -0700 Subject: [PATCH] perf(explorer): stop re-parsing markdown on every viewer re-render (#1118) (#1195) Profiling the viewer in headless Chromium (real DOM, production React) separated remark parse time, React commit time and DOM node count across large-prose, large-code-block, deep-nested-list and GFM-table fixtures. Two findings, one of which is fixed here. 1. Every re-render re-parsed the whole document and remounted the whole subtree. remarkPlugins and the ~20-entry components map were inline literals, so each render allocated fresh arrow components; React saw a new element type per mapped tag and replaced the DOM rather than updating it. A DOM-identity probe confirmed the remount on every fixture. Because react-markdown runs the remark pipeline inside its own render, an unrelated state change -- clicking Copy, toggling Preview/Source -- re-paid the full parse. Measured 364ms for a 1000-row GFM table and 1121ms for 2000 rows. Hoisting both props to module scope and memoising the rendered element on rawContent drops re-render cost to ~0.1ms across every fixture and removes the remount (DOM identity now survives). Initial mount and node switching are unchanged, since those are genuine parses. 2. Initial parse of large GFM tables is quadratic and lives upstream in remark-gfm: the same table text parses in 12.5ms without the plugin and 1156ms with it at 2000 rows. Not addressed here -- any mitigation is a product decision and is tracked on the issue. Note that document size is the wrong threshold for this: 562KB of prose parses in 85ms while a 27KB GFM table takes 102ms. Row count, not bytes, predicts cost. Rendered output is unchanged; the components map is moved verbatim. All 66 Explorer graph-workspace tests pass. Co-authored-by: Pravit Ampapathini Co-authored-by: Sameer Kadam --- .../GraphWorkspace/MarkdownContentViewer.tsx | 195 ++++++++++-------- 1 file changed, 107 insertions(+), 88 deletions(-) diff --git a/explorer/src/workspaces/GraphWorkspace/MarkdownContentViewer.tsx b/explorer/src/workspaces/GraphWorkspace/MarkdownContentViewer.tsx index 77d7b6a6..f8121cf5 100644 --- a/explorer/src/workspaces/GraphWorkspace/MarkdownContentViewer.tsx +++ b/explorer/src/workspaces/GraphWorkspace/MarkdownContentViewer.tsx @@ -1,5 +1,5 @@ -import { useState, useRef, useEffect, type CSSProperties } from "react"; -import ReactMarkdown from "react-markdown"; +import { useState, useRef, useEffect, useMemo, type CSSProperties } from "react"; +import ReactMarkdown, { type Components } from "react-markdown"; import remarkGfm from "remark-gfm"; import { Check, Copy, Code2, Eye, ExternalLink, Image as ImageIcon } from "lucide-react"; import { GRAPH_THEME } from "./graphTheme"; @@ -47,6 +47,20 @@ export function MarkdownContentViewer({ const rawContent = typeof content === "string" ? content : ""; const hasContent = rawContent.trim().length > 0; + // react-markdown runs the whole remark pipeline synchronously inside its own + // render, so without this memo every unrelated re-render of this component -- + // clicking Copy, toggling Preview/Source -- re-parses the entire document. + // Measured at ~364ms per re-render for a 1000-row GFM table (issue #1118). + // Keyed on rawContent so a genuine node change still re-parses exactly once. + const renderedMarkdown = useMemo( + () => ( + + {rawContent} + + ), + [rawContent], + ); + const handleCopy = async () => { if (!hasContent) return; try { @@ -112,98 +126,103 @@ export function MarkdownContentViewer({ {rawContent} ) : ( -
- { - if (!isSafeUrl(href)) { - return {children}; - } - // isSafeUrl returning true guarantees href is a non-empty string. - const safeHref = href ?? ""; - // Fragment links (#section, footnote backlinks like - // #user-content-fnref-1) are in-document anchors. Opening them - // in a new tab would break GFM footnote back-navigation. - const isFragment = safeHref.startsWith("#"); - if (isFragment) { - return ( - - {children} - - ); - } - return ( - - {children} - - - ); - }, - img: ({ src, alt }) => ( - - - Image: {alt || src || "unlabeled"} - - ), - h1: ({ children }) =>

{children}

, - h2: ({ children }) =>

{children}

, - h3: ({ children }) =>

{children}

, - h4: ({ children }) =>

{children}

, - p: ({ children }) =>

{children}

, - ul: ({ children }) =>
    {children}
, - ol: ({ children }) =>
    {children}
, - li: ({ children }) =>
  • {children}
    • , - blockquote: ({ children }) =>
    {children}
    , - hr: () =>
    , - table: ({ children }) => ( -
    - {children}
    -
    - ), - thead: ({ children }) => {children}, - tbody: ({ children }) => {children}, - tr: ({ children }) => {children}, - th: ({ children }) => {children}, - td: ({ children }) => {children}, - pre: ({ children }) => 
    {children}
    , - // C-1: discard `node` here too — code elements are custom components - // and would otherwise receive node="[object Object]" in the DOM. - code: ({ className: codeClass, children }) => { - const isInline = !codeClass && typeof children === "string" && !children.includes("\n"); - return ( - - {children} - - ); - }, - }} - > - {rawContent} -     -
    +
    {renderedMarkdown}
    )} ); } +/* ─── Markdown rendering config ───────────────────────────────────── */ + +// Both props are hoisted to module scope so they keep a stable identity across +// renders. As inline literals they allocated a fresh plugin array and ~20 fresh +// arrow components on every render, which made React treat every mapped tag as a +// new element type and remount the entire rendered subtree instead of updating +// it (issue #1118). The arrow bodies only read the style constants below at call +// time, so declaring the map before them is safe. +const REMARK_PLUGINS = [remarkGfm]; + +const MARKDOWN_COMPONENTS: Components = { + // C-1: react-markdown passes a HAST `node` prop (the raw AST + // Element) to every custom component override via passNode:true. + // In React 19 any unknown prop spreads onto a native element are + // serialised as HTML attributes, producing node="[object Object]" + // on every rendered link. Fix: destructure `node` by name so it + // is explicitly discarded, then spread `...rest` to preserve all + // other legitimate HAST/remark-gfm attributes — e.g. the `id`, + // `aria-describedby`, `aria-label`, `data-footnote-ref`, + // `data-footnote-backref`, and `class` attrs that GFM footnotes + // require for correct in-page navigation and accessibility. + // + // C-2: fragment links (#anchor, GFM footnote backlinks) must + // navigate within the current document. External links continue + // to use target="_blank" with noopener noreferrer. + // + // eslint-disable-next-line @typescript-eslint/no-unused-vars + a: ({ href, children, title, node: _node, ...rest }) => { + if (!isSafeUrl(href)) { + return {children}; + } + // isSafeUrl returning true guarantees href is a non-empty string. + const safeHref = href ?? ""; + // Fragment links (#section, footnote backlinks like + // #user-content-fnref-1) are in-document anchors. Opening them + // in a new tab would break GFM footnote back-navigation. + const isFragment = safeHref.startsWith("#"); + if (isFragment) { + return ( + + {children} + + ); + } + return ( + + {children} + + + ); + }, + img: ({ src, alt }) => ( + + + Image: {alt || src || "unlabeled"} + + ), + h1: ({ children }) =>

    {children}

    , + h2: ({ children }) =>

    {children}

    , + h3: ({ children }) =>

    {children}

    , + h4: ({ children }) =>

    {children}

    , + p: ({ children }) =>

    {children}

    , + ul: ({ children }) => , + ol: ({ children }) =>
      {children}
    , + li: ({ children }) =>
  • {children}
    • , + blockquote: ({ children }) =>
    {children}
    , + hr: () =>
    , + table: ({ children }) => ( +
    + {children}
    +
    + ), + thead: ({ children }) => {children}, + tbody: ({ children }) => {children}, + tr: ({ children }) => {children}, + th: ({ children }) => {children}, + td: ({ children }) => {children}, + pre: ({ children }) => 
    {children}
    , + // C-1: discard `node` here too — code elements are custom components + // and would otherwise receive node="[object Object]" in the DOM. + code: ({ className: codeClass, children }) => { + const isInline = !codeClass && typeof children === "string" && !children.includes("\n"); + return ( + + {children} + + ); + }, +}; + /* ─── Styles ──────────────────────────────────────────────────────── */ const viewerContainerStyle: CSSProperties = {