From 5e8a70a010be0f0e1ef0df1ab658f20c72403837 Mon Sep 17 00:00:00 2001 From: Kam Date: Sat, 22 Aug 2026 17:01:01 +0300 Subject: [PATCH] fix(docs-infra): fail the build on duplicate heading anchors Two headings on the same page can generate the same anchor id, and every link to it then resolves to whichever comes first. Nothing caught this because the route manifest keeps anchors in a `Set`, so a repeat collapses into one entry before any check runs, and the existing link validation only asks whether an anchor exists, which a duplicate satisfies. The generator now inspects each page's headings while they are still an ordered list and fails with the offending pages and anchors. The scanning and the duplicate check move to `heading.mts` so they are covered by tests next to `getIdFromHeading`, which should keep this from coming back. --- adev/scripts/routes/generate-routes.mts | 43 ++++++------- adev/shared-docs/pipeline/shared/heading.mts | 48 ++++++++++++++ .../pipeline/shared/test/heading.spec.mts | 64 ++++++++++++++++++- 3 files changed, 132 insertions(+), 23 deletions(-) diff --git a/adev/scripts/routes/generate-routes.mts b/adev/scripts/routes/generate-routes.mts index f6cd639f971..c0d6f6e2aea 100644 --- a/adev/scripts/routes/generate-routes.mts +++ b/adev/scripts/routes/generate-routes.mts @@ -7,7 +7,7 @@ */ import {ALL_ITEMS} from '../../src/app/routing/navigation-entries/index.js'; -import {getIdFromHeading} from '../../shared-docs/pipeline/shared/heading.mjs'; +import {extractHeadingIds, findDuplicateIds} from '../../shared-docs/pipeline/shared/heading.mjs'; import {NavigationItem} from '@angular/docs'; import {writeFileSync, readFileSync} from 'fs'; import {join, resolve} from 'path'; @@ -20,7 +20,7 @@ const contentRoot = resolve(process.cwd(), '../../src/content'); * in a JSON file. This file then used by other bazel targets to know which routes are valid. */ -function extractRoutes(items: NavigationItem[]): string[] { +function extractRoutes(items: NavigationItem[], duplicatesByPage: Map): string[] { const routes: string[] = []; for (const item of items) { if (item.path && !item.path.startsWith('http')) { @@ -29,7 +29,11 @@ function extractRoutes(items: NavigationItem[]): string[] { const content = readFileSync(join(contentRoot, `${item.contentPath}.md`), { encoding: 'utf-8', }); - const headings = extractHeadings(content); + const headings = extractHeadingIds(content); + const duplicates = findDuplicateIds(headings); + if (duplicates.length > 0) { + duplicatesByPage.set(`${item.contentPath}.md`, duplicates); + } routes.push( ...headings.map( (heading) => `${item.path}#${heading.toLowerCase().replace(/\s+/g, '-')}`, @@ -38,33 +42,28 @@ function extractRoutes(items: NavigationItem[]): string[] { } } if (item.children) { - routes.push(...extractRoutes(item.children)); + routes.push(...extractRoutes(item.children, duplicatesByPage)); } } return routes; } -function extractHeadings(content: string): string[] { - const headings = content - .split('\n') - // Top level heading (H1) are used for the page title only - // and yes, headings can have leading spaces - .filter((line) => line.trim().startsWith('##')) - .map((line) => line.replace(/^#+\s*/, '').trim()); - - const stepRegex = /]*title="([^"]*)"/g; - let match; - while ((match = stepRegex.exec(content)) !== null) { - headings.push(match[1]); - } - - return headings.map((heading: string) => getIdFromHeading(heading)); -} - function main() { const allRoutes: string[] = []; + const duplicatesByPage = new Map(); - allRoutes.push(...extractRoutes(ALL_ITEMS)); + allRoutes.push(...extractRoutes(ALL_ITEMS, duplicatesByPage)); + + if (duplicatesByPage.size > 0) { + const details = Array.from(duplicatesByPage) + .map(([page, ids]) => ` ${page}: ${ids.map((id) => `#${id}`).join(', ')}`) + .join('\n'); + throw new Error( + `Headings must produce a unique anchor id within a page, otherwise every link to the ` + + `anchor resolves to the first heading that claims it. Give the later heading its own ` + + `id with the \`{#custom-id}\` syntax.\n${details}`, + ); + } const uniqueRoutes = Array.from(new Set(allRoutes.filter((r) => !!r))); diff --git a/adev/shared-docs/pipeline/shared/heading.mts b/adev/shared-docs/pipeline/shared/heading.mts index 9f051bca113..3dd5ee2cbd9 100644 --- a/adev/shared-docs/pipeline/shared/heading.mts +++ b/adev/shared-docs/pipeline/shared/heading.mts @@ -27,3 +27,51 @@ export function getIdFromHeading(heading: string): string { .replace(/\s|\//g, '-') // replace spaces and slashes with dashes .replace(/[^\p{L}\d\-]/gu, ''); // only keep letters, digits & dashes } + +/** + * Extracts the anchor ID of every heading on a page, in document order. + * Covers markdown headings below H1, which is reserved for the page title, + * and `` titles, which are rendered as headings too. Lines inside + * fenced code blocks are ignored so that samples cannot contribute headings. + */ +export function extractHeadingIds(content: string): string[] { + const headings: string[] = []; + let insideCodeBlock = false; + + for (const line of content.split('\n')) { + const trimmedLine = line.trim(); + + if (trimmedLine.startsWith('```')) { + insideCodeBlock = !insideCodeBlock; + continue; + } + + // Headings can have leading spaces. + if (!insideCodeBlock && trimmedLine.startsWith('##')) { + headings.push(trimmedLine.replace(/^#+\s*/, '').trim()); + } + } + + const stepRegex = /]*title="([^"]*)"/g; + let match; + while ((match = stepRegex.exec(content)) !== null) { + headings.push(match[1]); + } + + return headings.map((heading) => getIdFromHeading(heading)); +} + +/** Returns the IDs that appear more than once, each reported once. */ +export function findDuplicateIds(ids: string[]): string[] { + const seen = new Set(); + const duplicates = new Set(); + + for (const id of ids) { + if (seen.has(id)) { + duplicates.add(id); + } + seen.add(id); + } + + return Array.from(duplicates); +} diff --git a/adev/shared-docs/pipeline/shared/test/heading.spec.mts b/adev/shared-docs/pipeline/shared/test/heading.spec.mts index 15c952c67ea..d49da4d7fa6 100644 --- a/adev/shared-docs/pipeline/shared/test/heading.spec.mts +++ b/adev/shared-docs/pipeline/shared/test/heading.spec.mts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.dev/license */ -import {getIdFromHeading} from '../heading.mjs'; +import {extractHeadingIds, findDuplicateIds, getIdFromHeading} from '../heading.mjs'; describe('getIdFromHeading', () => { it('should generate id from simple text', () => { @@ -29,3 +29,65 @@ describe('getIdFromHeading', () => { expect(getIdFromHeading('Duplicate Heading {#unique-id}')).toBe('unique-id'); }); }); + +describe('extractHeadingIds', () => { + it('should collect ids for every heading below the page title', () => { + const content = [ + '# Page title', + '## First section', + '### A subsection', + '#### Deeper still', + ].join('\n'); + + expect(extractHeadingIds(content)).toEqual(['first-section', 'a-subsection', 'deeper-still']); + }); + + it('should collect ids for headings with leading spaces', () => { + expect(extractHeadingIds(' ## Indented heading')).toEqual(['indented-heading']); + }); + + it('should collect ids for docs-step titles', () => { + const content = [ + '## A section', + '', + '', + ].join('\n'); + + expect(extractHeadingIds(content)).toEqual(['a-section', 'install-the-package']); + }); + + it('should use the custom id when a heading declares one', () => { + expect(extractHeadingIds('## Before {#migration-before}')).toEqual(['migration-before']); + }); + + it('should ignore headings inside fenced code blocks', () => { + const content = [ + '## A real heading', + '```md', + '## Not a heading', + '### Also not a heading', + '```', + '## Another real heading', + ].join('\n'); + + expect(extractHeadingIds(content)).toEqual(['a-real-heading', 'another-real-heading']); + }); +}); + +describe('findDuplicateIds', () => { + it('should return an empty array when every id is unique', () => { + expect(findDuplicateIds(['one', 'two', 'three'])).toEqual([]); + }); + + it('should return an id that appears twice', () => { + expect(findDuplicateIds(['one', 'two', 'one'])).toEqual(['one']); + }); + + it('should report an id repeated many times only once', () => { + expect(findDuplicateIds(['one', 'one', 'one', 'one'])).toEqual(['one']); + }); + + it('should return every duplicated id', () => { + expect(findDuplicateIds(['before', 'after', 'before', 'after'])).toEqual(['before', 'after']); + }); +});