Skip to content

Commit cf117c2

Browse files
heiskrCopilot
andauthored
Fix article API 30s+ hangs on discovery-landing pages (#60416)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 1246760 commit cf117c2

File tree

8 files changed

+185
-81
lines changed

8 files changed

+185
-81
lines changed

src/article-api/lib/get-all-toc-items.ts

Lines changed: 67 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import type { Context, Page } from '@/types'
22
import type { LinkData } from '@/article-api/transformers/types'
3+
import { renderLiquid } from '@/content-render/liquid/index'
34
import { resolvePath } from './resolve-path'
45

56
interface PageWithChildren extends Page {
@@ -28,9 +29,16 @@ export async function getAllTocItems(
2829
options: {
2930
recurse?: boolean
3031
renderIntros?: boolean
32+
/** Use Liquid-only rendering for titles and intros instead of the full
33+
* Markdown/unified pipeline. Resolves {% data %} variables without
34+
* the cost of Markdown parsing, unified processing, and Cheerio unwrap. */
35+
liquidOnly?: boolean
36+
/** Only recurse into children whose resolved path starts with this prefix.
37+
* Prevents cross-product traversal (e.g. /en/rest listing /enterprise-admin). */
38+
basePath?: string
3139
} = {},
3240
): Promise<TocItem[]> {
33-
const { recurse = true, renderIntros = true } = options
41+
const { recurse = true, renderIntros = true, liquidOnly = false } = options
3442
const pageWithChildren = page as PageWithChildren
3543
const languageCode = page.languageCode || 'en'
3644

@@ -44,43 +52,69 @@ export async function getAllTocItems(
4452
)
4553
const pathname = pagePermalink ? pagePermalink.href : `/${languageCode}`
4654

47-
const items: TocItem[] = []
48-
49-
for (const childHref of pageWithChildren.children) {
50-
const childPage = resolvePath(childHref, languageCode, pathname, context) as
51-
| PageWithChildren
52-
| undefined
53-
54-
if (!childPage) continue
55-
56-
const title = await childPage.renderTitle(context, { unwrap: true })
57-
const intro =
58-
renderIntros && childPage.intro
59-
? await childPage.renderProp('intro', context, { textOnly: true })
60-
: ''
61-
62-
const childPermalink = childPage.permalinks.find(
63-
(p) => p.languageCode === languageCode && p.pageVersion === context.currentVersion,
55+
// On the first call, set basePath to this page's path so recursion
56+
// stays within the same product section.
57+
const basePath = options.basePath ?? pathname
58+
59+
const resolvedChildren = pageWithChildren.children
60+
.map((childHref) => ({
61+
childHref,
62+
childPage: resolvePath(childHref, languageCode, pathname, context) as
63+
| PageWithChildren
64+
| undefined,
65+
}))
66+
.filter(
67+
(entry): entry is { childHref: string; childPage: PageWithChildren } =>
68+
entry.childPage !== undefined,
6469
)
65-
const href = childPermalink ? childPermalink.href : childHref
6670

67-
const category = childPage.category || []
71+
const items = await Promise.all(
72+
resolvedChildren.map(async ({ childHref, childPage }) => {
73+
const childPermalink = childPage.permalinks.find(
74+
(p) => p.languageCode === languageCode && p.pageVersion === context.currentVersion,
75+
)
76+
const href = childPermalink ? childPermalink.href : childHref
77+
78+
let title: string
79+
if (liquidOnly) {
80+
const raw = childPage.shortTitle || childPage.title
81+
try {
82+
title = await renderLiquid(raw, context)
83+
} catch {
84+
// Fall back to raw frontmatter string if Liquid rendering fails
85+
// (e.g. translation errors in non-English pages)
86+
title = raw
87+
}
88+
} else {
89+
title = await childPage.renderTitle(context, { unwrap: true })
90+
}
6891

69-
const item: TocItem = {
70-
href,
71-
title,
72-
intro,
73-
category,
74-
childTocItems: [],
75-
}
92+
let intro = ''
93+
if (renderIntros && childPage.intro) {
94+
if (liquidOnly) {
95+
const rawIntro = childPage.rawIntro || childPage.intro
96+
try {
97+
intro = await renderLiquid(rawIntro, context)
98+
} catch {
99+
intro = rawIntro
100+
}
101+
} else {
102+
intro = await childPage.renderProp('intro', context, { textOnly: true })
103+
}
104+
}
76105

77-
// Recursively get children if enabled
78-
if (recurse && childPage.children && childPage.children.length > 0) {
79-
item.childTocItems = await getAllTocItems(childPage, context, options)
80-
}
106+
const category = childPage.category || []
81107

82-
items.push(item)
83-
}
108+
// Only recurse if the child is within the same product section
109+
const withinSection = href.startsWith(basePath)
110+
const childTocItems =
111+
recurse && withinSection && childPage.children && childPage.children.length > 0
112+
? await getAllTocItems(childPage, context, { ...options, basePath })
113+
: []
114+
115+
return { href, title, intro, category, childTocItems } as TocItem
116+
}),
117+
)
84118

85119
return items
86120
}

src/article-api/tests/discovery-landing-transformer.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,4 +51,39 @@ describe('discovery landing transformer', () => {
5151
// Should have at least one section
5252
expect(res.body).toContain('##')
5353
})
54+
55+
test('articles include intros rendered with Liquid', async () => {
56+
const res = await get(makeURL('/en/get-started/article-grid-discovery'))
57+
expect(res.statusCode).toBe(200)
58+
59+
// Articles should have intros (Liquid-rendered from frontmatter)
60+
expect(res.body).toContain('This is the first test article for the article grid component.')
61+
expect(res.body).toContain('This is the fourth test article for the article grid component.')
62+
})
63+
64+
test('filters articles by includedCategories', async () => {
65+
const res = await get(makeURL('/en/get-started/discovery-filtered'))
66+
expect(res.statusCode).toBe(200)
67+
68+
// "Included Article" has category "Getting started" and should appear
69+
expect(res.body).toContain('[Included Article]')
70+
71+
// "Excluded Article" has category "Advanced" and should be filtered out
72+
expect(res.body).not.toContain('[Excluded Article]')
73+
})
74+
75+
test('recursive articles stay within product section (basePath guard)', async () => {
76+
const res = await get(makeURL('/en/get-started/article-grid-discovery'))
77+
expect(res.statusCode).toBe(200)
78+
79+
// All four articles from both categories should appear
80+
expect(res.body).toContain('[Grid Article One]')
81+
expect(res.body).toContain('[Grid Article Two]')
82+
expect(res.body).toContain('[Grid Article Three]')
83+
expect(res.body).toContain('[Grid Article Four]')
84+
85+
// Category pages themselves should NOT appear (excludeParents: true)
86+
expect(res.body).not.toContain('[Grid Category One]')
87+
expect(res.body).not.toContain('[Grid Category Two]')
88+
})
5489
})

src/article-api/transformers/discovery-landing-transformer.ts

Lines changed: 30 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -129,12 +129,17 @@ export class DiscoveryLandingTransformer implements PageTransformer {
129129
}
130130
}
131131

