Skip to content

Commit da17fbd

Browse files
heiskrCopilot
andauthored
Render the article body from hast and activate the React enhancers (#61778)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 4e8381e commit da17fbd

10 files changed

Lines changed: 176 additions & 73 deletions

File tree

src/content-render/unified/wrap-procedural-images.ts

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -32,21 +32,28 @@ function insideOlLi(ancestors: Parent[]): boolean {
3232
}
3333

3434
function visitor(node: Element, ancestors: Parent[]): void {
35-
if (insideOlLi(ancestors)) {
36-
const shallowClone: Element = Object.assign({}, node)
37-
shallowClone.tagName = 'div'
38-
shallowClone.properties = { class: 'procedural-image-wrapper' }
39-
shallowClone.children = [node]
40-
const parent = ancestors.at(-1)
41-
if (parent && parent.children) {
42-
parent.children = parent.children.map((child) => {
43-
if (child.type === 'element' && (child as Element).tagName === 'img') {
44-
return shallowClone
45-
}
46-
return child
47-
})
35+
if (!insideOlLi(ancestors)) return
36+
const parent = ancestors.at(-1)
37+
if (!parent || !parent.children) return
38+
39+
// When the image is already inside a <p> (the writer left a blank line before
40+
// it), the paragraph already provides spacing. Wrapping a <div> inside that
41+
// <p> produces invalid HTML (`<div>` cannot be a descendant of `<p>`), which
42+
// the browser silently repairs for dangerouslySetInnerHTML but causes a React
43+
// hydration mismatch when the content is rendered as real elements. So only
44+
// add the wrapper for the no-<p> (tight list) case it was designed for.
45+
if ((parent as Element).tagName === 'p') return
46+
47+
const shallowClone: Element = Object.assign({}, node)
48+
shallowClone.tagName = 'div'
49+
shallowClone.properties = { class: 'procedural-image-wrapper' }
50+
shallowClone.children = [node]
51+
parent.children = parent.children.map((child) => {
52+
if (child.type === 'element' && (child as Element).tagName === 'img') {
53+
return shallowClone
4854
}
49-
}
55+
return child
56+
})
5057
}
5158

5259
export default function wrapProceduralImages() {

src/fixtures/tests/images.ts

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,19 @@
11
import { describe, expect, test } from 'vitest'
22
import sharp from 'sharp'
3-
import type { CheerioAPI } from 'cheerio'
3+
import type { Cheerio, CheerioAPI } from 'cheerio'
4+
import type { Element } from 'domhandler'
45

56
import { get, head, getDOM } from '@/tests/helpers/e2etest'
67
import { MAX_WIDTH } from '@/content-render/unified/rewrite-asset-img-tags'
78

9+
// `getDOM` parses with `xmlMode: true`, which is case-sensitive on attribute
10+
// names. The legacy string render path emits a lowercase `srcset`, but the
11+
// React render path (hast -> JSX) emits React 19's camelCase `srcSet`. Both are
12+
// valid HTML (attribute names are case-insensitive in browsers), so read either.
13+
function srcsetOf(el: Cheerio<Element>): string | undefined {
14+
return el.attr('srcset') ?? el.attr('srcSet')
15+
}
16+
817
describe('render Markdown image tags', () => {
918
test('page with a single image', async () => {
1019
const $: CheerioAPI = await getDOM('/get-started/images/single-image')
@@ -14,7 +23,7 @@ describe('render Markdown image tags', () => {
1423

1524
const sources = $('source', pictures)
1625
expect(sources.length).toBe(1)
17-
const srcset = sources.attr('srcset')
26+
const srcset = srcsetOf(sources)
1827
expect(srcset).toMatch(
1928
new RegExp(`^/assets/cb-\\w+/mw-${MAX_WIDTH}/images/_fixtures/screenshot\\.webp 2x$`),
2029
)
@@ -54,9 +63,9 @@ describe('render Markdown image tags', () => {
5463
const sources = $('source', pictures)
5564
expect(sources.length).toBe(3)
5665

57-
expect(sources.eq(0).attr('srcset')).toContain('1x') // 0
58-
expect(sources.eq(1).attr('srcset')).toContain('2x') // 1
59-
expect(sources.eq(2).attr('srcset')).toContain('2x') // 2
66+
expect(srcsetOf(sources.eq(0))).toContain('1x') // 0
67+
expect(srcsetOf(sources.eq(1))).toContain('2x') // 1
68+
expect(srcsetOf(sources.eq(2))).toContain('2x') // 2
6069
})
6170

6271
test('image inside a list keeps its span', async () => {

src/frame/components/article/ArticlePage.tsx

Lines changed: 61 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ import { CodeTabs } from '@/frame/components/CodeTabs'
2121
import { JourneyTrackCard, JourneyTrackNav } from '@/journeys/components'
2222
import { CopyMarkdownMenu } from './ViewMarkdownButton'
2323
import { ExperimentContentSwap } from '@/events/components/experiments/ExperimentContentSwap'
24+
import { SelectionProvider } from '@/tools/components/SelectionContext'
25+
import { CodeTabsProvider } from '@/frame/components/CodeTabsGroup'
2426

2527
const ClientSideRefresh = dynamic(() => import('@/frame/components/ClientSideRefresh'), {
2628
ssr: false,
@@ -34,6 +36,7 @@ export const ArticlePage = () => {
3436
intro,
3537
effectiveDate,
3638
renderedPage,
39+
renderedPageHast,
3740
permissions,
3841
includesPlatformSpecificContent,
3942
includesToolSpecificContent,
@@ -77,7 +80,11 @@ export const ArticlePage = () => {
7780

7881
const articleContents = (
7982
<div id="article-contents">
80-
<MarkdownContent>{renderedPage}</MarkdownContent>
83+
{renderedPageHast ? (
84+
<MarkdownContent hast={renderedPageHast} />
85+
) : (
86+
<MarkdownContent>{renderedPage}</MarkdownContent>
87+
)}
8188
<ExperimentContentSwap containerRef="#article-contents" />
8289
{effectiveDate && (
8390
<div className="mt-4" id="effectiveDate">
@@ -92,56 +99,62 @@ export const ArticlePage = () => {
9299

93100
return (
94101
<DefaultLayout>
95-
<LinkPreviewPopover />
96-
<UtmPreserver />
97-
<CodeTabs />
98-
{isDev && <ClientSideRefresh />}
99-
{router.pathname.includes('/rest/') && <RestRedirect />}
100-
{currentLayout === 'inline' ? (
101-
<>
102-
<ArticleInlineLayout
103-
supportPortalVaIframeProps={supportPortalVaIframeProps}
104-
topper={<ArticleTitle>{title}</ArticleTitle>}
105-
intro={introProp}
106-
introCallOuts={introCalloutsProp}
107-
toc={toc}
108-
breadcrumbs={<Breadcrumbs />}
109-
>
110-
{articleContents}
111-
</ArticleInlineLayout>
112-
{isJourneyTrack ? (
113-
<div className="container-lg mt-4 px-3">
114-
<JourneyTrackNav context={currentJourneyTrack} />
115-
</div>
116-
) : null}
117-
</>
118-
) : (
119-
<div className="container-xl px-3 px-md-6 my-4">
120-
<div className={cx('d-none d-xxl-block mt-3 mr-auto width-full')}>
121-
<Breadcrumbs />
122-
</div>
102+
<SelectionProvider>
103+
<CodeTabsProvider>
104+
<LinkPreviewPopover />
105+
<UtmPreserver />
106+
{/* The imperative CodeTabs enhancer only runs for the string fallback
107+
path; on the hast path, CodeTabsGroup renders tabs React-natively. */}
108+
{!renderedPageHast && <CodeTabs />}
109+
{isDev && <ClientSideRefresh />}
110+
{router.pathname.includes('/rest/') && <RestRedirect />}
111+
{currentLayout === 'inline' ? (
112+
<>
113+
<ArticleInlineLayout
114+
supportPortalVaIframeProps={supportPortalVaIframeProps}
115+
topper={<ArticleTitle>{title}</ArticleTitle>}
116+
intro={introProp}
117+
introCallOuts={introCalloutsProp}
118+
toc={toc}
119+
breadcrumbs={<Breadcrumbs />}
120+
>
121+
{articleContents}
122+
</ArticleInlineLayout>
123+
{isJourneyTrack ? (
124+
<div className="container-lg mt-4 px-3">
125+
<JourneyTrackNav context={currentJourneyTrack} />
126+
</div>
127+
) : null}
128+
</>
129+
) : (
130+
<div className="container-xl px-3 px-md-6 my-4">
131+
<div className={cx('d-none d-xxl-block mt-3 mr-auto width-full')}>
132+
<Breadcrumbs />
133+
</div>
123134

124-
<ArticleGridLayout
125-
supportPortalVaIframeProps={supportPortalVaIframeProps}
126-
topper={<ArticleTitle>{title}</ArticleTitle>}
127-
intro={
128-
<>
129-
{introProp}
130-
{introCalloutsProp}
131-
</>
132-
}
133-
toc={toc}
134-
>
135-
{articleContents}
136-
</ArticleGridLayout>
135+
<ArticleGridLayout
136+
supportPortalVaIframeProps={supportPortalVaIframeProps}
137+
topper={<ArticleTitle>{title}</ArticleTitle>}
138+
intro={
139+
<>
140+
{introProp}
141+
{introCalloutsProp}
142+
</>
143+
}
144+
toc={toc}
145+
>
146+
{articleContents}
147+
</ArticleGridLayout>
137148

138-
{isJourneyTrack ? (
139-
<div className="container-lg mt-4 px-3">
140-
<JourneyTrackNav context={currentJourneyTrack} />
149+
{isJourneyTrack ? (
150+
<div className="container-lg mt-4 px-3">
151+
<JourneyTrackNav context={currentJourneyTrack} />
152+
</div>
153+
) : null}
141154
</div>
142-
) : null}
143-
</div>
144-
)}
155+
)}
156+
</CodeTabsProvider>
157+
</SelectionProvider>
145158
</DefaultLayout>
146159
)
147160
}

src/frame/components/context/ArticleContext.tsx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ export type ArticleContextT = {
1717
intro: string
1818
effectiveDate: string
1919
renderedPage: string | JSX.Element[]
20+
renderedPageHast?: import('hast').Root
2021
miniTocItems: Array<MiniTocItem>
2122
permissions?: string
2223
includesPlatformSpecificContent: boolean
@@ -60,6 +61,7 @@ interface ContextRequest {
6061
context: {
6162
page: Record<string, unknown> & { fullPath: string; title: string; intro: string }
6263
renderedPage?: string
64+
renderedPageHast?: import('hast').Root
6365
miniTocItems?: MiniTocItem[]
6466
currentJourneyTrack?: JourneyContext
6567
currentLayoutName?: string
@@ -97,6 +99,7 @@ export const getArticleContextFromRequest = (req: ContextRequest): ArticleContex
9799
intro: page.intro,
98100
effectiveDate,
99101
renderedPage: (req.context.renderedPage as string) || '',
102+
renderedPageHast: req.context.renderedPageHast,
100103
miniTocItems: req.context.miniTocItems || [],
101104
permissions: (page.permissions as string) || '',
102105
includesPlatformSpecificContent: (page.includesPlatformSpecificContent as boolean) || false,

src/frame/components/context/MainContext.tsx

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,11 @@ export const getMainContext = async (
191191
if (context.currentJourneyTrack?.trackId) {
192192
addUINamespaces(req, ui, ['journey_track_nav'])
193193
}
194+
// CodeTabs (rendered React-natively from the article body hast) needs its i18n
195+
// strings shipped to the page; only articles can contain code tabs.
196+
if (documentType === 'article') {
197+
addUINamespaces(req, ui, ['code_tabs'])
198+
}
194199

195200
// Product index pages (depth-2 index.md, e.g. actions/index.md) need the
196201
// full product tree for landing rendering.

src/frame/components/ui/MiniTocs/MiniTocs.tsx

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,11 @@ import cx from 'classnames'
44

55
import type { MiniTocItem } from '@/frame/components/context/ArticleContext'
66
import { useTranslation } from '@/languages/components/useTranslation'
7+
import {
8+
classifyToggleClass,
9+
isContentVisible,
10+
useSelection,
11+
} from '@/tools/components/SelectionContext'
712

813
import styles from './Minitocs.module.scss'
914

@@ -13,6 +18,7 @@ export type MiniTocsPropsT = {
1318

1419
function RenderTocItem(item: MiniTocItem) {
1520
const [currentAnchor, setCurrentAnchor] = useState('')
21+
const { platform, tool } = useSelection()
1622

1723
useEffect(() => {
1824
const onHashChanged = () => {
@@ -26,6 +32,14 @@ function RenderTocItem(item: MiniTocItem) {
2632
}
2733
}, [])
2834

35+
// `item.platform` holds the class string of the heading's `.ghd-tool` ancestor
36+
// (platform OR tool value). Hide the TOC entry when its platform/tool isn't the
37+
// selected one, replacing the old imperative parent-<li> `style.display` hack.
38+
const classification = classifyToggleClass(item.platform)
39+
if (classification && !isContentVisible(classification, { platform, tool })) {
40+
return null
41+
}
42+
2943
return (
3044
<>
3145
<NavList.Item

src/frame/middleware/render-page.ts

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import type { ExtendedRequest } from '@/types'
1212
import { allVersions } from '@/versions/lib/all-versions'
1313
import { transformerRegistry } from '@/article-api/transformers'
1414
import { normalizeRenderedMarkdown } from '@/article-api/lib/normalize-markdown'
15+
import { renderContentToHast } from '@/content-render/index'
1516
import { minimumNotFoundHtml } from '../lib/constants'
1617
import { contentTypeCacheControl, defaultCacheControl } from './cache-control'
1718
import { nextHandleRequest } from './next'
@@ -40,6 +41,40 @@ async function buildRenderedPage(req: ExtendedRequest): Promise<string> {
4041
return (await pageRenderTimed(context)) as string
4142
}
4243

44+
/**
45+
* Spike for #6619 (remove dangerouslySetInnerHTML): produce the article body as
46+
* a serializable hast (HTML AST) tree alongside the legacy HTML string.
47+
*
48+
* Must run AFTER buildRenderedPage, which calls page.render and populates the
49+
* context fields the pipeline reads (englishHeadings, alertTitles). We render
50+
* the same raw `page.markdown`, but with a context clone that omits
51+
* `collectMiniToc` so the mini-TOC isn't collected a second time.
52+
*
53+
* Wrapped so a hast failure can never break the page: the React layer falls
54+
* back to the string path when this is undefined. NOTE: this currently renders
55+
* the body pipeline twice; the production design (see #6619 plan) should produce
56+
* hast once and derive the string from it.
57+
*/
58+
async function buildRenderedPageHast(req: ExtendedRequest) {
59+
const { context } = req
60+
if (!context) throw new Error('request not contextualized')
61+
const { page } = context
62+
if (!page || !page.markdown) return undefined
63+
64+
try {
65+
const hastContext = { ...context, collectMiniToc: undefined }
66+
const { hast } = await renderContentToHast(page.markdown, hastContext)
67+
return hast || undefined
68+
} catch (error) {
69+
logger.error(
70+
'buildRenderedPageHast failed; falling back to string path',
71+
error instanceof Error ? error : new Error(String(error)),
72+
{ path: req.pagePath || req.path },
73+
)
74+
return undefined
75+
}
76+
}
77+
4378
function buildMiniTocItems(req: ExtendedRequest) {
4479
const { context } = req
4580
if (!context) throw new Error('request not contextualized')
@@ -119,6 +154,7 @@ export default async function renderPage(req: ExtendedRequest, res: Response) {
119154
)
120155
} else {
121156
req.context.renderedPage = await buildRenderedPage(req)
157+
req.context.renderedPageHast = await buildRenderedPageHast(req)
122158
req.context.miniTocItems = buildMiniTocItems(req)
123159
}
124160

src/tools/components/PlatformPicker.tsx

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { useEffect, useState } from 'react'
33
import { useArticleContext } from '@/frame/components/context/ArticleContext'
44
import { parseUserAgent } from '@/events/components/user-agent'
55
import { InArticlePicker } from './InArticlePicker'
6+
import { useSelection } from './SelectionContext'
67
import { OS_PREFERRED_COOKIE_NAME } from '@/frame/lib/constants'
78

89
const platformQueryKey = 'platform'
@@ -47,7 +48,8 @@ function showPlatformSpecificContent(platform: string) {
4748
}
4849

4950
export const PlatformPicker = () => {
50-
const { defaultPlatform, detectedPlatforms } = useArticleContext()
51+
const { defaultPlatform, detectedPlatforms, renderedPageHast } = useArticleContext()
52+
const { setPlatform } = useSelection()
5153

5254
const [defaultUA, setDefaultUA] = useState('')
5355
useEffect(() => {
@@ -75,7 +77,13 @@ export const PlatformPicker = () => {
7577
}
7678
cookieKey={OS_PREFERRED_COOKIE_NAME}
7779
queryStringKey={platformQueryKey}
78-
onValue={showPlatformSpecificContent}
80+
onValue={(value: string) => {
81+
// Drive visibility through React state on the hast path (#6619). Only the
82+
// string fallback (renderedPageHast undefined) still needs the imperative
83+
// DOM mutation, since that markup isn't React-owned.
84+
setPlatform(value)
85+
if (!renderedPageHast) showPlatformSpecificContent(value)
86+
}}
7987
preferenceName="os"
8088
ariaLabel="Platform"
8189
options={options}

0 commit comments

Comments
 (0)