Skip to content

Commit c3c641e

Browse files
heiskrCopilot
andauthored
Fix article API perf: refactor resolvePath to use findPage, remove liquidOnly (#60496)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent c5162a9 commit c3c641e

File tree

3 files changed

+47
-121
lines changed

3 files changed

+47
-121
lines changed

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

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

65
interface PageWithChildren extends Page {
@@ -29,16 +28,12 @@ export async function getAllTocItems(
2928
options: {
3029
recurse?: boolean
3130
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
3631
/** Only recurse into children whose resolved path starts with this prefix.
3732
* Prevents cross-product traversal (e.g. /en/rest listing /enterprise-admin). */
3833
basePath?: string
3934
} = {},
4035
): Promise<TocItem[]> {
41-
const { recurse = true, renderIntros = true, liquidOnly = false } = options
36+
const { recurse = true, renderIntros = true } = options
4237
const pageWithChildren = page as PageWithChildren
4338
const languageCode = page.languageCode || 'en'
4439

@@ -75,32 +70,11 @@ export async function getAllTocItems(
7570
)
7671
const href = childPermalink ? childPermalink.href : childHref
7772

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-
}
73+
const title = await childPage.renderTitle(context, { unwrap: true })
9174

9275
let intro = ''
9376
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-
}
77+
intro = await childPage.renderProp('intro', context, { textOnly: true })
10478
}
10579

10680
const category = childPage.category || []
Lines changed: 41 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -1,107 +1,62 @@
1+
import findPage from '@/frame/lib/find-page'
2+
import { allVersionKeys } from '@/versions/lib/all-versions'
13
import type { Context, Page } from '@/types'
24

