Skip to content

Commit 05461db

Browse files
heiskrCopilot
andauthored
Fix .md landing pages returning empty markdown (#60191)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent a445a89 commit 05461db

File tree

8 files changed

+103
-45
lines changed

8 files changed

+103
-45
lines changed

src/article-api/middleware/article-body.ts

Lines changed: 12 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -50,48 +50,23 @@ export async function getArticleBody(req: ExtendedRequestWithPageInfo) {
5050
// Extract apiVersion from query params if provided
5151
const apiVersion = req.query.apiVersion as string | undefined
5252

53-
// Check if there's a transformer for this page type (e.g., REST, webhooks, etc.)
53+
// With the catch-all ArticleTransformer registered last,
54+
// findTransformer always returns a transformer.
5455
const transformer = transformerRegistry.findTransformer(page)
56+
if (!transformer) throw new Error(`No transformer found for page: ${pathname}`)
5557

56-
if (transformer) {
57-
// Use the transformer for autogenerated pages
58-
const renderingReq = await createContextualizedRenderingRequest(pathname, page)
59-
60-
// Determine the API version to use (provided or latest)
61-
// Validation is handled by apiVersionValidationMiddleware
62-
const currentVersion = renderingReq.context.currentVersion
63-
let effectiveApiVersion = apiVersion
64-
65-
// Use latest version if not provided
66-
if (!effectiveApiVersion && currentVersion && allVersions[currentVersion]) {
67-
effectiveApiVersion = allVersions[currentVersion].latestApiVersion || undefined
68-
}
69-
70-
return await transformer.transform(page, pathname, renderingReq.context, effectiveApiVersion)
71-
}
72-
73-
// For regular articles (non-autogenerated)
74-
if (page.documentType !== 'article') {
75-
throw new Error(`Page ${pathname} isn't yet available in markdown.`)
76-
}
77-
78-
// these parts allow us to render the page
58+
// Use the transformer
7959
const renderingReq = await createContextualizedRenderingRequest(pathname, page)
80-
renderingReq.context.markdownRequested = true
81-
const content = await page.render(renderingReq.context)
8260

83-
// Get title and intro for consistency with transformer-based pages
84-
const title = page.title
85-
const intro = page.intro
86-
? await page.renderProp('intro', renderingReq.context, { textOnly: true })
87-
: ''
61+
// Determine the API version to use (provided or latest)
62+
// Validation is handled by apiVersionValidationMiddleware
63+
const currentVersion = renderingReq.context.currentVersion
64+
let effectiveApiVersion = apiVersion
8865

89-
// Prepend title and intro to the content
90-
let result = `# ${title}\n\n`
91-
if (intro) {
92-
result += `${intro}\n\n`
66+
// Use latest version if not provided
67+
if (!effectiveApiVersion && currentVersion && allVersions[currentVersion]) {
68+
effectiveApiVersion = allVersions[currentVersion].latestApiVersion || undefined
9369
}
94-
result += content
9570

96-
return result
71+
return await transformer.transform(page, pathname, renderingReq.context, effectiveApiVersion)
9772
}
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
import type { Context, Page } from '@/types'
2+
import type { PageTransformer } from './types'
3+
4+
/**
5+
* Transformer for regular articles.
6+
*
7+
* This is a catch-all transformer registered last in the registry.
8+
* It renders the page body as markdown and prepends the title and intro.
9+
*/
10+
export class ArticleTransformer implements PageTransformer {
11+
canTransform(page: Page): boolean {
12+
// Catch-all: handles any page not matched by a more specific transformer.
13+
return page != null
14+
}
15+
16+
async transform(page: Page, _pathname: string, context: Context): Promise<string> {
17+
const body = await page.render({ ...context, markdownRequested: true })
18+
const title = page.title
19+
const intro = page.intro ? await page.renderProp('intro', context, { textOnly: true }) : ''
20+
21+
let result = `# ${title}\n\n`
22+
if (intro) result += `${intro}\n\n`
23+
result += body
24+
return result
25+
}
26+
}

src/article-api/transformers/index.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import { DiscoveryLandingTransformer } from './discovery-landing-transformer'
1717
import { ProductGuidesTransformer } from './product-guides-transformer'
1818
import { ProductLandingTransformer } from './product-landing-transformer'
1919
import { SearchPageTransformer } from './search-page-transformer'
20+
import { ArticleTransformer } from './article-transformer'
2021

2122
/**
2223
* Global transformer registry
@@ -42,6 +43,8 @@ transformerRegistry.register(new DiscoveryLandingTransformer())
4243
transformerRegistry.register(new ProductGuidesTransformer())
4344
transformerRegistry.register(new ProductLandingTransformer())
4445
transformerRegistry.register(new SearchPageTransformer())
46+
// ArticleTransformer is the catch-all — must be registered last.
47+
transformerRegistry.register(new ArticleTransformer())
4548

4649
export { TransformerRegistry } from './types'
4750
export type { PageTransformer } from './types'

src/frame/middleware/context/context.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,10 @@ export default async function contextualize(
4343
// req.pagePath is used later in the rendering pipeline to
4444
// locate the file in the tree so it cannot have .md
4545
req.pagePath = req.pagePath.replace(/\/index\.md$/, '').replace(/\.md$/, '')
46+
req.context.markdownRequested = true
47+
// Track that markdown was requested via URL suffix, not Accept header.
48+
// This avoids adding a misleading Vary: accept cache header.
49+
req.context.markdownViaUrl = true
4650
}
4751

4852
// define each context property explicitly for code-search friendliness

src/frame/middleware/render-page.ts

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import FailBot from '@/observability/lib/failbot'
99
import statsd from '@/observability/lib/statsd'
1010
import type { ExtendedRequest } from '@/types'
1111
import { allVersions } from '@/versions/lib/all-versions'
12+
import { transformerRegistry } from '@/article-api/transformers'
1213
import { minimumNotFoundHtml } from '../lib/constants'
1314
import { contentTypeCacheControl, defaultCacheControl } from './cache-control'
1415
import { isConnectionDropped } from './halt-on-dropped-connection'
@@ -97,8 +98,20 @@ export default async function renderPage(req: ExtendedRequest, res: Response) {
9798
}
9899

99100
if (!req.context) throw new Error('request not contextualized')
100-
req.context.renderedPage = await buildRenderedPage(req)
101-
req.context.miniTocItems = buildMiniTocItems(req)
101+
102+
if (context.markdownRequested) {
103+
const transformer = transformerRegistry.findTransformer(page)
104+
if (!transformer) throw new Error(`No transformer found for page: ${req.pagePath}`)
105+
// Pass context without markdownRequested — transformers set it themselves
106+
// when rendering templates. Having it set during prepareTemplateData()
107+
// causes renderTitle/renderProp to output markdown instead of HTML,
108+
// which breaks the cheerio-based unwrap logic.
109+
const transformerContext = { ...context, markdownRequested: false }
110+
req.context.renderedPage = await transformer.transform(page, path, transformerContext)
111+
} else {
112+
req.context.renderedPage = await buildRenderedPage(req)
113+
req.context.miniTocItems = buildMiniTocItems(req)
114+
}
102115

103116
// Stop processing if the connection was already dropped
104117
if (isConnectionDropped(req, res)) return
@@ -151,7 +164,13 @@ export default async function renderPage(req: ExtendedRequest, res: Response) {
151164
}
152165

153166
if (context.markdownRequested) {
154-
contentTypeCacheControl(res)
167+
if (context.markdownViaUrl) {
168+
// .md URL suffix always returns markdown — Vary: accept would be misleading
169+
defaultCacheControl(res)
170+
} else {
171+
// Accept header determines the representation — Vary: accept is correct
172+
contentTypeCacheControl(res)
173+
}
155174
return res.type('text/markdown').send(req.context.renderedPage)
156175
}
157176

src/frame/tests/server.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -311,6 +311,40 @@ describe('server', () => {
311311
expect(res.statusCode).toBe(200)
312312
expect(res.headers['content-type']).toContain('text/html')
313313
})
314+
315+
test('landing page returns non-empty markdown with title via Accept header', async () => {
316+
const res = await get('/en/get-started', {
317+
headers: {
318+
accept: 'text/markdown',
319+
},
320+
})
321+
expect(res.statusCode).toBe(200)
322+
expect(res.headers['content-type']).toContain('text/markdown')
323+
expect(res.body).toMatch(/^# .+/)
324+
// Verify the landing page has content beyond just the title
325+
expect(res.body).toMatch(/\n\n/)
326+
expect(res.body.split('\n').length).toBeGreaterThan(3)
327+
})
328+
329+
test('.md URL extension returns markdown with correct content type', async () => {
330+
const res = await get('/en/get-started.md')
331+
expect(res.statusCode).toBe(200)
332+
expect(res.headers['content-type']).toContain('text/markdown')
333+
expect(res.body).toMatch(/^# .+/)
334+
})
335+
336+
test('/index.md redirects to the page without /index.md', async () => {
337+
const res = await get('/en/get-started/index.md')
338+
expect(res.statusCode).toBe(302)
339+
expect(res.headers.location).toBe('/en/get-started')
340+
})
341+
342+
test('regular article .md URL includes title and intro', async () => {
343+
const res = await get('/en/get-started/start-your-journey/hello-world.md')
344+
expect(res.statusCode).toBe(200)
345+
expect(res.headers['content-type']).toContain('text/markdown')
346+
expect(res.body).toMatch(/^# Hello World/)
347+
})
314348
})
315349
})
316350

src/shielding/middleware/handle-invalid-paths.ts

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -87,11 +87,7 @@ export default function handleInvalidPaths(
8787
defaultCacheControl(res)
8888
const newUrl = req.originalUrl.replace(req.path, req.path.replace(/\/index\.md$/, ''))
8989
return res.safeRedirect(newUrl)
90-
} else if (req.path.endsWith('.md')) {
91-
req.url = req.url.replace(/\.md($|\?)/, '$1')
92-
req.originalUrl = req.originalUrl.replace(/\.md($|\?)/, '$1')
93-
req.headers.accept = 'text/markdown'
94-
return next()
9590
}
91+
9692
return next()
9793
}

src/types/types.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,7 @@ export type Context = {
190190
renderedPage?: string
191191
miniTocItems?: MiniTocItem[]
192192
markdownRequested?: boolean
193+
markdownViaUrl?: boolean
193194
}
194195
export type LearningTracks = {
195196
[group: string]: {

0 commit comments

Comments
 (0)