132-
// Articles section: recursively gather ALL descendant articles
133-
// This matches the behavior of the site which uses genericTocFlat/genericTocNested
132+
// Articles section: recursively gather descendant articles within
133+
// this product section. Uses Liquid-only rendering for titles and intros
134+
// instead of the full Markdown/unified pipeline, which would take 30s+
135+
// for large sections like /rest (297 descendants × 2 renderContent calls).
136+
// The basePath guard prevents cross-product recursion (e.g. /rest listing
137+
// /enterprise-admin children that point outside the /rest hierarchy).
134138
if (discoveryPage.children && discoveryPage.children.length > 0) {
135139
const tocItems = await getAllTocItems(page, context, {
136140
recurse: true,
137141
renderIntros: true,
142+
liquidOnly: true,
138143
})
139144

140145
// Flatten to get all leaf articles (excludeParents: true means only get articles, not category pages)
@@ -143,21 +148,31 @@ export class DiscoveryLandingTransformer implements PageTransformer {
143148
// Apply includedCategories filter if specified
144149
if (discoveryPage.includedCategories && discoveryPage.includedCategories.length > 0) {
145150
const includedCategories = discoveryPage.includedCategories.map((c) => c.toLowerCase())
146-
// Filter tocItems before flattening to preserve category info
147-
const filterByCategory = (items: typeof tocItems): typeof tocItems => {
148-
return items.filter((item) => {
149-
const itemCategories = (item.category || []).map((c: string) => c.toLowerCase())
150-
return itemCategories.some((cat) => includedCategories.includes(cat))
151-
})
151+
152+
// Build a map of href → category from the full tree
153+
const categoryMap = new Map<string, string[]>()
154+
interface TocNode {
155+
href: string
156+
category?: string[]
157+
childTocItems?: TocNode[]
158+
}
159+
function collectCategories(items: TocNode[]) {
160+
for (const item of items) {
161+
if (item.category && item.category.length > 0) {
162+
categoryMap.set(item.href, item.category)
163+
}
164+
if (item.childTocItems) collectCategories(item.childTocItems)
165+
}
152166
}
167+
collectCategories(tocItems)
153168

154-
// Re-flatten with category filtering
155-
const filteredTocItems = filterByCategory(flattenTocItemsWithCategory(tocItems))
156-
allArticles = filteredTocItems.map((item) => ({
157-
href: item.href,
158-
title: item.title,
159-
intro: item.intro,
160-
}))
169+
allArticles = allArticles.filter((item) => {
170+
const itemCategories = (categoryMap.get(item.href) || []).map((c) => c.toLowerCase())
171+
return (
172+
itemCategories.length === 0 ||
173+
itemCategories.some((cat) => includedCategories.includes(cat))
174+
)
175+
})
161176
}
162177

163178
if (allArticles.length > 0) {
@@ -178,36 +193,3 @@ export class DiscoveryLandingTransformer implements PageTransformer {
178193
}
179194
}
180195
}
181-
182-
/**
183-
* Helper to flatten TOC items while preserving category info for filtering
184-
*/
185-
interface TocItemWithCategory {
186-
href: string
187-
title: string
188-
intro?: string
189-
category?: string[]
190-
childTocItems?: TocItemWithCategory[]
191-
}
192-
193-
function flattenTocItemsWithCategory(tocItems: TocItemWithCategory[]): TocItemWithCategory[] {
194-
const result: TocItemWithCategory[] = []
195-
196-
function recurse(items: TocItemWithCategory[]) {
197-
for (const item of items) {
198-
const hasChildren = item.childTocItems && item.childTocItems.length > 0
199-
200-
// Only include leaf nodes (articles, not category pages)
201-
if (!hasChildren) {
202-
result.push(item)
203-
}
204-
205-
if (hasChildren) {
206-
recurse(item.childTocItems!)
207-
}
208-
}
209-
}
210-
211-
recurse(tocItems)
212-
return result
213-
}
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
title: Excluded Article
3+
intro: "This article has the Advanced category and should be excluded."
4+
versions:
5+
fpt: "*"
6+
ghes: "*"
7+
ghec: "*"
8+
category:
9+
- Advanced
10+
layout: inline
11+
---
12+
13+
This article should not appear when filtering by Getting started.
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
title: Included Article
3+
intro: "This article has the Getting started category and should be included."
4+
versions:
5+
fpt: "*"
6+
ghes: "*"
7+
ghec: "*"
8+
category:
9+
- Getting started
10+
layout: inline
11+
---
12+
13+
This article should appear when filtering by Getting started.
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
title: Filtered Category
3+
intro: "A test category for filtering."
4+
versions:
5+
fpt: "*"
6+
ghes: "*"
7+
ghec: "*"
8+
children:
9+
- /included-article
10+
- /excluded-article
11+
---
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
---
2+
title: Discovery Filtered
3+
intro: "A test page for discovery landing with includedCategories filtering."
4+
versions:
5+
fpt: "*"
6+
ghes: "*"
7+
ghec: "*"
8+
layout: discovery-landing
9+
includedCategories:
10+
- Getting started
11+
children:
12+
- /filtered-category
13+
---
14+
15+
This page tests includedCategories filtering on discovery landing pages.

src/fixtures/fixtures/content/get-started/index.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ children:
4747
- /article-grid-bespoke
4848
- /multi-carousel
4949
- /non-child-resolution
50+
- /discovery-filtered
5051
communityRedirect:
5152
name: Provide HubGit Feedback
5253
href: 'https://hubgit.com/orgs/community/discussions/categories/get-started'

0 commit comments

Comments
 (0)