35
/**
4-
* Resolves an href to a Page object from the context
6+
* Resolves an href to a Page object from the context.
57
*
6-
* This function handles various href formats:
7-
* - External URLs (http/https) - returns undefined
8-
* - Language-prefixed absolute paths (/en/copilot/...) - direct lookup
9-
* - Absolute paths without language (/copilot/...) - adds language prefix
10-
* - Relative paths (get-started) - resolved relative to pathname
11-
*
12-
* The function searches through context.pages using multiple strategies:
13-
* 1. Direct key lookup with language prefix
14-
* 2. Relative path joining with current pathname
15-
* 3. endsWith matching for versioned keys (e.g., /en/enterprise-cloud@latest/...)
16-
*
17-
* @param href - The href to resolve
18-
* @param languageCode - The language code (e.g., 'en')
19-
* @param pathname - The current page's pathname (e.g., '/en/copilot')
20-
* @param context - The rendering context containing all pages
21-
* @returns The resolved Page object, or undefined if not found
22-
*
23-
* @example
24-
* ```typescript
25-
* // Absolute path with language
26-
* resolvePath('/en/copilot/quickstart', 'en', '/en/copilot', context)
27-
*
28-
* // Absolute path without language (adds /en/)
29-
* resolvePath('/copilot/quickstart', 'en', '/en/copilot', context)
30-
*
31-
* // Relative path (resolves to /en/copilot/quickstart)
32-
* resolvePath('quickstart', 'en', '/en/copilot', context)
33-
*
34-
* // Relative path with leading slash (resolves relative to pathname)
35-
* resolvePath('/quickstart', 'en', '/en/copilot', context) // -> /en/copilot/quickstart
36-
* ```
8+
* Normalizes various href formats (relative, absolute, with/without language
9+
* prefix) to canonical paths, then delegates to findPage for lookup with
10+
* redirect support and English fallback.
3711
*/
3812
export function resolvePath(
3913
href: string,
4014
languageCode: string,
4115
pathname: string,
4216
context: Context,
4317
): Page | undefined {
44-
// External URLs cannot be resolved
45-
if (href.startsWith('http://') || href.startsWith('https://')) {
46-
return undefined
47-
}
48-
49-
if (!context.pages) {
50-
return undefined
51-
}
18+
if (href.startsWith('http://') || href.startsWith('https://')) return undefined
19+
if (!context.pages) return undefined
5220

53-
// Normalize href to start with /
54-
const normalizedHref = href.startsWith('/') ? href : `/${href}`
21+
const { pages, redirects } = context
5522

56-
// Build full path with language prefix if needed
57-
let fullPath: string
58-
if (normalizedHref.startsWith(`/${languageCode}/`)) {
59-
// Already has language prefix
60-
fullPath = normalizedHref
61-
} else if (href.startsWith('/') && !href.startsWith(`/${languageCode}/`)) {
62-
// Path with leading slash but no language prefix - treat as relative to pathname
63-
// e.g., pathname='/en/copilot', href='/get-started' -> '/en/copilot/get-started'
64-
fullPath = pathname + href
65-
} else {
66-
// Relative path - add language prefix
67-
// e.g., href='quickstart' -> '/en/quickstart'
68-
fullPath = `/${languageCode}${normalizedHref}`
23+
for (const candidate of candidates(href, languageCode, pathname)) {
24+
const page =
25+
findPage(candidate, pages, redirects) ||
26+
findPage(candidate.replace(/\/?$/, '/'), pages, redirects)
27+
if (page) return page
6928
}
7029

71-
// Clean up trailing slashes
72-
const cleanPath = fullPath.replace(/\/$/, '')
73-
74-
// Strategy 1: Direct lookup
75-
if (context.pages[cleanPath]) {
76-
return context.pages[cleanPath]
77-
}
78-
79-
// Strategy 2: Try relative to current pathname
80-
const currentPath = pathname.replace(/\/$/, '')
81-
const relativeHref = href.startsWith('/') ? href.slice(1) : href
82-
const joinedPath = `${currentPath}/${relativeHref}`
83-
84-
if (context.pages[joinedPath]) {
85-
return context.pages[joinedPath]
86-
}
30+
return undefined
31+
}
8732

88-
// Strategy 3: Search for keys that end with the path (handles versioned keys)
89-
// e.g., key='/en/enterprise-cloud@latest/copilot' should match path='/en/copilot'
90-
for (const [key, page] of Object.entries(context.pages)) {
91-
if (key.endsWith(cleanPath) || key.endsWith(`${cleanPath}/`)) {
92-
return page
93-
}
33+
// Lazily yields candidate paths in priority order, stopping at first match.
34+
function* candidates(href: string, lang: string, pathname: string) {
35+
const langPrefix = `/${lang}/`
36+
const cleanPathname = pathname.replace(/\/$/, '')
37+
38+
if (href.startsWith(langPrefix)) {
39+
// Already has language prefix — use as-is
40+
yield href
41+
} else if (href.startsWith('/')) {
42+
// Leading slash without lang prefix — try relative to pathname first,
43+
// then as a direct path with lang prefix
44+
yield `${cleanPathname}${href}`
45+
yield `${langPrefix.slice(0, -1)}${href}`
46+
} else {
47+
// Relative path — try relative to pathname, then with lang prefix
48+
yield `${cleanPathname}/${href}`
49+
yield `${langPrefix}${href}`
9450
}
9551

96-
// Strategy 4: If href started with /, try endsWith matching on that too
97-
if (href.startsWith('/')) {
98-
const hrefClean = href.replace(/\/$/, '')
99-
for (const [key, page] of Object.entries(context.pages)) {
100-
if (key.endsWith(hrefClean) || key.endsWith(`${hrefClean}/`)) {
101-
return page
102-
}
52+
// Versioned fallback: try inserting each version slug for
53+
// enterprise-only pages that don't exist on FPT.
54+
const suffix = href.startsWith(langPrefix)
55+
? href.slice(langPrefix.length).replace(/\/$/, '')
56+
: href.replace(/^\//, '').replace(/\/$/, '')
57+
if (suffix) {
58+
for (const version of allVersionKeys) {
59+
yield `${langPrefix}${version}/${suffix}`
10360
}
10461
}
105-
106-
return undefined
10762
}

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

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -130,16 +130,13 @@ export class DiscoveryLandingTransformer implements PageTransformer {
130130
}
131131

132132
// 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).
133+
// this product section. The basePath guard prevents cross-product
134+
// recursion (e.g. /rest listing /enterprise-admin children that
135+
// point outside the /rest hierarchy).
138136
if (discoveryPage.children && discoveryPage.children.length > 0) {
139137
const tocItems = await getAllTocItems(page, context, {
140138
recurse: true,
141139
renderIntros: true,
142-
liquidOnly: true,
143140
})
144141

145142
// Flatten to get all leaf articles (excludeParents: true means only get articles, not category pages)

0 commit comments

Comments
 (0)