and append the Copilot icon.
+// The prompt tag wraps content in code and appends Copilot links with responsive labels.
import octicons from '@primer/octicons'
import type { TagToken, TopLevelToken } from 'liquidjs'
@@ -32,9 +32,9 @@ export const Prompt: LiquidTag = {
const promptParam: string = encodeURIComponent(contentString)
const href: string = `https://github.com/copilot?prompt=${promptParam}`
- // Use murmur hash for deterministic ID (avoids hydration mismatch)
+ // Deterministic IDs prevent hydration mismatches.
const promptId: string = generatePromptId(contentString)
- // Show long text on larger screens and short text on smaller screens (set via accessibility.scss)
+ // accessibility.scss shows the long label on large screens and short label on small screens.
const promptLabelLong: string = 'Run this prompt in Copilot Chat'
const promptLabelShort: string = 'Run prompt'
return [
diff --git a/src/content-render/liquid/tool.ts b/src/content-render/liquid/tool.ts
index 922893032e37..47118cef29d7 100644
--- a/src/content-render/liquid/tool.ts
+++ b/src/content-render/liquid/tool.ts
@@ -3,53 +3,18 @@ import { allPlatforms } from '@/tools/lib/all-platforms'
export const tags: string[] = Object.keys(allTools).concat(allPlatforms).concat(['rowheaders'])
-// The trailing newline is important. Without it, the line immediately after
-// the `` will be considered part of the previous block, which means the Markdown following the `` will not be rendered to HTML correctly. For example:
-//
-// Here's some stuff
-// And *here* us also some stuff.
-//
-// Another **sentence** here.
-//
-// Will yield:
-//
-// Here's some stuff
-// And *here* us also some stuff.
-//
-// Another sentence here.
-//
-// when rendering this template with unified.
-// If you instead inject an extra newline after the ``, you
-// go from:
-//
-// Here's some stuff
-//
-// And *here* us also some stuff.
-//
-// Another **sentence** here.
-//
-// which yields:
-//
-// Here's some stuff
-//
-// And here us also some stuff.
-//
-// Another sentence here.
-//
-// The Tool Liquid tags are a little bit fragile because we hope and assume
-// that the author of the Liquid+Markdown *don't* do this:
-//
-// {% vscode %}Bla bla.{% endvscode %}Next stuff here...
-//
+// The trailing newline keeps Markdown after outside the HTML block so unified renders it.
+// Tool tags require content after the closing tag to start on a new line.
+// Example: \nText stays in the HTML block; \n\nText renders as Markdown.
const template = '{{ output }}\n'
export const Tool = {
type: 'block' as const,
tagName: '',
- // Liquid template objects don't have TypeScript definitions
+ // Liquid does not publish TypeScript definitions for template objects.
templates: [] as unknown[],
- // tagToken and remainTokens are Liquid internal types without TypeScript definitions
+ // Liquid internal types do not cover tagToken or remainTokens.
parse(tagToken: unknown, remainTokens: unknown) {
const token = tagToken as { name: string; getText: () => string }
this.tagName = token.name
@@ -58,7 +23,6 @@ export const Tool = {
const stream = this.liquid.parser.parseStream(remainTokens)
stream
.on(`tag:end${this.tagName}`, () => stream.stop())
- // tpl is a Liquid template object without TypeScript definitions
.on('template', (tpl: unknown) => this.templates.push(tpl))
.on('end', () => {
throw new Error(`tag ${token.getText()} not closed`)
@@ -66,7 +30,7 @@ export const Tool = {
stream.start()
},
- // scope is a Liquid scope object, Generator yields/returns Liquid template values - no TypeScript definitions available
+ // Liquid does not type scope or generator template values.
*render(scope: unknown): Generator {
const output = yield this.liquid.renderer.renderTemplates(this.templates, scope)
return yield this.liquid.parseAndRender(template, {
diff --git a/src/content-render/scripts/add-content-type.ts b/src/content-render/scripts/add-content-type.ts
index f6286ea7b4a2..0f5925e55768 100644
--- a/src/content-render/scripts/add-content-type.ts
+++ b/src/content-render/scripts/add-content-type.ts
@@ -1,7 +1,5 @@
-/**
- * @purpose Writer tool
- * @description Auto-populate the `contentType` frontmatter property based on the directory location of the content file
- */
+// @purpose Writer tool
+// @description Auto-populate the `contentType` frontmatter property based on the directory location of the content file
import fs from 'fs'
import path from 'path'
@@ -54,8 +52,7 @@ async function main() {
if (file.includes('early-access')) return false
if (!options.paths) return true
return options.paths.some((p: string) => {
- // Allow either a full content path like "content/foo/bar.md"
- // or a top-level directory name like "copilot"
+ // Accept full content paths like content/foo/bar.md or top-level dirs like copilot.
if (!p.startsWith('content')) {
p = path.join('content', p)
}
@@ -130,7 +127,7 @@ function processFile(filePath: string, scriptOptions: ScriptOptions) {
frontmatter.stringify(
content,
data,
- // lineWidth is a js-yaml option passed through gray-matter, not in gray-matter's type definitions
+ // gray-matter passes lineWidth to js-yaml, but its types omit it.
{ lineWidth: -1 } as unknown as Parameters[2],
),
)
@@ -144,38 +141,31 @@ function processFile(filePath: string, scriptOptions: ScriptOptions) {
}
function determineContentType(relativePath: string): string {
- // The split path array will be structured like:
- // [ 'copilot', 'how-tos', 'troubleshoot', 'index.md' ]
- // where the content type we want is in slot 1.
+ // For copilot/how-tos/troubleshoot/index.md, pathSegments[1] is the content type.
const pathSegments = relativePath.split(path.sep)
const topLevelDirectory = pathSegments[0]
const derivedContentType = pathSegments[1]
- // There is only one content/index.md, and it's the homepage.
+ // content/index.md is the only homepage.
if (topLevelDirectory === 'index.md') return 'homepage'
- // SPECIAL HANDLING FOR RAI
- // If a directory name includes a responsible-use string, assume the 'rai' type.
+ // Responsible-use directories map to the rai content type.
if (derivedContentType.includes(RESPONSIBLE_USE_STRING)) {
return RAI_TYPE
}
- // Allow 'getting-started' as an alternative directory name for 'get-started'.
+ // getting-started directories map to get-started.
if (derivedContentType === 'getting-started') {
return 'get-started'
}
- // When the content directory matches any of the allowed
- // content type values (such as 'get-started',
- // 'concepts', 'how-tos', 'reference', and 'tutorials'),
- // immediately return it. We're satisfied.
+ // Directories matching contentTypesEnum map to their content type.
if (contentTypesEnum.includes(derivedContentType)) {
return derivedContentType
}
- // There is only one content//index.md file per doc set.
- // This index.md is always a landing page.
+ // Product index.md files are landing pages.
if (derivedContentType === 'index.md') {
return LANDING_TYPE
}
diff --git a/src/content-render/scripts/all-documents/cli.ts b/src/content-render/scripts/all-documents/cli.ts
index 3e4893ef9793..5b304222c4f2 100644
--- a/src/content-render/scripts/all-documents/cli.ts
+++ b/src/content-render/scripts/all-documents/cli.ts
@@ -1,43 +1,14 @@
-/**
- * You specify one or more languages and versions, and this script
- * will output a JSON file with the metadata needed.
- * You run it with:
- *
- * npm run all-documents -- -o /tmp/all-documents.json
- *
- * By default, it will do free-pro-team, enterprise-cloud, and whatever
- * the latest enterprise-server is. You can specify versions with: --version
- * For example:
- *
- * npm run all-documents -- -v free-pro-team@latest -v ghes-3.12
- *
- * By default it will include all languages, but you can specify
- * with --language
- *
- * npm run all-documents -- -l en -l de
- *
- * For debugging purposes, because there are so *many* documents you can
- * apply a filter by URL matching, for example:
- *
- * npm run all-documents -- -f get-started/using-github
- *
- * This will only include documents whose URL contains the string
- * 'get-started/using-github'.
- *
- * If you don't specify an output file (the --output flag or -o for short),
- * it will print all the JSON to stdout.
- *
- * By default the fields set to include are: title, shortTitle, intro, url.
- * You can instead specify the fields you only want. For example
- *
- * npm run all-documents -- --field url --field title
- *
- * Now the JSON will look like this:
- *
- * ...
- * {"title": "Some title", "url": "/some-url"}
- * ...
- */
+// Generates JSON metadata for documents.
+// Run npm run all-documents -- -o /tmp/all-documents.json.
+// Defaults to all languages, free-pro-team, enterprise-cloud, latest enterprise-server,
+// fields title, shortTitle, intro, and url, and output file all-documents.json.
+// Use --version for versions such as free-pro-team@latest and ghes-3.12.
+// Use --language for languages such as en and de.
+// Use --filter to include only documents whose URL contains the given string.
+// Use --field to choose output fields, such as url and title.
+// Filter example: npm run all-documents -- -f get-started/using-github.
+// Field example: npm run all-documents -- --field url --field title.
+// Example field output: {"title":"Some title","url":"/some-url"}.
import { writeFileSync, statSync } from 'fs'
@@ -47,7 +18,7 @@ import { languageKeys } from '@/languages/lib/languages-server'
import { allVersions } from '@/versions/lib/all-versions'
import { allDocuments, POSSIBLE_FIELDS, type AllDocument } from './lib'
-// E.g. enteprise-server@3.12, free-pro-team@latest, etc
+// Version flags accept enterprise-server@3.12 and free-pro-team@latest.
const fullVersions = Object.keys(allVersions)
const defaultVersions: string[] = []
const shortAlias = new Map()
diff --git a/src/content-render/scripts/cta-builder.ts b/src/content-render/scripts/cta-builder.ts
index 96ca90b65f00..3cd26ab9597b 100644
--- a/src/content-render/scripts/cta-builder.ts
+++ b/src/content-render/scripts/cta-builder.ts
@@ -1,7 +1,5 @@
-/**
- * @purpose Writer tool
- * @description Create a properly formatted Call-to-Action URL with tracking parameters
- */
+// @purpose Writer tool
+// @description Create a properly formatted Call-to-Action URL with tracking parameters
import { Command } from 'commander'
import readline from 'readline'
import chalk from 'chalk'
@@ -92,7 +90,7 @@ program.action(() => {
interactiveBuilder()
})
-// Only run CLI when script is executed directly, not when imported
+// Avoid parsing CLI arguments when tests import this module.
if (import.meta.url === `file://${process.argv[1]}`) {
program.parse()
}
@@ -106,7 +104,7 @@ async function selectFromOptions(
console.log(chalk.yellow(`\n${message} (${paramName}):`))
for (let index = 0; index < options.length; index++) {
const option = options[index]
- const letter = String.fromCharCode(97 + index) // 97 is 'a' in ASCII
+ const letter = String.fromCharCode(97 + index) // 97 is the ASCII code for a.
console.log(chalk.white(` ${letter}. ${option}`))
}
@@ -115,7 +113,7 @@ async function selectFromOptions(
const answer = await promptFn('Enter the letter of your choice: ')
if (!answer) continue
- const letterIndex = answer.toLowerCase().charCodeAt(0) - 97 // Convert letter to index
+ const letterIndex = answer.toLowerCase().charCodeAt(0) - 97
if (letterIndex >= 0 && letterIndex < options.length && answer.length === 1) {
return options[letterIndex]
@@ -124,7 +122,7 @@ async function selectFromOptions(
const validLetters = options.map((_, index) => String.fromCharCode(97 + index)).join(', ')
console.log(chalk.red(`Invalid choice. Please enter one of: ${validLetters}`))
- // Safety: prevent infinite loops in automated scenarios
+ // Cap invalid answers for automated runs; empty answers reprompt without counting.
if (++attempts > 50) {
throw new Error('Too many invalid attempts. Please restart the tool.')
}
@@ -145,7 +143,7 @@ async function confirmChoice(
if (lower === 'n' || lower === 'no') return false
console.log(chalk.red('Please enter y or n'))
- // Safety: prevent infinite loops in automated scenarios
+ // Cap invalid answers for automated runs; empty answers reprompt without counting.
if (++attempts > 50) {
throw new Error('Too many invalid attempts. Please restart the tool.')
}
@@ -176,7 +174,6 @@ interface AjvError {
params: AjvErrorParams
}
-// Process AJV validation errors into readable messages
function formatValidationErrors(ctaParams: CTAParams, errors: AjvError[]): string[] {
const errorMessages: string[] = []
for (const error of errors) {
@@ -198,7 +195,6 @@ function formatValidationErrors(ctaParams: CTAParams, errors: AjvError[]): strin
return errorMessages
}
-// Full validation using AJV schema (consistent across all commands)
function validateCTAParams(params: CTAParams): { isValid: boolean; errors: string[] } {
const isValid = validateCTASchema(params)
const ajvErrors = validateCTASchema.errors || []
@@ -234,7 +230,7 @@ export function convertOldCTAUrl(oldUrl: string): { newUrl: string; notes: strin
const newParams: CTAParams = {}
- // Preserve any new-style params that are already on the URL.
+ // Keep CTA params that already pass the schema.
for (const [key, value] of url.searchParams.entries()) {
for (const param of Object.keys(ctaSchema.properties)) {
if (key === param && key in ctaSchema.properties) {
@@ -277,7 +273,7 @@ export function convertOldCTAUrl(oldUrl: string): { newUrl: string; notes: strin
}
}
- // Build new URL - preserve all existing parameters except old ref_ parameters
+ // Keep existing query parameters except ref_cta, ref_loc, and ref_page.
const newUrl = new URL(url.toString())
newUrl.searchParams.delete('ref_cta')
@@ -290,15 +286,12 @@ export function convertOldCTAUrl(oldUrl: string): { newUrl: string; notes: strin
}
}
- // The URL constructor may add a slash before the question mark in
- // "github.com?foo", but we don't want that. First, check if original
- // URL had trailing slash before query params.
+ // URL serializes github.com?foo as github.com/?foo; preserve the original slash shape.
const urlBeforeQuery = oldUrl.split('?')[0]
const hadTrailingSlash = urlBeforeQuery.endsWith('/')
let finalUrl = newUrl.toString()
- // Remove unwanted trailing slash if original didn't have one.
if (!hadTrailingSlash && finalUrl.includes('/?')) {
finalUrl = finalUrl.replace('/?', '?')
}
@@ -321,19 +314,19 @@ function inferProductFromUrl(url: string, refCta: string): string {
try {
hostname = new URL(url).hostname.toLowerCase()
} catch {
- // Fallback if url isn't valid: leave hostname empty
+ // Invalid URLs fall back to ref_cta or the default product.
}
if (hostname === 'desktop.github.com' || refCta.includes('desktop')) {
return 'desktop'
}
- // Hostname contains 'copilot' (e.g., copilot.github.com), or refCta mentions copilot
+ // GitHub subdomains containing copilot and ref_cta values containing copilot map to copilot.
if (
(hostname.includes('copilot') && hostname.endsWith('.github.com')) ||
refCta.toLowerCase().includes('copilot')
) {
return 'copilot'
}
- // Hostname contains 'enterprise' (e.g. enterprise.github.com), or refCta mentions GHEC
+ // GitHub subdomains containing enterprise and ref_cta values containing GHEC map to ghec.
if (
(hostname.includes('enterprise') && hostname.endsWith('.github.com')) ||
refCta.includes('GHEC')
@@ -344,8 +337,7 @@ function inferProductFromUrl(url: string, refCta: string): string {
}
function inferStyleFromContext(refLoc: string): string {
- // If location suggests it's in a button context, return button
- // Otherwise default to text for inline links
+ // Button-like ref_loc values map to button; everything else defaults to text.
const isButton = buttonKeywords.some((keyword) => refLoc.toLowerCase().includes(keyword))
return isButton ? 'button' : 'text'
}
@@ -393,7 +385,6 @@ async function interactiveBuilder(): Promise {
)
}
- // Optional parameters (properties not in required array)
console.log(chalk.white(`\nOptional parameters:\n`))
const allProperties = Object.keys(ctaSchema.properties)
@@ -458,7 +449,6 @@ async function convertUrls(options: { url?: string; quiet?: boolean }): Promise<
const result = convertOldCTAUrl(options.url)
if (options.quiet) {
- // In quiet mode, only output the new URL
console.log(result.newUrl)
return
}
@@ -469,7 +459,6 @@ async function convertUrls(options: { url?: string; quiet?: boolean }): Promise<
console.log(chalk.white('\nNew URL:'))
console.log(chalk.cyan(result.newUrl))
- // Validate the converted URL using shared validation function
try {
const newParams = extractCTAParams(result.newUrl)
const validation = validateCTAParams(newParams)
@@ -507,7 +496,7 @@ async function convertUrls(options: { url?: string; quiet?: boolean }): Promise<
}
}
- // The convert command doesn't use readline, so script should exit naturally
+ // The convert command opens no readline handle, so Node exits after logging.
}
async function validateUrl(options: { url?: string }): Promise {
@@ -531,7 +520,6 @@ async function validateUrl(options: { url?: string }): Promise {
return
}
- // Validate against schema using shared validation function
const validation = validateCTAParams(ctaParams)
if (validation.isValid) {
@@ -595,7 +583,6 @@ async function buildProgrammaticCTA(options: {
const validation = validateCTAParams(params)
if (!validation.isValid) {
- // Output validation errors to stderr and exit with error code
for (const error of validation.errors) {
console.error(`Validation error: ${error}`)
}
diff --git a/src/content-render/scripts/liquid-tags.ts b/src/content-render/scripts/liquid-tags.ts
index e24fdf6cfc73..5f9aaac84914 100644
--- a/src/content-render/scripts/liquid-tags.ts
+++ b/src/content-render/scripts/liquid-tags.ts
@@ -1,7 +1,5 @@
-/*
- * @purpose Writer tool
- * @description Expand and restore Liquid data references in content files
- */
+// @purpose Writer tool
+// @description Expand and restore Liquid data references in content files
// Usage: npm run liquid-tags -- expand --paths content/pull-requests/about.md
// Usage: npm run liquid-tags -- restore --paths content/pull-requests/about.md
@@ -38,23 +36,20 @@ function getErrorMessage(error: unknown): string {
return error instanceof Error ? error.message : String(error)
}
-// Regex pattern to match expanded content blocks
const EXPANDED_PATTERN = /(.+?)/gs
-// Validates and normalizes the incoming dataPath to prevent path traversal
-// and ensure the final resolved path remains within the expected root.
+// Reject absolute, traversal, empty, and unsafe data paths before resolving under data root.
function getDataFilePath(type: 'reusable' | 'variable', dataPath: string): string {
if (path.isAbsolute(dataPath)) {
throw new Error(`Invalid ${type} data path: absolute paths are not allowed: ${dataPath}`)
}
- // Disallow path traversal and empty segments
const segments = dataPath.split(/[\\/]/)
if (segments.some((segment) => segment === '..' || segment === '')) {
throw new Error(`Invalid ${type} data path: contains disallowed segments: ${dataPath}`)
}
- // Restrict allowed characters to a conservative safe set
+ // Restrict data paths to filename characters used by reusables and variables.
if (!/^[A-Za-z0-9_.\-/]+$/.test(dataPath)) {
throw new Error(`Invalid ${type} data path: contains disallowed characters: ${dataPath}`)
}
@@ -147,11 +142,11 @@ function getAllowedTypes(options: ExpandOptions): Array<'reusable' | 'variable'>
async function expandReferences(options: ExpandOptions): Promise {
const { paths, verbose, markers, shallow } = options
- // markers will be true by default, false when --no-markers is used
+ // --no-markers sets markers to false; missing flag leaves it true.
const withMarkers = markers !== false
- const recursive = !shallow // Recursive by default unless --shallow is specified
+ const recursive = !shallow // Omitting --shallow enables recursive expansion.
const allowedTypes = getAllowedTypes(options)
- const maxIterations = 10 // Safety limit for recursive expansion
+ const maxIterations = 10 // Stop recursive expansion after 10 passes to avoid circular references.
if (paths.length === 0) {
console.error(chalk.red('Error: No paths provided. Use --paths option.'))
@@ -204,7 +199,6 @@ async function expandReferences(options: ExpandOptions): Promise {
hasRemainingRefs = remainingRefs.length > 0
if (shallow) {
- // Shallow mode: show remaining references and break
if (hasRemainingRefs) {
console.log(
chalk.yellow(
@@ -296,10 +290,10 @@ async function restoreReferences(options: ExpandOptions): Promise {
console.log(chalk.dim(' Use --verbose to see details of the edits'))
}
- // Update data files with the edited content before restoring
+ // Write edited expanded blocks back to data files before restoring Liquid tags.
const updatedDataFiles = updateDataFiles(filePath, verbose, false, allowedTypes)
- // Automatically restore any updated data files back to liquid tags
+ // Restore updated data files so nested references return to Liquid tags too.
if (updatedDataFiles.length > 0) {
if (verbose)
console.log(chalk.blue(' Restoring updated data files back to liquid tags...'))
@@ -324,7 +318,7 @@ async function restoreReferences(options: ExpandOptions): Promise {
}
}
- // Always restore the main file content regardless of edits
+ // Restore the main file even when no data file changed.
const restoredContent = restoreFileContent(content, verbose, allowedTypes)
if (restoredContent !== content) {
@@ -414,12 +408,10 @@ async function detectContentEdits(
if (!allowedTypes || allowedTypes.includes(refType)) {
try {
- // Load the original content from data files
const originalContent = loadDataValue(refType, dataPath.trim())
if (originalContent !== null) {
- // Compare against the original content directly, not re-resolved
- // This avoids nested resolution issues that cause false positives
+ // Compare direct data file content to avoid false positives from nested resolution.
const currentContent = resolvedContent.trim()
if (currentContent !== originalContent.trim()) {
@@ -458,7 +450,7 @@ function loadDataValue(type: 'reusable' | 'variable', dataPath: string): string
if (type === 'reusable') {
const content = fs.readFileSync(targetPath, 'utf8')
- // Remove any frontmatter if present (same as resolveReusable)
+ // Strip reusable frontmatter before comparing content, matching resolveReusable.
const contentWithoutFrontmatter = content.replace(/^---[\s\S]*?---\s*/, '')
return contentWithoutFrontmatter.trim()
} else {
@@ -478,7 +470,7 @@ function loadDataValue(type: 'reusable' | 'variable', dataPath: string): string
return typeof current === 'string' ? current.trim() : String(current).trim()
}
} catch {
- // Silently return null for any errors
+ // Unreadable data returns null so callers can treat it as unverifiable.
}
return null
}
@@ -561,7 +553,7 @@ function extractDataUpdates(
const refType = type as 'reusable' | 'variable'
if (!allowedTypes || allowedTypes.includes(refType)) {
- // Check if this content was actually changed before including it
+ // Compare expanded blocks with their source before updating data files.
try {
const originalContent = loadDataValue(refType, dataPath.trim())
if (originalContent !== null && resolvedContent.trim() !== originalContent.trim()) {
@@ -572,7 +564,7 @@ function extractDataUpdates(
})
}
} catch {
- // If we can't verify, assume it was changed to be safe
+ // Keep blocks on unexpected errors; unreadable files return null from loadDataValue.
updates.push({
type: refType,
path: dataPath.trim(),
@@ -619,19 +611,18 @@ function applyDataUpdates(
} else {
console.log(chalk.green(` Updated: ${targetPath}`))
}
- return targetPath // Return path even in dry run
+ return targetPath // Dry runs return the target path so callers can report it.
}
try {
if (type === 'reusable') {
- // For reusables, replace entire file content
if (contents.length > 1) {
console.log(
chalk.yellow(` Warning: Multiple content blocks found for ${dataPath}, using first one`),
)
}
- // Preserve original file's newline behavior
+ // Preserve a trailing newline from the original reusable file.
const originalContent = fs.readFileSync(targetPath, 'utf8')
const hasTrailingNewline = originalContent.endsWith('\n')
const newContent =
@@ -642,12 +633,11 @@ function applyDataUpdates(
console.log(chalk.green(` Updated: ${type}s.${dataPath}`))
}
} else {
- // For variables, update YAML structure
const yamlContent = fs.readFileSync(targetPath, 'utf8')
const data = load(yamlContent) as Record
const pathParts = dataPath.split('.')
- const propertyPath = pathParts.slice(1) // Skip the file name
+ const propertyPath = pathParts.slice(1)
let current: Record = data
for (let i = 0; i < propertyPath.length - 1; i++) {
@@ -665,7 +655,7 @@ function applyDataUpdates(
}
current[finalKey] = contents[0]
- // Preserve original file's newline behavior for YAML
+ // Preserve a trailing newline from the original YAML file.
const hasTrailingNewline = yamlContent.endsWith('\n')
const yamlOutput = dump(data)
const finalYaml =
@@ -692,13 +682,13 @@ function findLiquidReferences(
const references: LiquidReference[] = []
const types = allowedTypes || ['reusable', 'variable']
- // Pattern to match {% data reusables.path %} and {% data variables.path %}
+ // Match data references for reusables and variables.
const liquidPattern = /{%\s*data\s+(reusables|variables)\.([^%]+)\s*%}/g
let match
while ((match = liquidPattern.exec(content)) !== null) {
const [original, type, dataPath] = match
- const refType = type.slice(0, -1) as 'reusable' | 'variable' // Remove 's' from end
+ const refType = type.slice(0, -1) as 'reusable' | 'variable'
if (types.includes(refType)) {
references.push({
@@ -745,7 +735,7 @@ async function resolveReusable(reusablePath: string, verbose?: boolean): Promise
try {
const content = fs.readFileSync(filePath, 'utf-8')
- // Remove any frontmatter if present
+ // Strip reusable frontmatter before inserting its body.
const contentWithoutFrontmatter = content.replace(/^---[\s\S]*?---\s*/, '')
return contentWithoutFrontmatter.trim()
} catch (error: unknown) {
@@ -781,8 +771,8 @@ async function resolveVariable(variablePath: string, verbose?: boolean): Promise
const yamlContent = fs.readFileSync(filePath, 'utf-8')
const data = load(yamlContent) as Record
- // Navigate through the key path to find the value
- const [, ...keyPath] = pathParts // Skip filename, get remaining path
+ // Variable paths start with the file name; remaining segments address YAML keys.
+ const [, ...keyPath] = pathParts
let value: unknown = data
for (const key of keyPath) {
if (value && typeof value === 'object' && key in value) {
diff --git a/src/content-render/scripts/move-by-content-type.ts b/src/content-render/scripts/move-by-content-type.ts
index e7773d92d881..1b4d395e5078 100644
--- a/src/content-render/scripts/move-by-content-type.ts
+++ b/src/content-render/scripts/move-by-content-type.ts
@@ -1,7 +1,5 @@
-/**
- * @purpose Writer tool
- * @description Move files to the relevant directory based on `contentType` frontmatter
- */
+// @purpose Writer tool
+// @description Move files to the relevant directory based on `contentType` frontmatter
import { program } from 'commander'
import fs from 'fs/promises'
@@ -16,8 +14,7 @@ const CONTENT_TYPES = contentTypesEnum.filter(
(type) => type !== 'homepage' && type !== 'other' && type !== 'landing',
)
-// The number of path segments at the product level (e.g., "content//...").
-// Used when determining whether a target directory is a deeper subdirectory.
+// Three segments identify content//index.md and top-level content-type directories.
const PRODUCT_LEVEL_PATH_SEGMENTS = 3
const contentTypeToDir = (contentType: string): string => {
@@ -31,10 +28,10 @@ function shouldSkipIndexFile(filePath: string): boolean {
const parts = relativePath.split(path.sep)
const contentIndex = parts.indexOf('content')
- // Skip product-level index.md: content/product/index.md
+ // Keep product-level index.md files in place.
if (parts.length === contentIndex + PRODUCT_LEVEL_PATH_SEGMENTS) return true
- // Skip content-type-level index.md that's already in place: content/product/content-type/index.md
+ // Keep content-type index.md files that already sit at content/product/content-type/index.md.
if (parts.length === contentIndex + 4) {
const parentDir = parts[parts.length - 2]
if (validContentTypeDirs.has(parentDir)) return true
@@ -52,18 +49,16 @@ function calculateTarget(filePath: string, contentType: string, productDir: stri
const targetContentType = contentTypeToDir(contentType)
if (targetContentType === 'how-tos') {
- // Preserve subdirectory structure for how-tos
+ // How-to pages keep their product subdirectory structure.
const pathAfterProduct = parts.slice(contentIndex + 2, -1)
if (pathAfterProduct[0] === 'how-tos') {
- // Already in how-tos, no change
return { targetDir: path.dirname(filePath), targetPath: filePath }
} else {
- // Move to how-tos preserving structure
const targetDir = path.join(productDir, targetContentType, ...pathAfterProduct)
return { targetDir, targetPath: path.join(targetDir, fileName) }
}
} else {
- // Flatten to content-type directory
+ // Other content types flatten into their content-type directory.
const targetDir = path.join(productDir, targetContentType)
return { targetDir, targetPath: path.join(targetDir, fileName) }
}
@@ -81,7 +76,6 @@ program
.description('Reorganize content files into subdirectories based on their contentType property')
.argument('[paths...]', 'Content paths to process')
.action(async (paths: string[]) => {
- // Gather files.
const filesToProcess: string[] = []
if (paths?.length > 0) {
for (const p of paths) {
@@ -102,8 +96,8 @@ program
const filesToMove: FileMove[] = []
const skipped: Array<{ file: string; reason: string }> = []
- const targetDirs = new Set() // Relative paths of all target directories
- const subdirTargets = new Set() // Subdirectories receiving index.md files
+ const targetDirs = new Set()
+ const subdirTargets = new Set()
const productDirs = new Set()
const productsWithRai = new Set()
@@ -111,7 +105,6 @@ program
const relativePath = path.relative(process.cwd(), filePath)
try {
- // Skip certain index.md files
if (path.basename(filePath) === 'index.md' && shouldSkipIndexFile(filePath)) {
continue
}
@@ -129,7 +122,7 @@ program
const parts = relativePath.split(path.sep)
const contentIndex = parts.indexOf('content')
- // Skip all landing pages - they should only be product-level index.md and don't move
+ // Landing pages belong at product-level index.md files; this script does not move them.
if (contentType === 'landing') {
console.log(chalk.gray(`→ Skipping ${relativePath}: landing pages don't move`))
continue
@@ -166,7 +159,7 @@ program
console.log(chalk.yellow(`⚠ Skipping ${relativePath}: Target file already exists`))
continue
} catch {
- // Good, doesn't exist
+ // Missing target means the move can proceed.
}
filesToMove.push({ filePath, targetDir, targetPath, contentType })
@@ -174,7 +167,6 @@ program
const relativeTargetDir = path.relative(process.cwd(), targetDir)
targetDirs.add(relativeTargetDir)
- // Track subdirectories that will receive index.md files
if (
path.basename(filePath) === 'index.md' &&
relativeTargetDir.split(path.sep).length > PRODUCT_LEVEL_PATH_SEGMENTS
@@ -195,7 +187,6 @@ program
console.log(chalk.white('Ensuring standard content-type directories exist...\n'))
- // Add standard content-type directories for each affected product
if (paths?.length > 0) {
for (const p of paths) {
const fullPath = path.resolve(process.cwd(), p)
@@ -237,10 +228,10 @@ program
await fs.access(indexPath)
console.log(chalk.gray(`- Skipping ${dirPath}/index.md (already exists)`))
} catch {
- // Only create placeholders for top-level content-type directories (not subdirectories)
+ // Create placeholders only for top-level content-type directories.
if (dirPath.split(path.sep).length > PRODUCT_LEVEL_PATH_SEGMENTS) continue
- // Skip if an index.md will be moved here
+ // Moved index.md files become the placeholder for their target directory.
if (subdirTargets.has(dirPath)) {
console.log(chalk.gray(`- Skipping ${dirPath}/index.md (will be moved)`))
continue
@@ -249,8 +240,6 @@ program
const contentTypeName = path.basename(dirPath)
const title = titleMap[contentTypeName] || contentTypeName
- // Determine the correct contentType for this placeholder
- // Map directory name back to contentType enum value
const placeholderContentType =
contentTypeName === 'responsible-use' ? 'rai' : contentTypeName
@@ -316,7 +305,7 @@ contentType: ${placeholderContentType}
const moved: Array<{ file: string; from: string; to: string }> = []
- // Categorize files by type for correct move order
+ // Move regular files and index.md files in separate groups to avoid path conflicts.
const regularFiles = filesToMove.filter((f) => path.basename(f.filePath) !== 'index.md')
const topLevelIndexFiles = filesToMove.filter((f) => {
if (path.basename(f.filePath) !== 'index.md') return false
@@ -333,7 +322,7 @@ contentType: ${placeholderContentType}
)
})
- // Move subdirectory index files first (copy only, delete later)
+ // Copy subdirectory index.md files first; delete sources after regular files move.
const indexFilesToDeleteLater: string[] = []
for (const file of subdirIndexFiles) {
try {
@@ -341,7 +330,7 @@ contentType: ${placeholderContentType}
const content = await fs.readFile(file.filePath, 'utf-8')
const { data, content: body } = readFrontmatter(content)
- // Clear children array because paths will be invalid in the new content-type directory structure
+ // Clear children because the new content-type directory structure invalidates child paths.
if (data?.children) data.children = []
await fs.writeFile(
@@ -526,7 +515,7 @@ contentType: ${placeholderContentType}
if (!data) continue
- // For how-tos, build children from subdirectories
+ // how-tos children point to subdirectories.
if (path.basename(dirPath) === 'how-tos') {
const entries = await fs.readdir(absoluteDirPath, { withFileTypes: true })
const subdirs = entries
@@ -544,7 +533,7 @@ contentType: ${placeholderContentType}
)
}
}
- // For others, sort with about-* first
+ // Other content types sort about-* pages first.
else if (data.children && Array.isArray(data.children) && data.children.length > 0) {
const sorted = [...data.children].sort((a, b) => {
const aBasename = path.basename(a)
diff --git a/src/content-render/scripts/move-content.ts b/src/content-render/scripts/move-content.ts
index d0f02a9e0f14..7c4b35603053 100755
--- a/src/content-render/scripts/move-content.ts
+++ b/src/content-render/scripts/move-content.ts
@@ -1,25 +1,13 @@
-/**
- * @purpose Writer tool
- * @description Move or rename a file or a folder and automatically add redirects
- */
-// [start-readme]
-//
-// Use this script to help you move or rename a single file or a folder. The script will move or rename the file or folder for you, update relevant `children` in the index.md file(s), and add a `redirect_from` to frontmatter in the renamed file(s). Note: You will still need to manually update the `title` if necessary.
-//
-// By default, the `move-content.ts` script will commit the changes it makes. If you don't want the script to run any git commands for you, run it with the `--no-git` flag. Note: In most cases it will be easier and safer to let the script run the git commands for you, since git can get confused when a file is both renamed and edited.
-//
-// To learn more about the script, you can run `npm run move-content --help`.
-//
-// To run the script for a file:
-// - `npm run move-content PATH/TO/CURRENT-FILE.md PATH/TO/DESIRED-FILE-LOCATION-OR-NAME.md`
-//
-// To run the script for a folder:
-// - `npm run move-content PATH/TO/CURRENT-FOLDER PATH/TO/DESIRED-FOLDER-LOCATION-OR-NAME`
-//
-// To undo the script, run the same command that you used to run the script, but add an `--undo` flag:
-// - `npm run move-content --undo PATH/TO/OLD PATH/TO/NEW`
-//
-// [end-readme]
+// @purpose Writer tool
+// @description Move or rename a file or a folder and automatically add redirects
+// Moves one file or folder, updates relevant children entries, and adds redirect_from.
+// It does not update title frontmatter.
+// By default, it runs git mv and git commit; pass --no-git to avoid git commands.
+// Keeping git enabled records rename and edit commits separately.
+// Run npm run move-content --help for options.
+// Run file: npm run move-content PATH/TO/CURRENT-FILE.md PATH/TO/DESIRED-FILE-LOCATION-OR-NAME.md.
+// Run folder: npm run move-content PATH/TO/CURRENT-FOLDER PATH/TO/DESIRED-FOLDER-LOCATION-OR-NAME.
+// Undo: npm run move-content --undo PATH/TO/OLD PATH/TO/NEW.
import fs from 'fs'
import path from 'path'
@@ -45,7 +33,7 @@ interface PositionInfo {
childGroupPositions: number[][]
}
-// This is so you can optionally run it again the test fixtures root.
+// ROOT lets tests run against a fixture content root.
const ROOT = process.env.ROOT || '.'
const CONTENT_ROOT = path.resolve(path.join(ROOT, 'content'))
@@ -99,7 +87,6 @@ async function main(opts: MoveOptions, nameTuple: string[]) {
newPath = new_
}
- // The file you're about to move needs to exist
if (!fs.existsSync(oldPath)) {
console.error(chalk.red(`${oldPath} does not exist.`))
process.exit(1)
@@ -107,20 +94,11 @@ async function main(opts: MoveOptions, nameTuple: string[]) {
let isFolder = fs.lstatSync(oldPath).isDirectory()
- // Before validating, see if we need to fake that the newPath should be.
- // This is to mimic how bash `mv` works where you can do:
- //
- // mv some/place/a/file.txt destin/ation/
- //
- // which is implied to mean the same as;
- //
- // mv some/place/a/file.txt destin/ation/file.txt
- //
+ // Emulate mv: moving path/file.md to an existing path/dir resolves to path/dir/file.md.
if (undo) {
if (isFolder) {
const wouldBe = path.join(oldPath, path.basename(newPath))
- // We can't know if the `newPath` is a directory or file because
- // whichever it is, it doesn't exist.
+ // For undo, infer a file move from the old folder plus the new file basename.
if (fs.existsSync(wouldBe) && !fs.lstatSync(wouldBe).isDirectory()) {
isFolder = false
oldPath = wouldBe
@@ -142,22 +120,19 @@ async function main(opts: MoveOptions, nameTuple: string[]) {
process.exit(2)
}
- // This will exit non-zero if anything is wrong with these inputs
validateFileInputs(oldPath, newPath, isFolder)
const oldHref = makeHref(CONTENT_ROOT, undo ? newPath : oldPath)
const newHref = makeHref(CONTENT_ROOT, undo ? oldPath : newPath)
if (isFolder) {
- // The folder must have an index.md file
+ // Folders can move only when they have an index.md landing file.
const indexFilePath = path.join(oldPath, 'index.md')
if (!fs.existsSync(indexFilePath)) {
throw new Error(`${oldPath} does not have an index.md file`)
}
- // Gather individual files by walking `oldPath` recursively.
const files = findFilesInFolder(oldPath, newPath, opts)
- // First take care of the `git mv` (or regular rename) part.
if (undo) {
undoFolder(oldPath, newPath, files, opts)
} else {
@@ -172,10 +147,8 @@ async function main(opts: MoveOptions, nameTuple: string[]) {
editFiles(files, false, opts)
}
} else {
- // When it's just an individual file, it's easier.
const files: FileTuple[] = [[oldPath, newPath, oldHref, newHref]]
- // First take care of the `git mv` (or regular rename) part.
moveFiles(files, opts)
if (undo) {
@@ -185,11 +158,9 @@ async function main(opts: MoveOptions, nameTuple: string[]) {
}
}
- // Updating featuredLinks front matter actually doesn't care if
- // the file is a folder or not. It just needs to know the old and new hrefs.
+ // featuredLinks updates need old and new hrefs, not whether the path is a file or folder.
changeFeaturedLinks(oldHref, newHref)
- // Update any links in ChildGroups on the homepage.
changeHomepageLinks(oldHref, newHref, verbose)
if (!undo) {
@@ -205,8 +176,7 @@ async function main(opts: MoveOptions, nameTuple: string[]) {
function validateFileInputs(oldPath: string, newPath: string, isFolder: boolean) {
if (isFolder) {
- // Make sure that only the last portion of the path is different
- // and that all preceding are equal.
+ // Directory moves can change only the last path segment unless the destination base exists.
const [oldBase, oldName] = splitDirectory(oldPath)
const [newBase] = splitDirectory(newPath)
if (oldBase !== newBase && !existsAndIsDirectory(newBase)) {
@@ -333,9 +303,7 @@ function undoFolder(oldPath: string, newPath: string, files: FileTuple[], opts:
}
function getBasename(fileOrDirectory: string) {
- // Note, can't use fs.lstatSync().isDirectory() because it's just a string
- // at this point. It might not exist.
-
+ // Infer file or directory names from path strings because the destination may not exist.
if (fileOrDirectory.endsWith('index.md')) {
return path.basename(path.dirname(fileOrDirectory))
}
@@ -444,9 +412,9 @@ function addToChildren(newPath: string, positions: PositionInfo, opts: MoveOptio
}
}
+// When git runs, commit pure renames before edits so later merges avoid complex three-way diffs.
function moveFiles(files: FileTuple[], opts: MoveOptions) {
const { verbose, git: useGit } = opts
- // Before we do anything, assert that the files are valid
for (const [oldPath] of files) {
const fileContent = fs.readFileSync(oldPath, 'utf-8')
const { errors } = fm(fileContent, { filepath: oldPath })
@@ -458,13 +426,6 @@ function moveFiles(files: FileTuple[], opts: MoveOptions) {
if (errors.length > 0) throw new Error('There were more than 0 parse errors')
}
- // In the first loop, we exclusively perform the rename. No file edits!
- // The reason is that we don't want lump renaming and edits in the same
- // git commit.
- // By having a dedicated git commit that purely renames (without changing
- // any content) is best practice to avoid complex 3-way diffs that
- // `git merge` does when you later have to merge in the latest `main`
- // into your ongoing renaming branch.
for (const [oldPath, newPath] of files) {
if (verbose) {
console.log(`Moving ${chalk.bold(oldPath)} to ${chalk.bold(newPath)}`)
@@ -493,13 +454,10 @@ function moveFiles(files: FileTuple[], opts: MoveOptions) {
}
}
+// editFiles keeps redirect_from edits in a separate commit from renames when git runs.
function editFiles(files: FileTuple[], updateParent: boolean, opts: MoveOptions) {
const { verbose, git: useGit } = opts
- // Second loop. This time our only job is to edit the `redirects_from`
- // frontmatter key.
- // See comment in the first loop above for why we're looping over the files
- // two times.
for (const [oldPath, newPath, oldHref] of files) {
const fileContent = fs.readFileSync(newPath, 'utf-8')
const { content, data } = readFrontmatter(fileContent)
@@ -518,7 +476,7 @@ function editFiles(files: FileTuple[], updateParent: boolean, opts: MoveOptions)
}
}
- // Add contentType frontmatter to moved files
+ // Moved files get contentType from target paths.
if (files.length > 0) {
const filePaths = files.map(([, newPath]) => newPath)
try {
@@ -553,7 +511,6 @@ function editFiles(files: FileTuple[], updateParent: boolean, opts: MoveOptions)
function undoFiles(files: FileTuple[], updateParent: boolean, opts: MoveOptions) {
const { verbose, git: useGit } = opts
- // First undo any edits to the file
for (const [oldPath, newPath, oldHref] of files) {
const fileContent = fs.readFileSync(newPath, 'utf-8')
const { content, data } = readFrontmatter(fileContent)
@@ -580,10 +537,9 @@ function undoFiles(files: FileTuple[], updateParent: boolean, opts: MoveOptions)
}
}
+// Regex replacement preserves YAML formatting and comments that serialization would lose.
+// Homepage childGroup hrefs omit the leading slash.
function changeHomepageLinks(oldHref: string, newHref: string, verbose: boolean) {
- // Can't deserialize and serialize the Yaml because it would lose
- // formatting and comments. So regex replace it.
- // Homepage childGroup links do not have a leading '/', so we need to remove that.
const homepageOldHref = oldHref.replace('/', '')
const homepageNewHref = newHref.replace('/', '')
const escapedHomepageOldHref = RegExp.escape(homepageOldHref)
diff --git a/src/content-render/scripts/reusables-cli.ts b/src/content-render/scripts/reusables-cli.ts
index d253cd6d2a36..4c84f3496d0a 100644
--- a/src/content-render/scripts/reusables-cli.ts
+++ b/src/content-render/scripts/reusables-cli.ts
@@ -1,7 +1,5 @@
-/**
- * @purpose Writer tool
- * @description Find all content files that use a specific reusable
- */
+// @purpose Writer tool
+// @description Find all content files that use a specific reusable
// Usage: npm run reusables -- --help
// Usage: npm run reusables -- find used accounts/create-account.md
// Usage: npm run reusables -- find unused accounts/create-account.md
diff --git a/src/content-render/scripts/reusables-cli/find/potential-uses.ts b/src/content-render/scripts/reusables-cli/find/potential-uses.ts
index c0827117caa9..af2886568cbe 100644
--- a/src/content-render/scripts/reusables-cli/find/potential-uses.ts
+++ b/src/content-render/scripts/reusables-cli/find/potential-uses.ts
@@ -63,7 +63,7 @@ export function findPotentialUses({
reusableCount += 1
for (const { filePath, fileContents } of allFileContents) {
- // Skip the reusable file itself
+ // Do not report a reusable as a use of itself.
if (filePath === reusableFilePath) continue
const indices = findIndicesOfSubstringInString(reusableContents.trim(), fileContents)
diff --git a/src/content-render/scripts/reusables-cli/find/unused.ts b/src/content-render/scripts/reusables-cli/find/unused.ts
index 1f7bf29e8711..9906fb5b663e 100644
--- a/src/content-render/scripts/reusables-cli/find/unused.ts
+++ b/src/content-render/scripts/reusables-cli/find/unused.ts
@@ -33,7 +33,7 @@ export function findUnused({ absolute }: { absolute: boolean }) {
args.startsWith('reusables.')
) {
const reusableName = `${path.join('data', ...args.split(' ')[0].split('.'))}.md`
- // Special cases where we don't want them to count as reusables. It's an example in a how-to doc
+ // Ignore how-to examples that use fake reusable names.
if (
reusableName.includes('foo/bar.md') ||
reusableName.includes('foo/par.md') ||
diff --git a/src/content-render/scripts/reusables-cli/find/used.ts b/src/content-render/scripts/reusables-cli/find/used.ts
index 6f56c31512d6..23e44a37d38f 100644
--- a/src/content-render/scripts/reusables-cli/find/used.ts
+++ b/src/content-render/scripts/reusables-cli/find/used.ts
@@ -26,7 +26,7 @@ export function findUsed(reusablePath: string, { absolute }: { absolute: boolean
const filesWithReusables: FilesWithLineNumbers = []
for (const filePath of allFilePaths) {
- // Skip the reusable file itself
+ // Do not report a reusable as a use of itself.
if (filePath === reusableFilePath) continue
const fileContents = fs.readFileSync(filePath, 'utf-8')
diff --git a/src/content-render/scripts/reusables-cli/ignore-reusables.ts b/src/content-render/scripts/reusables-cli/ignore-reusables.ts
index 9c9979f80f54..2460a9878523 100644
--- a/src/content-render/scripts/reusables-cli/ignore-reusables.ts
+++ b/src/content-render/scripts/reusables-cli/ignore-reusables.ts
@@ -1,5 +1,4 @@
-// List of reusables to ignore when checking for potential uses of reusables
-// Make sure paths are relative to the root of the repo
+// List repo-relative reusables excluded from potential-use checks.
export const reusablesToIgnore = [
- 'data/reusables/copilot/trial-period.md', // Just a number, so it pops up in unrelated files
+ 'data/reusables/copilot/trial-period.md', // This numeric reusable matches unrelated files.
]
diff --git a/src/content-render/scripts/reusables-cli/shared.ts b/src/content-render/scripts/reusables-cli/shared.ts
index c04e24725d15..454e0477159e 100644
--- a/src/content-render/scripts/reusables-cli/shared.ts
+++ b/src/content-render/scripts/reusables-cli/shared.ts
@@ -73,12 +73,12 @@ export function getIndicesOfLiquidVariable(liquidVariable: string, fileContents:
}
export function resolveReusablePath(reusablePath: string): string {
- // Try .md if extension is not provided
+ // Append .md when the reusable path has no extension.
if (!reusablePath.endsWith('.md') && !reusablePath.endsWith('.yml')) {
reusablePath += '.md'
}
- // Allow user to just pass the name of the file. If it's not ambiguous, we'll find it.
+ // Resolve a path fragment only when it matches exactly one reusable file.
const allReusableFiles = getAllReusablesFilePaths()
const foundPaths = []
for (const possiblePath of allReusableFiles) {
@@ -130,13 +130,12 @@ export function findIndicesOfSubstringInString(substr: string, str: string): num
}
export function findSimilarSubStringInString(substr: string, str: string) {
- // Take every sentence in the substr, lower case it, and compare it to every sentence in the str to get a similarity score
+ // Score each substring sentence against each corpus sentence by shared words.
const substrSentences = substr.split('.').map((sentence) => sentence.toLowerCase())
const corpus = str.split('.').map((sentence) => sentence.toLowerCase())
let similarityScore = 0
- // Find how similar every two strings are based on the words they share
for (const substrSentence of substrSentences) {
for (const sentence of corpus) {
const substrTokens = substrSentence.split(' ')
diff --git a/src/content-render/scripts/update-filepaths.ts b/src/content-render/scripts/update-filepaths.ts
index 7760d5c7b441..45bc147c26d8 100755
--- a/src/content-render/scripts/update-filepaths.ts
+++ b/src/content-render/scripts/update-filepaths.ts
@@ -1,7 +1,5 @@
-/**
- * @purpose Writer tool
- * @description Update content filenames to match short titles
- */
+// @purpose Writer tool
+// @description Update content filenames to match short titles
import fs from 'fs'
import path from 'path'
@@ -53,11 +51,12 @@ const estimateScriptMinutes = (numberOfFiles: number): string => {
return estNum === 0 ? '<1' : estNum.toString()
}
+// main processes files sequentially because move-content must move files before directories,
+// and deepest directories before parents.
+// Async does not shorten this work because each path move depends on the ordered result.
async function main(): Promise {
const slugger = new GithubSlugger()
const contentDir: string = path.join(process.cwd(), 'content')
- // Filter to get all the content files we want to read in.
- // Then sort them from longest > shortest so we can do the file moves in order.
const filesToProcess: string[] = sortFiles(filterFiles(contentDir, options))
if (filesToProcess.length === 0) {
@@ -71,11 +70,6 @@ async function main(): Promise {
console.log(`Estimated time: ${estimate} min\n`)
}
- // Process files sequentially to maintain the correct order of operations.
- // Files must be moved before directories, and directories must be moved
- // from deepest to shallowest to avoid path conflicts during the move operations.
- // The result is rather slow, but an asynchronous approach that ensures
- // sequential processing would not be faster.
for (const file of filesToProcess) {
try {
slugger.reset()
@@ -110,24 +104,16 @@ async function processFile(
stringToSlugify = await renderContent(stringToSlugify, context, { textOnly: true })
}
- // Slugify the short title of each article.
- // Where: shortTitle = Foo bar
- // Returns: slug = foo-bar
- // Fall back to title if shortTitle doesn't exist.
+ // Slug shortTitle, or title when shortTitle is absent, to get the target basename.
const slug: string = slugger.slug(decode(stringToSlugify))
let basename: string
if (isDirectory) {
- // Where: content location = content/foobar/index.md
- // Returns: basename = foobar
basename = path.basename(path.dirname(file))
} else {
- // Where: content location = content/foobar.md
- // Returns: basename = foobar
basename = path.basename(file, '.md')
}
- // If slug and basename already match, all set here. Return early.
if (slug === basename) return null
const newPath = isDirectory
@@ -153,7 +139,7 @@ function moveFile(result: string[], scriptOptions: ScriptOptions): void {
return
}
- // Call out to well-tested move-content script for the moving and redirect adding functions.
+ // move-content handles file moves, redirects, and children updates.
const stdout = execFileSync(
'tsx',
[
@@ -166,7 +152,7 @@ function moveFile(result: string[], scriptOptions: ScriptOptions): void {
{ encoding: 'utf8' },
)
- // Grab just the "Moving..." and "Renamed..." output from stdout; otherwise output is too noisy.
+ // Print only Moving or Renamed lines unless verbose; full move-content output is noisy.
const moveMsg = stdout.split('\n').find((l) => l.startsWith('Moving') || l.startsWith('Renamed'))
if (moveMsg && !options.verbose) {
console.log(moveMsg, '\n')
@@ -176,11 +162,7 @@ function moveFile(result: string[], scriptOptions: ScriptOptions): void {
}
function sortFiles(filesArray: string[]): string[] {
- // The order of operations is important.
- // We need to return an array so that the moving operations happens in this order:
- // 1. Filepaths
- // 2. Deepest subdirectory path
- // 3. Shallowest subdirectory path (up to category level, e.g., content/product/category)
+ // Move files before directories, then deepest directories before parents.
return filesArray.toSorted((a, b) => {
if (!isDirectoryCheck(a) && isDirectoryCheck(b)) {
return -1
@@ -194,7 +176,7 @@ function sortFiles(filesArray: string[]): string[] {
if (isDirectoryCheck(a) && isDirectoryCheck(b)) {
const aDepth = a.split(path.sep).length
const bDepth = b.split(path.sep).length
- return bDepth - aDepth // Deeper paths first
+ return bDepth - aDepth
}
return 0
@@ -203,21 +185,19 @@ function sortFiles(filesArray: string[]): string[] {
function filterFiles(contentDir: string, scriptOptions: ScriptOptions) {
return walkFiles(contentDir, ['.md']).filter((file: string) => {
- // Never move readmes
+ // Keep README paths unchanged.
if (file.endsWith('README.md')) return false
- // Never move early access files
+ // Keep early access paths unchanged.
if (file.includes('early-access')) return false
- // Never move the homepage (content/index.md)
+ // Keep the homepage path unchanged.
if (path.relative(contentDir, file) === 'index.md') return false
- // Never move product landings (content/foo/index.md)
+ // Keep product landing paths unchanged.
if (path.relative(contentDir, file).split(path.sep)[1] === 'index.md') return false
- // If no specific paths are passed, we are done filtering.
if (!scriptOptions.paths) return true
return scriptOptions.paths.some((p: string) => {
- // Allow either a full content path like "content/foo/bar.md"
- // or a top-level directory name like "copilot"
+ // Accept full content paths like content/foo/bar.md or top-level dirs like copilot.
if (!p.startsWith('content')) {
p = path.join('content', p)
}
@@ -236,7 +216,7 @@ function determineProcessStatus(
isDirectory: boolean,
scriptOptions: ScriptOptions,
): boolean {
- // A directory is never processed when dirs are excluded, whatever else is set.
+ // exclude-dirs prevents directory moves even when force is set.
if (isDirectory && scriptOptions.excludeDirs) {
return false
}
diff --git a/src/content-render/tests/annotate.ts b/src/content-render/tests/annotate.ts
index c47a78d6fb76..05e220bd62d8 100644
--- a/src/content-render/tests/annotate.ts
+++ b/src/content-render/tests/annotate.ts
@@ -124,7 +124,6 @@ on: [push]
\`\`\`
`
- // Create a mock context with pages for AUTOTITLE resolution
const mockPages: Record = {
'/get-started/start-your-journey/hello-world': {
href: '/get-started/start-your-journey/hello-world',
@@ -141,7 +140,7 @@ on: [push]
currentVersion: 'free-pro-team@latest',
pages: mockPages,
redirects: {},
- // Mock test object doesn't need all Context properties, using 'as unknown as' to bypass strict type checking
+ // AUTOTITLE resolution reads only these Context fields.
} as unknown as Context
const res = await renderContent(autotitleExample, mockContext)
diff --git a/src/content-render/tests/collect-mini-toc.ts b/src/content-render/tests/collect-mini-toc.ts
index ae8b6942ccc4..eaf110d5d833 100644
--- a/src/content-render/tests/collect-mini-toc.ts
+++ b/src/content-render/tests/collect-mini-toc.ts
@@ -62,7 +62,7 @@ describe('collect-mini-toc rehype plugin', () => {
})
test('does not collect when collectMiniToc is not provided', async () => {
- // Should not throw — plugin is a no-op without collectInto
+ // Without collectMiniToc, the plugin is a no-op.
const result = await renderContent('## Heading')
expect(result).toContain('Heading')
})
diff --git a/src/content-render/tests/data.ts b/src/content-render/tests/data.ts
index 85fbe23faa54..99365061b34e 100644
--- a/src/content-render/tests/data.ts
+++ b/src/content-render/tests/data.ts
@@ -42,9 +42,7 @@ describe('data tag', () => {
currentPath: '/en/liquid-tags/good-data-variable',
}
const rendered = await page!.render(context)
- // The test fixture contains:
- // {% data variables.stuff.foo %}
- // which we control the value of here in the test.
+ // good-data-variable.md uses {% data variables.stuff.foo %} from the test data directory.
expect(rendered.includes('Foo')).toBeTruthy()
})
test('should throw if the data tag is used with something unrecognized', async () => {
diff --git a/src/content-render/tests/link-error-line-numbers.ts b/src/content-render/tests/link-error-line-numbers.ts
index 36cd3d1f842e..734e2fa2d77c 100644
--- a/src/content-render/tests/link-error-line-numbers.ts
+++ b/src/content-render/tests/link-error-line-numbers.ts
@@ -54,9 +54,6 @@ More content here.`
} catch (error) {
expect(error).toBeInstanceOf(TitleFromAutotitleError)
- // The broken link is on line 10 in the original file
- // (3 lines of frontmatter + 1 blank line + 1 title + 1 blank + 1 content + 1 blank + 1 link line)
- // The error message should reference the correct line number
expect((error as TitleFromAutotitleError).message).toContain('/nonexistent/page')
expect((error as TitleFromAutotitleError).message).toContain('could not be resolved')
expect((error as TitleFromAutotitleError).message).toContain('(Line: 10)')
diff --git a/src/content-render/tests/liquid-tags.ts b/src/content-render/tests/liquid-tags.ts
index db28d494733b..5151423349d5 100644
--- a/src/content-render/tests/liquid-tags.ts
+++ b/src/content-render/tests/liquid-tags.ts
@@ -55,7 +55,8 @@ This uses {% data variables.product.prodname_dotcom %} in content.
const expandedContent = await fs.readFile(testFile, 'utf8')
expect(expandedContent).not.toBe(testContent)
- expect(expandedContent).toContain('GitHub') // Should expand to actual fixture value
+ // The fixture data tag expands to GitHub.
+ expect(expandedContent).toContain('GitHub')
})
test('restore command should complete successfully', async () => {
diff --git a/src/content-render/tests/liquid.ts b/src/content-render/tests/liquid.ts
index e38b32f68ba7..82e8053b10c4 100644
--- a/src/content-render/tests/liquid.ts
+++ b/src/content-render/tests/liquid.ts
@@ -8,10 +8,7 @@ import { allVersions } from '@/versions/lib/all-versions'
import enterpriseServerReleases from '@/versions/lib/enterprise-server-releases'
import type { Context, ExtendedRequest, Page } from '@/types'
-// Setup these variables so we don't need to manually update tests as GHES
-// versions continually get deprecated. For example, if we deprecate GHES 3.0,
-// oldestSupportedGhes will be 3.1, secondOldestSupportedGhes will be 3.2, and
-// thirdOldestSupportedGhes will be 3.3.
+// Derive GHES versions from supported releases so deprecations do not require test updates.
const oldestSupportedGhes =
enterpriseServerReleases.supported[enterpriseServerReleases.supported.length - 1]
const secondOldestSupportedGhes =
@@ -50,7 +47,7 @@ describe('liquid template parser', () => {
vi.setConfig({ testTimeout: 60 * 1000 })
describe('short versions', () => {
- // Create a fake req so we can test the shortVersions middleware
+ // shortVersionsMiddleware reads and mutates a request context.
const req = { language: 'en', query: {} } as ExtendedRequest
test('FPT works as expected when it is FPT', async () => {
@@ -61,7 +58,7 @@ describe('liquid template parser', () => {
} as Context
contextualize(req)
const output = await liquid.parseAndRender(shortVersionsTemplate, req.context)
- // We should have TWO results because we are supporting two shortcuts
+ // FPT matches directly and through the fpt or ghes shortcut.
expect(output.replace(/\s\s+/g, ' ').trim()).toBe(
`I am FPT I am FTP or GHES < ${secondOldestSupportedGhes}`,
)
@@ -70,7 +67,6 @@ describe('liquid template parser', () => {
test('GHEC works as expected', async () => {
req.context = {
currentVersion: 'enterprise-cloud@latest',
- // page: {},
allVersions,
enterpriseServerReleases,
} as Context
@@ -144,13 +140,13 @@ describe('liquid template parser', () => {
})
describe('feature versions', () => {
- // Create a fake req so we can test the feature versions middleware
+ // featureVersionsMiddleware reads and mutates a request context.
const req = { language: 'en', query: {} } as ExtendedRequest
test('does not render in FPT because feature is not available in FPT', async () => {
req.context = {
currentVersion: 'free-pro-team@latest',
- page: {} as Page, // it just has to be any truthy value
+ page: {} as Page, // featureVersionsMiddleware only checks that page is truthy.
allVersions,
enterpriseServerReleases,
} as Context
@@ -162,7 +158,7 @@ describe('liquid template parser', () => {
test('renders in GHES because feature is available in GHES', async () => {
req.context = {
currentVersion: `enterprise-server@${enterpriseServerReleases.latest}`,
- page: {} as Page, // it just has to be any truthy value
+ page: {} as Page, // featureVersionsMiddleware only checks that page is truthy.
allVersions,
enterpriseServerReleases,
} as Context
@@ -174,7 +170,7 @@ describe('liquid template parser', () => {
test('renders in GHEC because feature is available in GHEC', async () => {
req.context = {
currentVersion: 'enterprise-cloud@latest',
- page: {} as Page, // it just has to be any truthy value
+ page: {} as Page, // featureVersionsMiddleware only checks that page is truthy.
allVersions,
enterpriseServerReleases,
} as Context
diff --git a/src/content-render/tests/prompt-id.ts b/src/content-render/tests/prompt-id.ts
index 71e046b0fd48..ff26162f4653 100644
--- a/src/content-render/tests/prompt-id.ts
+++ b/src/content-render/tests/prompt-id.ts
@@ -39,13 +39,13 @@ describe('generatePromptId', () => {
})
test('generates deterministic IDs (regression test)', () => {
- // These specific values ensure the hash function remains consistent
+ // Fixed hash outputs catch unintended murmurhash changes.
expect(generatePromptId('hello world')).toBe('1730621824')
expect(generatePromptId('test')).toBe('4180565944')
})
test('handles prompts with code context (ref pattern)', () => {
- // When ref= is used, the prompt includes referenced code + prompt text separated by newline
+ // ref= prompts include referenced code, a newline, then prompt text.
const codeContext =
'function logPersonAge(name, age, revealAge) {\n if (revealAge) {\n console.log(name);\n }\n}'
const promptText = 'Improve the variable names in this function'
@@ -59,15 +59,15 @@ describe('generatePromptId', () => {
})
test('handles very long prompts', () => {
- // Real-world prompts can include entire code blocks (100+ lines)
- const longCode = 'x\n'.repeat(500) // 500 lines
+ // Real prompts can include code blocks longer than 100 lines.
+ const longCode = 'x\n'.repeat(500)
const id = generatePromptId(longCode)
expect(typeof id).toBe('string')
expect(id.length).toBeGreaterThan(0)
})
test('handles prompts with backticks and template literals', () => {
- // Prompts often include inline code with backticks
+ // Prompts can include inline code delimiters.
const prompt = "In JavaScript I'd write: `The ${numCats === 1 ? 'cat is' : 'cats are'} hungry.`"
const id = generatePromptId(prompt)
expect(typeof id).toBe('string')
@@ -75,7 +75,7 @@ describe('generatePromptId', () => {
})
test('handles prompts with placeholders', () => {
- // Content uses placeholders like NEW-LANGUAGE, OWNER/REPOSITORY
+ // Content uses placeholders like NEW-LANGUAGE and OWNER/REPOSITORY.
const id1 = generatePromptId('What is NEW-LANGUAGE best suited for?')
const id2 = generatePromptId('In OWNER/REPOSITORY, create a feature request')
expect(id1).not.toBe(id2)
@@ -84,7 +84,7 @@ describe('generatePromptId', () => {
})
test('handles unicode and international characters', () => {
- // May encounter non-ASCII characters in prompts
+ // Prompts can include non-ASCII text.
const id1 = generatePromptId('Explique-moi le code en français')
const id2 = generatePromptId('コードを説明してください')
const id3 = generatePromptId('Объясните этот код')
diff --git a/src/content-render/tests/render-changed-and-deleted-files.ts b/src/content-render/tests/render-changed-and-deleted-files.ts
index 617089e59ea4..b61d0b715cc5 100644
--- a/src/content-render/tests/render-changed-and-deleted-files.ts
+++ b/src/content-render/tests/render-changed-and-deleted-files.ts
@@ -1,37 +1,13 @@
-/**
- * To "debug" this test locally, you need to set at least one of these
- * environment variables:
- *
- * - CHANGED_FILES
- * - DELETED_FILES
- * - RENAMED_FILES
- *
- * `CHANGED_FILES` and `DELETED_FILES` are whitespace-separated lists of
- * paths to content files. `RENAMED_FILES` is a whitespace-separated list
- * of `oldPath,newPath` pairs (as emitted by tj-actions/changed-files
- * `all_old_new_renamed_files` output). For example:
- *
- * export CHANGED_FILES="content/get-started/index.md content/get-started/start-your-journey/hello-world.md"
- * export RENAMED_FILES="content/old/path.md,content/new/path.md"
- *
- * If any of the paths in there, split by ' ', don't match real files, the
- * test will fail before it even starts. Meaning, it will throw an error
- * rather than failing an `expect(...)` assertion.
- *
- * Technically, the value is any whitespace. So you can actually use:
- *
- * export DELETED_FILES=`git diff --name-only main...`
- *
- * which will make the environment variable be newline-separated and that
- * works too.
- *
- * So, for example, if you've made some deletions and some edits the
- * staged files:
- *
- * export DELETED_FILES=`git diff --name-only --diff-filter=D main...`
- * export CHANGED_FILES=`git diff --name-only --diff-filter=M main...`
- * npm run test -- src/content-render/tests/render-changed-and-deleted-files.ts
- */
+// To run this test locally, set CHANGED_FILES, DELETED_FILES, or RENAMED_FILES.
+// CHANGED_FILES and DELETED_FILES contain whitespace-separated content paths.
+// RENAMED_FILES contains oldPath,newPath pairs from tj-actions/changed-files.
+// CHANGED_FILES paths must identify loaded pages or the test throws before expectations run.
+// Newline-separated git diff output works because the parser accepts all whitespace.
+// Example:
+// export CHANGED_FILES="content/get-started/index.md content/actions/index.md"
+// export RENAMED_FILES="content/old/path.md,content/new/path.md"
+// export DELETED_FILES="$(git diff --name-only --diff-filter=D main...)"
+// npm run test -- src/content-render/tests/render-changed-and-deleted-files.ts
import path from 'path'
@@ -52,10 +28,8 @@ function getDeletedContentFiles() {
return getContentFiles(process.env.DELETED_FILES)
}
-// Parse `RENAMED_FILES` from tj-actions/changed-files `all_old_new_renamed_files`
-// output. Each whitespace-separated entry is an `oldPath,newPath` pair. We return
-// the OLD paths so they can be checked the same way deleted files are: the test
-// will fail if the old URL 404s (i.e. no redirect was set up for the rename).
+// RENAMED_FILES comes from tj-actions/changed-files all_old_new_renamed_files.
+// Each oldPath,newPath entry adds the old path because old URLs must not return 404.
function getRenamedOldContentFiles() {
const raw = (process.env.RENAMED_FILES || '').split(/\s+/g).filter(Boolean)
const oldPaths = raw.map((pair) => pair.split(',')[0]).filter(Boolean)
@@ -64,7 +38,7 @@ function getRenamedOldContentFiles() {
function getContentFiles(spaceSeparatedList: string | undefined): string[] {
return (spaceSeparatedList || '').split(/\s+/g).filter((filePath) => {
- // This filters out things like '', or `data/foo.md` or `content/something/README.md`
+ // Only content Markdown pages count; data files and content README files do not render.
return (
filePath.endsWith('.md') &&
filePath.split(path.sep)[0] === 'content' &&
@@ -73,23 +47,18 @@ function getContentFiles(spaceSeparatedList: string | undefined): string[] {
})
}
-// If the list of changed pages is very large, this test can take a long time.
-// It can also happen if some of the pages involves are infamously slow.
-// For example guide pages because they involved a lot of processing
-// to gather and preview linked data.
+// Large changes and guide pages can render slowly because guides gather linked data.
vi.setConfig({ testTimeout: 60 * 1000 })
describe('changed-content', () => {
const changedContentFiles = getChangedContentFiles()
- // `test.each` will throw if the array is empty, so we need to add a dummy
- // when there are no changed files in the environment.
+ // test.each throws on an empty array, so EMPTY stands in when no files are present.
const testFiles: Array = changedContentFiles.length
? changedContentFiles
: [EMPTY]
test.each(testFiles)('changed-content: %s', async (file: string | symbol) => {
- // Necessary because `test.each` will throw if the array is empty
if (file === EMPTY) return
const page = pageList.find((p) => {
@@ -98,7 +67,7 @@ describe('changed-content', () => {
if (!page) {
throw new Error(`Could not find page for ${file as string} in all loaded English content`)
}
- // Each version of the page should successfully render
+ // Every permalink must render because changed files can affect all versions.
for (const { href } of page.permalinks) {
const res = await get(href)
if (!res.ok) {
@@ -114,19 +83,16 @@ describe('changed-content', () => {
})
describe('deleted-content', () => {
- // Renamed files (status `R` from git) don't appear in `DELETED_FILES`, but
- // the old path is just as gone from the user's perspective and needs a
- // redirect. Treat the old path of each rename the same as a deleted file.
+ // RENAMED_FILES provides old paths separately because git status R paths skip DELETED_FILES.
const deletedContentFiles = [...getDeletedContentFiles(), ...getRenamedOldContentFiles()]
- // `test.each` will throw if the array is empty, so we need to add a dummy
- // when there are no deleted files in the environment.
+ // test.each throws on an empty array, so EMPTY stands in when no files are present.
const testFiles: Array = deletedContentFiles.length
? deletedContentFiles
: [EMPTY]
+ // Deleted pages no longer have versions frontmatter, so this checks the versionless permalink.
test.each(testFiles)('deleted-content: %s', async (file: string | symbol) => {
- // Necessary because `test.each` will throw if the array is empty
if (file === EMPTY) return
const page = pageList.find((p) => {
@@ -137,9 +103,6 @@ describe('deleted-content', () => {
`The supposedly deleted file ${file as string} is still in list of loaded pages`,
)
}
- // You can't know what the possible permalinks were for a deleted page,
- // because it's deleted so we can't look at its `versions` front matter.
- // However, we always make sure all pages work in versionless.
const indexmdSuffixRegex = new RegExp(`${path.sep}index\\.md$`)
const mdSuffixRegex = /\.md$/
const relativePath = (file as string).split(path.sep).slice(1).join(path.sep)
@@ -150,9 +113,7 @@ describe('deleted-content', () => {
res.statusCode === 404
? `The deleted or renamed file ${file as string} did not set up a redirect.`
: ''
- // Certain articles that are deleted and moved under a directory with the same article name
- // should just route to the subcategory page instead of redirecting (docs content team confirmed).
- // So, in this scenario, we'd get a 200 status code.
+ // Same-name subcategory moves return 200 instead of redirecting.
expect(res.statusCode === 301 || res.statusCode === 200, error).toBe(true)
})
})
diff --git a/src/content-render/tests/render-content.ts b/src/content-render/tests/render-content.ts
index dc1cdbbf9576..939abf540582 100644
--- a/src/content-render/tests/render-content.ts
+++ b/src/content-render/tests/render-content.ts
@@ -4,8 +4,7 @@ import { describe, expect, test } from 'vitest'
import { renderContent } from '@/content-render/index'
import { EOL } from 'os'
-// Use platform-specific line endings for realistic tests when templates have
-// been loaded from disk
+// Disk-loaded templates use platform line endings, so tests do too.
const nl = (str: string): string => str.replace(/\n/g, EOL)
describe('renderContent', () => {
@@ -240,8 +239,8 @@ var a = 1
const html = await renderContent(template)
const $ = load(html)
const el = $('button.js-btn-copy')
+ // Copy buttons use a murmurhash ID that matches the paired pre element.
expect(el.data('clipboard')).toBe(2967273189)
- // Generates a murmurhash based ID that matches a
})
describe('wrap-code-terms ( in table code)', () => {
diff --git a/src/content-render/tests/render-to-hast.ts b/src/content-render/tests/render-to-hast.ts
index c50f67640d84..0bfddca3c574 100644
--- a/src/content-render/tests/render-to-hast.ts
+++ b/src/content-render/tests/render-to-hast.ts
@@ -4,11 +4,8 @@ import { renderContentToHast } from '@/content-render/index'
import { renderUnified, renderUnifiedToHast } from '@/content-render/unified/index'
import type { Context } from '@/types'
-// A corpus that exercises the parts of the pipeline most likely to differ
-// between "stringify the processed vfile" (today) and "stringify the hast tree
-// we stopped at" (the new hast path): headings (slug + anchor links), code
-// blocks (highlight + code-header), tables (several rewrite plugins), alerts,
-// raw inline HTML (rehype-raw), and images.
+// This corpus covers pipeline stages where vfile HTML and hast-derived HTML can diverge:
+// headings, highlighted code, tables, alerts, raw inline HTML, images, and blockquotes.
const fixtures: Array<{ name: string; template: string }> = [
{ name: 'paragraph', template: 'Hello **world**, this is a [link](https://github.com).' },
{
diff --git a/src/content-render/tests/table-accessibility-labels.ts b/src/content-render/tests/table-accessibility-labels.ts
index e17e246cf096..a69844db08c0 100644
--- a/src/content-render/tests/table-accessibility-labels.ts
+++ b/src/content-render/tests/table-accessibility-labels.ts
@@ -4,8 +4,7 @@ import { describe, expect, test } from 'vitest'
import { renderContent } from '@/content-render/index'
import { EOL } from 'os'
-// Use platform-specific line endings for realistic tests when templates have
-// been loaded from disk
+// Disk-loaded templates use platform line endings, so tests do too.
const nl = (str: string) => str.replace(/\n/g, EOL)
describe('table accessibility labels', () => {
@@ -170,7 +169,7 @@ Some additional context here.
const tables = $('table')
expect(tables.length).toBe(2)
expect($(tables[0]).attr('aria-labelledby')).toBe('first-heading')
- // Second table should not get the same heading since the first table is in between
+ // A prior table stops heading lookup, so the second table stays unlabeled.
expect($(tables[1]).attr('aria-labelledby')).toBeUndefined()
})
diff --git a/src/fixtures/helpers/color-contrast.ts b/src/fixtures/helpers/color-contrast.ts
index 4d2e6fc8fe77..1f6463defa36 100644
--- a/src/fixtures/helpers/color-contrast.ts
+++ b/src/fixtures/helpers/color-contrast.ts
@@ -1,5 +1,5 @@
-// WCAG contrast for computed `rgb()`/`rgba()` colours. Keywords, hex and
-// translucent values throw rather than being coerced — `rgba(0, 0, 0, 0)` would
+// Computes WCAG contrast only for opaque computed rgb()/rgba() colours.
+// Reject keywords, hex, and translucent values, because rgba(0, 0, 0, 0) would
// otherwise read as opaque black and yield a confident, wrong ratio.
function parseComputedColor(color: string) {
diff --git a/src/fixtures/helpers/turn-off-experiments.ts b/src/fixtures/helpers/turn-off-experiments.ts
index cfb9547b4e2e..b26fb37bee9e 100644
--- a/src/fixtures/helpers/turn-off-experiments.ts
+++ b/src/fixtures/helpers/turn-off-experiments.ts
@@ -18,7 +18,7 @@ async function alterExperimentsInPage(
variation: typeof TREATMENT_VARIATION | typeof CONTROL_VARIATION,
) {
const experiments = getActiveExperiments('all')
- // Include a page.evaluate call to simulate the same # of events as if an experiment were active
+ // When no experiments run, page.evaluate keeps the Playwright event count matching active runs.
if (!experiments.length) {
await page.evaluate(() => {
console.log('No experiments to turn off, skipping')
@@ -28,7 +28,7 @@ async function alterExperimentsInPage(
for (const experiment of getActiveExperiments('all')) {
await page.evaluate(
({ experimentKey, variationType }) => {
- // @ts-expect-error overrideControlGroup is a custom function added to the window object
+ // @ts-expect-error -- overrideControlGroup is a custom window helper for experiment tests.
window.overrideControlGroup(experimentKey, variationType)
},
{ experimentKey: experiment.key, variationType: variation },
@@ -36,8 +36,7 @@ async function alterExperimentsInPage(
}
}
-// Place Playwright tests in control group for every active experiment
-// To write a test for an experiment, explicitly turn that experiment on in the test
+// Playwright fixtures start in the control group; tests opt into treatments explicitly.
export function turnOffExperimentsBeforeEach(test: typeof Test) {
test.beforeEach(async ({ page }) => {
await page.goto('/')
diff --git a/src/fixtures/playwright.config.ts b/src/fixtures/playwright.config.ts
index 7d4e382171aa..b0d0bbda814a 100644
--- a/src/fixtures/playwright.config.ts
+++ b/src/fixtures/playwright.config.ts
@@ -5,14 +5,8 @@ const CI = Boolean(JSON.parse(process.env.CI || 'false'))
const PLAYWRIGHT_START_SERVER_COMMAND =
process.env.PLAYWRIGHT_START_SERVER_COMMAND || 'npm run start-for-playwright'
-// All of these "patience" related settings follow a simple pattern;
-// If the env var are explicitly set, use that value, otherwise, if
-// we're in CI, be very patient, otherwise, be much less patient.
-// The reasoning is that most engineer laptops are faster than CI
-// and most importantly, if a test gets stuck it's probably not because
-// of a slow CPU, but because the test is plainly wrong. The engineer
-// working on it doesn't want to have to wait half a minute to find out
-// they have a bug in a test action or an assertion.
+// Environment variables override the retry and timeout defaults. CI gets longer waits
+// than local runs, so broken local tests fail quickly instead of waiting on CI-sized timeouts.
const RETRIES = process.env.PLAYWRIGHT_RETRIES ? Number(process.env.PLAYWRIGHT_RETRIES) : CI ? 2 : 0
const TIMEOUT = process.env.PLAYWRIGHT_TIMEOUT
? Number(process.env.PLAYWRIGHT_TIMEOUT)
@@ -25,17 +19,12 @@ const EXPECT_TIMEOUT = process.env.PLAYWRIGHT_EXPECT_TIMEOUT
? 5 * 1000
: 2 * 1000
-/**
- * See https://playwright.dev/docs/test-configuration.
- */
+// See https://playwright.dev/docs/test-configuration.
export default defineConfig({
testDir: './tests',
timeout: TIMEOUT,
expect: {
- /**
- * Maximum time expect() should wait for the condition to be met.
- * For example in `await expect(locator).toHaveText();`
- */
+ // EXPECT_TIMEOUT controls waits such as await expect(locator).toHaveText().
timeout: EXPECT_TIMEOUT,
},
fullyParallel: true,
@@ -46,61 +35,21 @@ export default defineConfig({
: CI
? 1
: undefined,
- /* Reporter to use. See https://playwright.dev/docs/test-reporters */
- // reporter: 'html',
- /* Shared settings for all the projects below. See https://playwright.dev/docs/api/class-testoptions. */
+ // See https://playwright.dev/docs/api/class-testoptions for shared project options.
use: {
- /* Maximum time each action such as `click()` can take. Defaults to 0 (no limit). */
actionTimeout: 0,
baseURL: 'http://localhost:4000',
- /* Collect trace when retrying the failed test. See https://playwright.dev/docs/trace-viewer */
+ // See https://playwright.dev/docs/trace-viewer for trace collection behavior.
trace: 'on-first-retry',
},
projects: [
- // {
- // name: 'chromium',
- // use: {
- // ...devices['Desktop Chrome'],
- // // need this wider width because of our slightly wider than normal xl
- // // breakpoint that helps prevent overlapping main content with the minitoc
- // viewport: {
- // width: 1400,
- // height: 720,
- // },
- // },
- // },
-
- // {
- // name: 'firefox',
- // use: { ...devices['Desktop Firefox'] },
- // },
-
- // {
- // name: 'webkit',
- // use: { ...devices['Desktop Safari'] },
- // },
-
- /* Test against mobile viewports. */
- // {
- // name: 'Mobile Chrome',
- // use: { ...devices['Pixel 5'] },
- // },
- // {
- // name: 'Mobile Safari',
- // use: { ...devices['iPhone 12'] },
- // },
-
- /* Test against branded browsers. */
- // {
- // name: 'Microsoft Edge',
- // use: { channel: 'msedge' },
- // },
{
name: 'Google Chrome',
use: {
channel: 'chromium',
+ // The 1400px width avoids overlap between main content and the mini table of contents.
viewport: {
width: 1400,
height: 720,
@@ -109,9 +58,6 @@ export default defineConfig({
},
],
- /* Folder for test artifacts such as screenshots, videos, traces, etc. */
- // outputDir: 'test-results/',
-
webServer: {
command: PLAYWRIGHT_START_SERVER_COMMAND,
port: 4000,
diff --git a/src/fixtures/tests/annotations.ts b/src/fixtures/tests/annotations.ts
index 87f35190137e..0f9b7122146e 100644
--- a/src/fixtures/tests/annotations.ts
+++ b/src/fixtures/tests/annotations.ts
@@ -8,13 +8,9 @@ describe('annotations', () => {
const $: CheerioAPI = await getDOM('/get-started/foo/code-snippet-with-hashbang')
const annotations = $('#article-contents .annotate')
- // Check http://localhost:4000/en/get-started/foo/code-snippet-with-hashbang
- // to understand the confidence in the assertions.
-
- // This fixture page has 2 bash annotations and 1 yaml
+ // The fixture page intentionally has 2 Bash annotations and 1 YAML annotation.
expect(annotations.length).toBe(2 + 1)
- // First code snippet block
{
const annotation = annotations.eq(0)
expect(annotation.find('.annotate-header').length).toBe(1)
@@ -25,7 +21,6 @@ describe('annotations', () => {
const noteTexts = notes.map((_, el) => $(el).text()).get()
expect(noteTexts).toEqual(["Let's get started", 'This is just a sample', 'End of the script'])
}
- // Second code snippet block
{
const annotation = annotations.eq(1)
expect(annotation.find('.annotate-header').length).toBe(1)
@@ -36,7 +31,7 @@ describe('annotations', () => {
const noteTexts = notes.map((_, el) => $(el).text()).get()
expect(noteTexts).toEqual(['Has to start with a comment.', 'This is the if statement'])
}
- // Yaml code snippet that starts with an empty comment
+ // The YAML snippet starts with an empty comment.
{
const annotation = annotations.eq(2)
expect(annotation.find('.annotate-header').length).toBe(1)
diff --git a/src/fixtures/tests/api-article-body.ts b/src/fixtures/tests/api-article-body.ts
index fb4d3de6d23a..d74f787a25ad 100644
--- a/src/fixtures/tests/api-article-body.ts
+++ b/src/fixtures/tests/api-article-body.ts
@@ -6,15 +6,13 @@ const makeURL = (pathname: string) => `/api/article/body?${new URLSearchParams({
describe('article body api', () => {
beforeAll(() => {
- // If you didn't set the `ROOT` variable, the tests will fail rather
- // cryptically. So as a warning for engineers running these tests,
- // alert in case it was accidentally forgotten.
+ // Missing ROOT makes local fixture failures hard to trace.
if (!process.env.ROOT) {
console.warn(
'WARNING: The articlebody tests require the ROOT environment variable to be set to the fixture root',
)
}
- // Ditto for fixture-based translations to work
+ // Missing TRANSLATIONS_FIXTURE_ROOT breaks fixture-based translations.
if (!process.env.TRANSLATIONS_FIXTURE_ROOT) {
console.warn(
'WARNING: The articlebody tests require the TRANSLATIONS_FIXTURE_ROOT environment variable to be set',
@@ -28,7 +26,7 @@ describe('article body api', () => {
expect(res.headers['content-type']).toContain('text/markdown')
expect(res.body).toContain('## About GitHub')
expect(res.body).toContain('## About Git')
- expect(res.body).toMatch(/^#+\s+\w+/m) // Check for any markdown heading pattern
+ expect(res.body).toMatch(/^#+\s+\w+/m)
expect(res.headers['set-cookie']).toBeUndefined()
expect(res.headers['cache-control']).toContain('public')
@@ -123,7 +121,7 @@ describe('article body api', () => {
})
test('codespaces content included in production markdown API', async () => {
- // Test a real production page that has codespaces content
+ // This production URL exercises real Codespaces tool content when fixtures can reach it.
const res = await get(
makeURL(
'/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/reviewing-proposed-changes-in-a-pull-request',
@@ -144,8 +142,7 @@ describe('article body api', () => {
})
test('verifies original issue #5400 is resolved', async () => {
- // This test specifically addresses the original issue where tool picker
- // content was missing from the Markdown API response
+ // This production URL verifies the Markdown API includes Codespaces tool content.
const res = await get(
makeURL(
'/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/reviewing-proposed-changes-in-a-pull-request',
@@ -162,7 +159,6 @@ describe('article body api', () => {
expect(res.statusCode).toBe(200)
expect(res.headers['content-type']).toContain('text/markdown')
- // The original issue was that only webui content was returned, missing codespaces
expect(res.body).toContain('')
expect(res.body).toContain('')
diff --git a/src/fixtures/tests/breadcrumbs.ts b/src/fixtures/tests/breadcrumbs.ts
index 9559fcd43eef..d53b2fc4bd00 100644
--- a/src/fixtures/tests/breadcrumbs.ts
+++ b/src/fixtures/tests/breadcrumbs.ts
@@ -6,13 +6,11 @@ describe('breadcrumbs', () => {
test('links always prefixed with language', async () => {
const $ = await getDOM('/get-started/start-your-journey/hello-world')
const links = $('[data-testid=breadcrumbs-bar] a')
- // Home and the two ancestors are links; the current article is static text.
+ // The current article is static text, so only Home and two ancestors are links.
expect(links.length).toBe(3)
links.each((i, element) => {
const href = $(element).attr('href')!
- // The Home crumb points at the locale root (`/en` on the default version,
- // no trailing slash); every other crumb is under `/en/…`. Both are
- // language-prefixed, which is what this test guards.
+ // Home uses /en; every other crumb starts with /en/.
expect(href === '/en' || href.startsWith('/en/')).toBe(true)
})
})
@@ -45,7 +43,7 @@ describe('breadcrumbs', () => {
expect(current.text()).toBe('Hello World')
expect(current.is('a')).toBe(false)
expect(current.attr('href')).toBeUndefined()
- // The secondary-bar variant shows the full trail (no hidden last crumb).
+ // The secondary bar shows the full trail, including the last crumb.
expect(current.hasClass('d-none')).toBe(false)
})
diff --git a/src/fixtures/tests/categories-and-subcategory.ts b/src/fixtures/tests/categories-and-subcategory.ts
index 24aa47af6056..36abceb149d5 100644
--- a/src/fixtures/tests/categories-and-subcategory.ts
+++ b/src/fixtures/tests/categories-and-subcategory.ts
@@ -12,12 +12,10 @@ describe('subcategories', () => {
const links = $('[data-testid=table-of-contents] a[href]')
expect(links.length).toBeGreaterThan(0)
- // They all have the same prefix
const hrefs = links.map((i: number, el: Element) => $(el).attr('href')).get()
expect(
hrefs.every((href: string) => href.startsWith('/en/get-started/start-your-journey/')),
).toBeTruthy()
- // They all resolve to a 200 OK without redirects
const responses = await Promise.all(hrefs.map((href: string) => head(href)))
expect(responses.every((r: { statusCode: number }) => r.statusCode === 200)).toBeTruthy()
})
@@ -35,7 +33,7 @@ describe('subcategories', () => {
expect(firstArticleH2.text()).toMatch('Article title')
const firstArticleIntro = $('[data-testid=table-of-contents] p').first()
- // Its HTML in the intro is escaped and Markdown converted
+ // The intro escapes title HTML and converts Markdown.
expect(firstArticleIntro.html()).toMatch(
'This page uses < and > in the title and shortTitle',
)
@@ -50,10 +48,8 @@ describe('categories', () => {
const links = $('[data-testid=table-of-contents] a[href]')
expect(links.length).toBeGreaterThan(0)
- // They all have the same prefix
const hrefs = links.map((i: number, el: Element) => $(el).attr('href')).get()
expect(hrefs.every((href: string) => href.startsWith('/en/actions/category/'))).toBeTruthy()
- // They all resolve to a 200 OK without redirects
const responses = await Promise.all(hrefs.map((href: string) => head(href)))
expect(responses.every((r: { statusCode: number }) => r.statusCode === 200)).toBeTruthy()
})
diff --git a/src/fixtures/tests/footer.ts b/src/fixtures/tests/footer.ts
index 2df4b449b796..ac302a742c55 100644
--- a/src/fixtures/tests/footer.ts
+++ b/src/fixtures/tests/footer.ts
@@ -14,7 +14,7 @@ describe('footer', () => {
})
test('renders minimal 404 page', async () => {
- // 404 pages now render a minimal HTML response without the full layout
+ // Minimal 404 responses omit the full layout.
const $ = await getDOM('/en/delicious-snacks/donuts.php', { allow404: true })
expect($('p').text()).toContain('Page not found.')
})
diff --git a/src/fixtures/tests/glossary.ts b/src/fixtures/tests/glossary.ts
index e214b7927aa4..dab192b44eef 100644
--- a/src/fixtures/tests/glossary.ts
+++ b/src/fixtures/tests/glossary.ts
@@ -17,7 +17,7 @@ describe('glossary', () => {
const $: CheerioAPI = await getDOM('/get-started/learning-about-github/github-glossary')
const internalLink = $('#article-contents a[href="/en/get-started/foo"]')
expect(internalLink.length).toBe(1)
- // That link used AUTOTITLE so it should be "expanded"
+ // AUTOTITLE expands this fixture link to the page title.
expect(internalLink.text()).toBe('Fooing Around')
})
@@ -29,7 +29,6 @@ describe('glossary', () => {
})
test('liquid in one of the description depends on version', async () => {
- // fpt
{
const $: CheerioAPI = await getDOM('/get-started/learning-about-github/github-glossary')
const paragraphs = $('#article-contents p')
@@ -40,7 +39,6 @@ describe('glossary', () => {
expect(paragraphTexts).toContain('status check on HubGit.')
}
- // ghes
{
const $: CheerioAPI = await getDOM(
'/enterprise-server@latest/get-started/learning-about-github/github-glossary',
diff --git a/src/fixtures/tests/head.ts b/src/fixtures/tests/head.ts
index 253f43190423..6eb6ab0fd8d0 100644
--- a/src/fixtures/tests/head.ts
+++ b/src/fixtures/tests/head.ts
@@ -6,10 +6,10 @@ import { getDOM } from '@/tests/helpers/e2etest'
describe('', () => {
test('includes page intro in `description` meta tag', async () => {
const $: CheerioAPI = await getDOM('/get-started/markdown/intro')
- // The intro has Markdown syntax which becomes HTML encoded in the lead element.
+ // The lead renders Markdown syntax as HTML.
const lead = $('[data-testid="lead"] p')
expect(lead.html()).toMatch('syntax')
- // As a meta description its content is stripped of all HTML
+ // Meta descriptions strip all HTML from Markdown-rendered intros.
const description = $('head meta[name="description"]')
expect(description.attr('content')).toBe('This intro has Markdown syntax for HubGit')
})
diff --git a/src/fixtures/tests/homepage.ts b/src/fixtures/tests/homepage.ts
index 8ba9d7ece5e0..db1536eac79a 100644
--- a/src/fixtures/tests/homepage.ts
+++ b/src/fixtures/tests/homepage.ts
@@ -21,7 +21,8 @@ describe('home page', () => {
for (const href of hrefs) {
if (!href.attr('href')?.startsWith('https://')) {
const res = await get(href.attr('href')!)
- expect(res.statusCode).toBe(200) // Not needing to redirect
+ // Product group links resolve without redirects.
+ expect(res.statusCode).toBe(200)
expect(href.text().includes('{%')).toBe(false)
} else {
externalLinks++
diff --git a/src/fixtures/tests/images.ts b/src/fixtures/tests/images.ts
index a1eaaccfec5d..b9aee2601786 100644
--- a/src/fixtures/tests/images.ts
+++ b/src/fixtures/tests/images.ts
@@ -6,15 +6,16 @@ import type { Element } from 'domhandler'
import { get, head, getDOM } from '@/tests/helpers/e2etest'
import { MAX_WIDTH } from '@/content-render/unified/rewrite-asset-img-tags'
-// `getDOM` parses with `xmlMode: true`, which is case-sensitive on attribute
-// names. The legacy string render path emits a lowercase `srcset`, but the
-// React render path (hast -> JSX) emits React 19's camelCase `srcSet`. Both are
-// valid HTML (attribute names are case-insensitive in browsers), so read either.
+// getDOM parses in xmlMode, so attribute names are case-sensitive.
+// The string render path emits srcset, and the React render path emits srcSet.
+// Browsers treat both as valid HTML, so read either spelling.
function srcsetOf(el: Cheerio): string | undefined {
return el.attr('srcset') ?? el.attr('srcSet')
}
describe('render Markdown image tags', () => {
+ // _fixtures/screenshot.png is 2000x1494 and wider than MAX_WIDTH, so picture
+ // sources include mw-XXXXX resizing and preserve aspect ratio at 1076px tall.
test('page with a single image', async () => {
const $: CheerioAPI = await getDOM('/get-started/images/single-image')
@@ -41,16 +42,9 @@ describe('render Markdown image tags', () => {
expect(res.statusCode).toBe(200)
expect(res.headers['content-type']).toBe('image/webp')
- // The fixture image `_fixtures/screenshot.png` is known to be very
- // large. Larger than MAX_WIDTH pixels wide.
- // When transformed as a source in a `` tag, it's automatically
- // injected with the `mw-XXXXX` virtual indicator in the URL that
- // resizes it on-the-fly.
const image = sharp(Buffer.from(res.body as ArrayBuffer))
const { width, height } = await image.metadata()
expect(width).toBe(MAX_WIDTH)
- // The `_fixtures/screenshot.png` is 2000x1494.
- // So if 2000/1494==MAX_WIDTH/x, then x becomes 1494*MAX_WIDTH/2000=1076
expect(height).toBe(Math.round((1494 * MAX_WIDTH) / 2000))
})
@@ -63,9 +57,9 @@ describe('render Markdown image tags', () => {
const sources = $('source', pictures)
expect(sources.length).toBe(3)
- expect(srcsetOf(sources.eq(0))).toContain('1x') // 0
- expect(srcsetOf(sources.eq(1))).toContain('2x') // 1
- expect(srcsetOf(sources.eq(2))).toContain('2x') // 2
+ expect(srcsetOf(sources.eq(0))).toContain('1x')
+ expect(srcsetOf(sources.eq(1))).toContain('2x')
+ expect(srcsetOf(sources.eq(2))).toContain('2x')
})
test('image inside a list keeps its span', async () => {
@@ -77,10 +71,10 @@ describe('render Markdown image tags', () => {
test("links directly to images aren't rewritten", async () => {
const $: CheerioAPI = await getDOM('/get-started/images/link-to-image')
- // There is only 1 link inside that page
- const links = $('#article-contents a[href^="/"]') // exclude header link
+ // The fixture has one article link; header links are out of scope.
+ const links = $('#article-contents a[href^="/"]')
expect(links.length).toBe(1)
- // This proves that the link didn't get rewritten to `/en/...`
+ // Asset links must stay under /assets instead of gaining a language prefix.
expect(links.attr('href'), '/assets/images/_fixtures/screenshot.png')
const res = await head(links.attr('href')!)
expect(res.statusCode).toBe(200)
diff --git a/src/fixtures/tests/internal-links.ts b/src/fixtures/tests/internal-links.ts
index 353dc286168c..e659aebf97dc 100644
--- a/src/fixtures/tests/internal-links.ts
+++ b/src/fixtures/tests/internal-links.ts
@@ -15,13 +15,12 @@ describe('autotitle', () => {
expect($(element).text()).toBe('Hello World')
}
})
- // There are 4 links on the `autotitling.md` content.
+ // autotitling.md has 4 AUTOTITLE links.
expect.assertions(4)
})
test('typos lead to error when NODE_ENV !== production', async () => {
- // The fixture typo-autotitling.md contains two different typos
- // of the word "AUTOTITLE", separated by `{% if version ghes %}`
+ // typo-autotitling.md contains two AUTOTITLE typos split by {% if version ghes %}.
{
const res = await get('/get-started/foo/typo-autotitling', { followRedirects: true })
expect(res.statusCode).toBe(500)
@@ -48,14 +47,14 @@ describe('cross-version-links', () => {
const $: CheerioAPI = await getDOM(URL)
const links = $('#article-contents a[href]')
- // Tests that the hardcoded prefix is always removed
+ // Cross-version links drop hardcoded free-pro-team prefixes.
const firstLink = links.filter(
(i: number, element: Element) =>
$(element).text() === 'Hello world always in free-pro-team',
)
expect(firstLink.attr('href')).toBe('/en/get-started/start-your-journey/hello-world')
- // Tests that the second link always goes to enterprise-server@X.Y
+ // Cross-version links keep explicit enterprise-server targets.
const secondLink = links.filter(
(i: number, element: Element) =>
$(element).text() === 'Autotitling page always in enterprise-server latest',
@@ -79,7 +78,7 @@ describe('link-rewriting', () => {
expect(link.attr('href')).toMatch('/en/get-started/')
}
- // Some links are left untouched
+ // External, asset, public, and enterprise links keep their original prefixes.
{
const link = links.filter((i: number, element: Element) =>
@@ -120,7 +119,7 @@ describe('link-rewriting', () => {
})
test('/en and current version number is injected', async () => {
- // enterprise-server, unlike enterprise-cloud, use numbers
+ // enterprise-server URLs use numbered releases, unlike enterprise-cloud.
const $: CheerioAPI = await getDOM(
'/enterprise-server@latest/get-started/start-your-journey/link-rewriting',
)
diff --git a/src/fixtures/tests/liquid.ts b/src/fixtures/tests/liquid.ts
index af380472590f..78da5ef5b547 100644
--- a/src/fixtures/tests/liquid.ts
+++ b/src/fixtures/tests/liquid.ts
@@ -56,70 +56,44 @@ describe('post', () => {
expect(html).toMatch('HubGit ')
expect(html).toMatch('CramFPTped')
- // Test what happens to `Cram{% ifversion fpt %}FPT{% endif %}ped.`
- // when it's not free-pro-team.
+ // Cram{% ifversion fpt %}FPT{% endif %}ped renders as Cramped outside free-pro-team.
{
const $inner: CheerioAPI = await getDOM(
'/enterprise-server@latest/get-started/liquid/whitespace',
)
const innerHtml = $inner('#article-contents').html()
- // Assures that there's not whitespace left when the `{% ifversion %}`
- // yields an empty string.
+ // Empty ifversion output must not leave extra whitespace.
expect(innerHtml).toMatch('Cramped')
}
})
})
describe('rowheaders', () => {
+ // The first fixture table rewrites the first cell in each of two tbody rows to th,
+ // leaving three td cells per row.
+ // The second fixture table has three tbody rows with three td cells each.
+ // Axe's scope-attr-valid rule requires col scope on thead th and row scope on tbody th.
+ // https://dequeuniversity.com/rules/axe/4.1/scope-attr-valid?application=RuleDescription
test('rowheaders', async () => {
const $: CheerioAPI = await getDOM('/get-started/liquid/table-row-headers')
const tables = $('#article-contents table')
expect(tables.length).toBe(2)
- // The first table should have this structure:
- //
- // table
- // tbody
- // tr
- // th
- // td
- // td
- // td
- //
- // (and there are 2 of these rows)
- //
- // That's because a Liquid + Markdown solution rewrites the
- // *first* `tbody td` to become a `th` instead.
const firstTable = tables.filter((i: number) => i === 0)
expect($('tbody tr th', firstTable).length).toBe(2)
expect($('tbody tr td', firstTable).length).toBe(2 * 3)
- // The second table should have this structure:
- //
- // table
- // tbody
- // tr
- // td
- // td
- // td
- //
- // (and there are 3 of these rows)
const secondTable = tables.filter((i: number) => i === 1)
expect($('tbody tr th', secondTable).length).toBe(0)
expect($('tbody tr td', secondTable).length).toBe(3 * 3)
- // More specifically, the tags should have the appropriate
- // `scope` attribute.
- // See "Scope attribute should be used correctly on tables"
- // https://dequeuniversity.com/rules/axe/4.1/scope-attr-valid?application=RuleDescription
$('thead th', firstTable).each((i, element) => {
expect($(element).attr('scope')).toBe('col')
})
$('tbody th', firstTable).each((i, element) => {
expect($(element).attr('scope')).toBe('row')
})
- // The 5 here is the other `expect(...)` that happens before these
- // two, just above, `expect(...)` inside the `.each(...)` loops.
+ // Start with the five fixed assertions before counting each loop assertion.
let totalAssertions = 5
totalAssertions += $('thead th', firstTable).length
totalAssertions += $('tbody th', firstTable).length
@@ -128,9 +102,7 @@ describe('rowheaders', () => {
})
describe('ifversion', () => {
- // the matchesPerVersion object contains a list of conditions that
- // should match per version tested, but we also operate against it
- // to find out versions that shouldn't match
+ // matchesPerVersion lists expected conditions and also defines the inverse set per version.
const ghesLast = `enterprise-server@${supported[supported.length - 1]}`
const ghesPenultimate = `enterprise-server@${supported[supported.length - 2]}`
const matchesPerVersion: Record = {
@@ -174,12 +146,10 @@ describe('ifversion', () => {
const allConditions = Object.values(matchesPerVersion).flat()
- // this is all conditions that should match for this rendered version
const wantedConditions = allConditions.filter((condition: string) => {
return matchesPerVersion[version].includes(condition)
})
- // this is the inverse of the above, conditions that shouldn't match for this rendered version
const unwantedConditions = allConditions.filter((condition: string) => {
return !matchesPerVersion[version].includes(condition)
})
@@ -197,7 +167,6 @@ describe('ifversion', () => {
describe('misc Liquid', () => {
test('links with liquid from data', async () => {
const $: CheerioAPI = await getDOM('/get-started/liquid/links-with-liquid')
- // The URL comes from variables.product.pricing_url
const url = getDataByLanguage('variables.product.pricing_url', 'en')
if (!url) throw new Error('variable could not be found')
const links = $(`#article-contents a[href="${url}"]`)
@@ -212,10 +181,7 @@ describe('misc Liquid', () => {
})
test('page with tool Liquid tag followed by Markdown', async () => {
- // This test tests Markdown being correctly rendered when the
- // Markdown directly follows a tool tag like `{% linux %}...{% endlinux %}`.
- // The next line immediately after the `{% endlinux %}` should not
- // leave the Markdown unrendered
+ // Markdown must render when it immediately follows a {% linux %}...{% endlinux %} tag.
const $: CheerioAPI = await getDOM('/get-started/liquid/tool-platform-switcher')
const innerHTML = $('#article-contents').html()
expect(innerHTML).not.toMatch('On *this* line is `Markdown` too.')
@@ -227,62 +193,33 @@ describe('data tag', () => {
test('injects data reusables with the right whitespace', async () => {
const $: CheerioAPI = await getDOM('/get-started/liquid/data')
- // This proves that the two injected reusables tables work.
- // CommonMark is finicky if the indentation isn't perfect, so
- // if you don't get exactly 2 tables, something is wrong, and if it's
- // wrong it's most likely because of the leading whitespaces.
+ // Incorrect reusable indentation can break CommonMark parsing, so expect exactly two tables.
expect($('#article-contents table').length).toBe(2)
- // To truly understand this test, you have to see
- // http://localhost:4000/en/get-started/liquid/data to understand it.
- // The page uses `{% data ... %}` within the bodies of bullet points.
- // If the whitespace isn't correct and working, the bullet points
- // would get confused and think the bullet point "body" is a new
- // bullet point on its own.
+ // Data tags inside ordered-list items must not split item bodies into new list items.
expect($('#article-contents ol').length).toBe(3)
expect($('#article-contents ol li').length).toBe(2 + 1 + 2)
- // In the very first bullet point we inject something that multiple
- // linebreaks in it. The source looks like this:
- //
- // 1. Bullet point
- //
- // {% data reusables.injectables.multiple_numbers %}
- //
- // (The code comment itself here has 3 spaces of manual indentation)
- // What's important is that all the expected lines of that reusables
- // stick inside this `ul li` block.
+ // The indented {% data reusables.injectables.multiple_numbers %} call keeps every line in the first list item.
const liText = $('#article-contents ol li').first().text()
expect(liText).toMatch(/Bullet point\nOne\nTwo\nThree\nFour/)
- // The code block uses `{% data ... %}` and it should be indented
- // so that it aligns perfectly with the code block itself.
- // One of the injected data reusables contains multiple lines.
- // It's important that each line from that starts at the far
- // left. No more or less whitespace.
+ // Multi-line code-block reusables start at the far left, with no extra indentation.
const codeBlock = $('#article-contents li pre').text()
expect(codeBlock).toMatch(/^One\n/)
expect(codeBlock).toMatch(/^One\nTwo\n/)
expect(codeBlock).toMatch(/^One\nTwo\nThree\n/)
- // The code block also a reusables that is just one line.
+ // The code block also receives one single-line reusable.
expect(codeBlock).toMatch(/One Two Three Four\n/)
- // On its own, if you look at
- // src/fixtures/fixtures/data/reusables/injectables/paragraphs.md, you'll
- // see each line is NOT prefixed with whitespace indentation.
- // But because `{% data reusables.injectables.paragraphs %}` is
- // inserted with some indentation, that's replicated on every line.
+ // src/fixtures/fixtures/data/reusables/injectables/paragraphs.md inherits indentation from its data call.
const li = $('#article-contents li')
.filter((_, element) => {
return $(element).text().trim().startsWith('Point 1')
})
.eq(0)
- // You can't really test the exact whitespace with cheerio,
- // of the original HTML, but it doesn't actually matter. What
- // matters is that within the bullet point, that starts with "Point 1",
- // it *contains* all the paragraphs
- // from src/fixtures/fixtures/data/reusables/injectables/paragraphs.md.
+ // Cheerio cannot test original HTML whitespace, so the bullet text checks every paragraph.
expect(li.text()).toMatch(/Paragraph one/)
expect(li.text()).toMatch(/Paragraph two/)
expect(li.text()).toMatch(/Paragraph three/)
diff --git a/src/fixtures/tests/markdown.ts b/src/fixtures/tests/markdown.ts
index b7f15b614a3a..cd2a8280532c 100644
--- a/src/fixtures/tests/markdown.ts
+++ b/src/fixtures/tests/markdown.ts
@@ -17,8 +17,7 @@ describe('alerts', () => {
test('basic rendering', async () => {
const $: CheerioAPI = await getDOM('/get-started/markdown/alerts')
const alerts = $('#article-contents .ghd-alert')
- // See src/fixtures/fixtures/content/get-started/markdown/alerts.md
- // to be this confident in the assertions.
+ // src/fixtures/fixtures/content/get-started/markdown/alerts.md defines five alert types.
expect(alerts.length).toBe(5)
const svgs = $('svg', alerts)
expect(svgs.length).toBe(5)
diff --git a/src/fixtures/tests/permissions-callout.ts b/src/fixtures/tests/permissions-callout.ts
index 93cc31cd9d87..e23b9af0f615 100644
--- a/src/fixtures/tests/permissions-callout.ts
+++ b/src/fixtures/tests/permissions-callout.ts
@@ -11,11 +11,7 @@ describe('permission statements', () => {
})
test('callout disappears depend on Liquid inside it', async () => {
- // This page has `product:` property which is a piece of Liquid
- // which makes it so that the rendered output of that becomes
- // an empty string.
- // This test tests that alert is not rendered if its output
- // "exits" but is empty.
+ // Liquid in the product: frontmatter property renders empty, so the product statement disappears.
const $: CheerioAPI = await getDOM(
'/enterprise-server@latest/get-started/foo/page-with-callout',
)
@@ -32,16 +28,13 @@ describe('permission statements', () => {
test('page with permission frontmatter', async () => {
const $: CheerioAPI = await getDOM('/get-started/markdown/permissions')
const html = $('[data-testid=permissions-statement] div').html()
- // Markdown
expect(html).toMatch('admin')
- // Liquid
expect(html).toMatch('HubGit Pages site')
})
test('page with permission frontmatter and product statement', async () => {
const $: CheerioAPI = await getDOM('/get-started/foo/page-with-permissions-and-product-callout')
const html = $('[data-testid=permissions-callout] div').html()
- // part of the UI
expect(html).toMatch('Who can use this feature')
const permission = $('[data-testid=permissions-statement] div')
diff --git a/src/fixtures/tests/playwright-a11y.spec.ts b/src/fixtures/tests/playwright-a11y.spec.ts
index 1c2017dce6ce..8cb28892669d 100644
--- a/src/fixtures/tests/playwright-a11y.spec.ts
+++ b/src/fixtures/tests/playwright-a11y.spec.ts
@@ -7,12 +7,9 @@ const SEARCH_TESTS = !!process.env.ELASTICSEARCH_URL
const pages: { [key: string]: string } = {
category: '/actions/category',
codeAnnotations: '/get-started/markdown/code-annotations',
- // The only fixture page that renders a CTA button. A `.btn-primary` anchor is the
- // one shape the brand article-link override can drive under 4.5:1 — its label sits
- // on a coloured fill rather than the page background — which is exactly what it did
- // before `:not(.btn)` was added to
- // src/frame/stylesheets/article-link-overrides.scss. Without this entry that
- // exclusion has no test at all.
+ // This CTA fixture is the only page that covers the .btn-primary article-link override.
+ // Its filled label can fall below 4.5:1 without the :not(.btn) exclusion in
+ // src/frame/stylesheets/article-link-overrides.scss.
ctaButton: '/get-started/foo/page-with-permissions-and-product-callout',
homepage: '/',
learningPath:
@@ -28,7 +25,6 @@ const pages: { [key: string]: string } = {
tableWithHeaders: '/get-started/liquid/table-row-headers',
}
-// create a test for each page, will eventually be separated into finer grain tests
for (const pageName of Object.keys(pages)) {
test.describe(`${pageName}`, () => {
test('full page axe scan without experiments', async ({ page }) => {
@@ -55,14 +51,12 @@ for (const pageName of Object.keys(pages)) {
})
}
-// The search facet filters collapse behind a "Show filters" disclosure below
-// Primer Brand's `medium` breakpoint. The scans above run at the default desktop
-// viewport, where that disclosure is display:none, so the expanded panel would
+// The search facet filters collapse behind a Show filters disclosure below
+// Primer Brand's medium breakpoint. The scans above run at the default desktop
+// viewport, where that disclosure has display: none, so the expanded panel would
// otherwise never be scanned.
test.describe('search filters (narrow viewport)', () => {
- // Without a local Elasticsearch the middleware proxies to production, so there are no
- // aggregations, the disclosure never renders, and this would time out rather than
- // skip. Matches the guard every search test in playwright-rendering.spec.ts uses.
+ // Without local Elasticsearch, the production proxy returns no aggregations, so the disclosure never renders.
test.skip(!SEARCH_TESTS, 'No local Elasticsearch, no tests involving search')
test('expanded filter disclosure passes axe', async ({ page }) => {
@@ -76,8 +70,7 @@ test.describe('search filters (narrow viewport)', () => {
await toggle.click()
await expect(toggle).toHaveAttribute('aria-expanded', 'true')
- // Scoped to the disclosure's own panel: a bare `fieldset` locator would hit strict
- // mode the moment anything else on the page renders one.
+ // Scope to the panel, because other fieldsets would trigger Playwright strict mode.
const panelId = await toggle.getAttribute('aria-controls')
await expect(page.locator(`#${panelId} fieldset`)).toBeVisible()
diff --git a/src/fixtures/tests/playwright-header.spec.ts b/src/fixtures/tests/playwright-header.spec.ts
index 36261f1d7ffc..8fdeb5661b79 100644
--- a/src/fixtures/tests/playwright-header.spec.ts
+++ b/src/fixtures/tests/playwright-header.spec.ts
@@ -9,35 +9,29 @@ import {
} from '../../frame/lib/constants'
const ARTICLE = '/en/get-started/foo/bar'
-// `find-page.ts` narrows `context.languages` to English alone for early-access
-// pages, which makes this the production route through the single-language
-// branch of the header's language slot.
+// find-page.ts narrows context.languages to English for early-access pages, so
+// this route exercises the single-language branch of the header's language slot.
const ENGLISH_ONLY_ARTICLE = '/en/early-access/secrets/deeper/mariana-trench'
const SEARCH_LABEL = 'Search or ask Copilot'
const LANGUAGE_LABEL = 'Select language: current language is English'
const PLAN_LABEL = 'Select your plan:'
const VERSION_LABEL = 'Select your version:'
-// The pill's line-height is the Docs design's own decision, set in
-// HeaderPicker.module.scss -- Brand's --brand-text-lineHeight-100 is 1.5 -- so
-// unlike the sizes below it is not resolved from a token.
+// The pill's line-height comes from the Docs design, not Brand's
+// --brand-text-lineHeight-100 value of 1.5.
const PILL_LINE_HEIGHT = 1.2
const PLAN_TRIGGER_TESTID = 'version-picker-button'
const LANGUAGE_TRIGGER_TESTID = 'language-picker-button'
-// Brand renders the trailing slot on `trailingComponent != null`, so the wrapper
-// survives a child that renders nothing. Its class name is CSS-module hashed, so
-// only the stable fragment can be matched -- and an absence assertion on a name
-// Brand might rename would pass vacuously, which is why the test below always
-// pairs it with a page where the same selector must still match.
+// Brand renders the trailing slot when trailingComponent != null, so the wrapper
+// survives a child that renders nothing.
+// The CSS module hash leaves only this stable fragment to match; the paired
+// presence test prevents a vacuous absence check after a Brand rename.
const BRAND_TRAILING_SLOT = '[class*="SubdomainNavBar-trailing-component"]'
-/**
- * Resolve Brand custom properties in whatever theme the page is currently in,
- * instead of hardcoding light-mode RGB values. The probe is appended inside
- * `locator` on purpose: the plan menu renders inside its own nested Brand
- * ThemeProvider, so tokens have to be read from within that subtree to reflect
- * the color mode the menu actually paints with. The hidden probe only
- * normalizes CSS color syntax into rgb(); it never styles the UI.
- */
+// Resolve Brand custom properties in the page's current theme instead of
+// hardcoding light-mode RGB values.
+// Append the probe inside locator because the plan menu has its own nested Brand
+// ThemeProvider, so tokens must come from that subtree.
+// The hidden probe normalizes CSS color syntax into rgb() without styling the UI.
async function resolveThemeTokens(locator: Locator, tokens: string[]) {
return locator.evaluate((element, tokenNames: string[]) => {
const probe = document.createElement('span')
@@ -59,12 +53,10 @@ async function resolveThemeTokens(locator: Locator, tokens: string[]) {
}, tokens)
}
-/**
- * Resolve Brand length tokens to pixels, so the pill's geometry can be checked
- * against the tokens it is built from instead of the numbers those tokens happen
- * to produce today. The probe is laid out (absolute + hidden rather than
- * `hidden`) so `width` resolves through calc()/max() to a used pixel value.
- */
+// Resolve Brand length tokens to pixels so the pill geometry stays tied to
+// tokens, not their current numeric values.
+// The absolute hidden probe stays laid out so width resolves through calc() and
+// max() to a used pixel value.
async function resolveTokenPixels(locator: Locator, tokens: string[]) {
return locator.evaluate((element, tokenNames: string[]) => {
const probe = document.createElement('div')
@@ -92,7 +84,7 @@ async function resolveTokenPixels(locator: Locator, tokens: string[]) {
}, tokens)
}
-/** Read raw custom-property values (font weights resolve to plain numbers). */
+// Font weights resolve to plain numbers, so this reads raw custom-property values.
async function resolveTokenValues(locator: Locator, tokens: string[]) {
return locator.evaluate((element, tokenNames: string[]) => {
const resolved: Record = {}
@@ -122,10 +114,7 @@ async function expectHeaderPlanPicker(page: Page) {
expect(valueId).toBeTruthy()
await expect(button).toHaveAttribute('aria-labelledby', `${labelId} ${valueId}`)
- // Every size below is arithmetic over Brand tokens, so resolve the tokens and
- // derive the expectations rather than hardcoding today's pixels: a
- // @primer/react-brand bump that moves --base-size-* then updates both sides at
- // once, instead of failing CI with no user-visible regression.
+ // Resolve Brand tokens so expected sizes move with --base-size-* changes instead of failing.
const sizes = await resolveTokenPixels(picker, [
'--brand-text-size-100',
'--base-size-2',
@@ -197,8 +186,7 @@ async function expectHeaderPlanPicker(page: Page) {
expect(buttonBox.x - (labelBox.x + labelBox.width)).toBeCloseTo(labelGap, 0)
expect(labelBox.y + labelBox.height / 2).toBeCloseTo(buttonBox.y + buttonBox.height / 2, 0)
expect(buttonBox.height).toBeCloseTo(pillHeight, 0)
- // The normal plan name must fit even with Signup visible at 1012px. Keep
- // ellipsis available for unusually long labels, not this default English one.
+ // Default English plan name must fit with Signup at 1012px; ellipsis is for longer labels.
await expect
.poll(() => value.evaluate((element) => element.scrollWidth - element.clientWidth))
.toBeLessThanOrEqual(0)
@@ -217,16 +205,12 @@ async function expectHeaderPlanPicker(page: Page) {
await expectFilledTriangleCaret(button, colors.text)
}
-/**
- * Both header triggers end in the same caret, so both are checked the same way.
- * The design's caret is a filled triangle. Brand's ActionMenu.Button hardcodes a
- * ChevronDownIcon and only loses to a caller-supplied trailingVisual because it
- * spreads rest props after that default -- a single shared cast (ActionMenuTrigger)
- * relies on that. A Brand upgrade that destructures trailingVisual would silently
- * restore the chevron on both controls at once, so assert the chevron is gone and
- * that the glyph really has the triangle's geometry: the triangle's path is
- * ~7.15 x 3.82 user units, where chevron-down's is ~9.56 x 5.31.
- */
+// Both header triggers use the same filled triangle caret, checked through one helper.
+// Brand's ActionMenu.Button defaults to ChevronDownIcon; the ActionMenuTrigger
+// cast relies on a caller-supplied trailingVisual overriding it.
+// A Brand change that destructures trailingVisual would restore chevrons on both controls.
+// Assert the chevron is gone and the triangle path is about 7.15 by 3.82 user
+// units, not chevron-down's 9.56 by 5.31.
async function expectFilledTriangleCaret(trigger: Locator, color: string) {
const caret = trigger.locator('svg.octicon-triangle-down')
await expect(caret).toBeVisible()
@@ -244,13 +228,10 @@ async function expectFilledTriangleCaret(trigger: Locator, color: string) {
expect(glyph.height).toBeLessThan(4.6)
}
-/**
- * The language trigger deliberately does *not* match the plan pill: Figma draws
- * it as a flat control -- a 16px globe, the language in muted 14px regular, then
- * the same filled caret. Only the dropdown below it is shared, so this asserts
- * the trigger keeps its own treatment and never drifts into the pill (which is
- * exactly what reusing the shared pill class would do).
- */
+// The language trigger deliberately does not match the plan pill.
+// Figma specifies a flat control: 16px globe, muted 14px regular language text,
+// then the same filled caret.
+// Only the dropdown is shared, so this catches accidental reuse of the shared pill class.
async function expectHeaderLanguageTrigger(page: Page) {
const picker = page.getByTestId('desktop-header').getByTestId('language-picker')
const trigger = picker.getByTestId(LANGUAGE_TRIGGER_TESTID)
@@ -272,10 +253,7 @@ async function expectHeaderLanguageTrigger(page: Page) {
)
expect(valueFontSize).toBeCloseTo(sizes['--brand-text-size-100'], 1)
- // Flat, not a pill: no fill at rest, no border, and a small corner rather than
- // the pill's full radius. The canvas-subtle comparison keeps this honest -- it
- // is the fill the pill carries and the fill this control only takes on hover
- // and while open.
+ // Flat trigger: no rest fill or border, a 6px corner, and canvas-subtle on hover or open.
await expect(trigger).toHaveCSS('background-color', 'rgba(0, 0, 0, 0)')
expect(tokens['--brand-color-canvas-subtle']).not.toBe('rgba(0, 0, 0, 0)')
for (const side of ['top', 'right', 'bottom', 'left']) {
@@ -285,9 +263,7 @@ async function expectHeaderLanguageTrigger(page: Page) {
await expect(trigger).toHaveCSS(`border-${corner}-radius`, '6px')
}
const triggerBox = (await trigger.boundingBox())!
- // Brand's ActionMenu remaps --brand-borderRadius-medium to the full radius on
- // its own trigger, so a 6px corner is the difference between this control and
- // a pill rather than a cosmetic detail.
+ // Brand's ActionMenu remaps --brand-borderRadius-medium to full radius; 6px prevents a pill.
expect(triggerBox.height / 2).toBeGreaterThan(6)
const globe = trigger.locator('svg.octicon-globe')
@@ -301,29 +277,25 @@ async function expectHeaderLanguageTrigger(page: Page) {
await expectFilledTriangleCaret(trigger, tokens['--brand-color-text-muted'])
}
-/**
- * The two header dropdowns are the same control with different content: both are
- * Brand ActionMenus whose surface and rows come entirely from the shared
- * HeaderPicker.module.scss. Every design assertion below therefore runs against
- * both -- that is what proves they are identical rather than merely similar --
- * so only the content is parameterized here.
- */
+// The two header dropdowns use the same Brand ActionMenu surface and row styles
+// from HeaderPicker.module.scss.
+// Running each design assertion against both menus proves shared styling, not similar styling.
type HeaderDropdown = {
name: string
pickerTestId: string
triggerTestId: string
- /** The span each row wraps its label in. */
+ // The span each row wraps its label in.
itemTestId: string
expectTrigger: (page: Page) => Promise
- /** The row that opens already chosen: tinted, with the trailing green dot. */
+ // The row that opens already chosen: tinted, with the trailing green dot.
selectedRow: string
- /** Another selectable row: no tint, no dot. */
+ // Another selectable row: no tint, no dot.
unselectedRow: string
- /** Rows that navigate instead of selecting, so they stay plain menuitems. */
+ // Rows that navigate instead of selecting, so they stay plain menuitems.
navigationRowCount: number
- /** The plan menu keeps one rule between its versions and its navigation rows. */
+ // The plan menu keeps one rule between its versions and its navigation rows.
separatorCount: number
- /** The final row -- whatever a clipped menu loses first. */
+ // A clipped menu loses this final row first.
lastRowRole: 'menuitem' | 'menuitemradio'
lastRowName: RegExp
}
@@ -358,11 +330,8 @@ const LANGUAGE_DROPDOWN: HeaderDropdown = {
lastRowName: /日本語/,
}
-/**
- * A Docs 2026 header dropdown, rebuilt on Brand's ActionMenu. Opens the menu,
- * checks the surface, rows, selection indicator and the absence of Brand's own
- * leading check slot, then closes it and confirms focus returns to the trigger.
- */
+// Docs 2026 rebuilds header dropdowns on Brand ActionMenu, so this helper checks
+// the shared menu contract end to end.
async function expectHeaderDropdownDesign(
page: Page,
colorScheme: 'light' | 'dark',
@@ -374,7 +343,7 @@ async function expectHeaderDropdownDesign(
await trigger.click()
await expect(trigger).toHaveAttribute('aria-expanded', 'true')
- // Brand's menu is not portalled -- it renders inside the picker wrapper.
+ // Brand's menu renders inside the picker wrapper, not a portal.
const menu = picker.getByRole('menu')
await expect(menu).toBeVisible()
@@ -385,8 +354,7 @@ async function expectHeaderDropdownDesign(
'--brand-color-text-default',
'--brand-color-success-fg',
])
- // Proves the emulated scheme reached Brand's tokens: a dark run that silently
- // stayed light would satisfy every assertion above on its own.
+ // A dark run that stays light would pass above, so verify Brand tokens changed.
const luminance = relativeLuminance(tokens['--brand-color-canvas-default'])
if (colorScheme === 'dark') {
expect(luminance).toBeLessThan(0.2)
@@ -394,8 +362,7 @@ async function expectHeaderDropdownDesign(
expect(luminance).toBeGreaterThan(0.8)
}
- // Menu surface: canvas-default fill, 1px subtle border, 6px radius, 8px pad.
- // Brand's own defaults are a border-muted border and a 16px radius.
+ // Overrides Brand's border-muted border and 16px radius; assertions also pin fill and 8px pad.
await expect(menu).toHaveCSS('background-color', tokens['--brand-color-canvas-default'])
for (const side of ['top', 'right', 'bottom', 'left']) {
await expect(menu).toHaveCSS(`border-${side}-width`, '1px')
@@ -406,13 +373,10 @@ async function expectHeaderDropdownDesign(
for (const corner of ['top-left', 'top-right', 'bottom-left', 'bottom-right']) {
await expect(menu).toHaveCSS(`border-${corner}-radius`, '6px')
}
- // The design's menu is 256px wide; a long row may grow it, never shrink it.
+ // The design sets a 256px minimum menu width; long rows can grow it, never shrink it.
const menuBox = (await menu.boundingBox())!
expect(menuBox.width).toBeGreaterThanOrEqual(256)
- // Brand anchors with `allowOutOfBounds`, so nothing clamps a menu that would
- // overhang -- which matters most for the language menu, the one control sitting
- // at the header's right edge. `menuAlignment` is what keeps it on screen, so
- // assert the result instead of trusting the prop.
+ // Brand allowOutOfBounds can overhang the right-edge menu; menuAlignment keeps it on screen.
const viewportWidth = page.viewportSize()!.width
expect(menuBox.x).toBeGreaterThanOrEqual(-1)
expect(menuBox.x + menuBox.width).toBeLessThanOrEqual(viewportWidth + 1)
@@ -420,16 +384,10 @@ async function expectHeaderDropdownDesign(
const selectableRows = menu.getByRole('menuitemradio')
const navigationRows = menu.getByRole('menuitem')
expect(await selectableRows.count()).toBeGreaterThanOrEqual(2)
- // In the plan menu "All Enterprise Server releases" and "About versions"
- // navigate rather than select, so they stay plain menuitems. The language menu
- // has no such rows.
+ // In the plan menu, All Enterprise Server releases and About versions stay navigation menuitems.
await expect(navigationRows).toHaveCount(dropdown.navigationRowCount)
- // A single rule divides the versions from those two navigation rows. Brand has
- // no divider child, so the picker renders the separator itself; it must not be
- // focusable, and must be neither the first nor the last row, because Brand
- // focuses the first and wires its arrow-key wrap-around to the first and
- // the last. The language menu divides nothing, so it carries no separator.
+ // Brand lacks a divider child; keep the separator unfocusable and outside arrow-key wrap ends.
const separator = menu.locator('[role="separator"]')
await expect(separator).toHaveCount(dropdown.separatorCount)
if (dropdown.separatorCount > 0) {
@@ -462,8 +420,7 @@ async function expectHeaderDropdownDesign(
expect(rule.previousRole).toBe('menuitemradio')
expect(rule.nextRole).toBe('menuitem')
expect(rule.nextText).toMatch(/All Enterprise Server releases/)
- // A plain is a block box, so the rule spans the menu's inner width
- // rather than sitting inside a row's own 12px insets.
+ // A block li spans the menu's inner width instead of a row's 12px insets.
expect(rule.width).toBeCloseTo(rule.innerWidth, 0)
expect(rule.marginTop).toBeCloseTo(8, 0)
expect(rule.marginBottom).toBeCloseTo(8, 0)
@@ -476,8 +433,7 @@ async function expectHeaderDropdownDesign(
const row = rows.nth(index)
expect((await row.boundingBox())!.height).toBeCloseTo(32, 0)
await expect(row).toHaveCSS('padding-left', '12px')
- // The reserved indicator column replaces Brand's 48px single-selection
- // gutter: a 12px inset, the 16px dot, then a 12px gap before the label.
+ // The indicator column reserves 12px, a 16px dot and a 12px gap, replacing Brand's 48px gutter.
await expect(row).toHaveCSS('padding-right', '40px')
for (const corner of ['top-left', 'top-right', 'bottom-left', 'bottom-right']) {
await expect(row).toHaveCSS(`border-${corner}-radius`, '6px')
@@ -509,10 +465,7 @@ async function expectHeaderDropdownDesign(
expect(selectedBox.x + selectedBox.width - (dotBox.x + dotBox.width)).toBeCloseTo(12, 0)
expect(dotBox.y + dotBox.height / 2).toBeCloseTo(selectedBox.y + selectedBox.height / 2, 0)
- // Brand renders a leading check slot on every row of a single-selection menu;
- // the design marks the current row with the trailing dot instead. Assert the
- // rendered result rather than Brand's hashed class names: the selected row's
- // only visible glyph is the dot.
+ // The selected row's only visible glyph must be the trailing dot, not Brand's leading check slot.
await expect(selectedRow.locator('svg.octicon-check')).not.toBeVisible()
const visibleGlyphs = await selectedRow
.locator('svg')
@@ -521,8 +474,7 @@ async function expectHeaderDropdownDesign(
)
expect(visibleGlyphs).toHaveLength(1)
expect(visibleGlyphs[0]).toContain('octicon-dot-fill')
- // When Brand renders that slot it must be hidden outright. Written so a future
- // Brand release that stops rendering it altogether does not fail the suite.
+ // Accept a missing leading slot so Brand can remove it without failing this suite.
const leadingSlotDisplay = await selectedRow.evaluate((row) => {
const first = row.firstElementChild
return first && row.children.length > 1 ? getComputedStyle(first).display : null
@@ -543,8 +495,7 @@ async function expectHeaderDropdownDesign(
for (let index = 0; index < dropdown.navigationRowCount; index++) {
const extra = navigationRows.nth(index)
- // axe rejects aria-checked on role=menuitem, so the extras must opt out of
- // the selection semantics ActionMenu.Overlay injects into its children.
+ // axe rejects aria-checked on menuitem, so navigation rows opt out of selection semantics.
await expect(extra).not.toHaveAttribute('aria-checked')
await expect(extra.locator('svg.octicon-dot-fill')).toHaveCount(0)
}
@@ -555,9 +506,8 @@ async function expectHeaderDropdownDesign(
await expect(trigger).toBeFocused()
}
-// The properties a shared stylesheet is supposed to fix identically for both
-// dropdowns. Content-dependent geometry (the menu's used width, a row's text) is
-// deliberately absent: only the styling has to match.
+// The shared stylesheet must fix these properties identically for both dropdowns.
+// Content-dependent geometry is absent; only styling has to match.
const SURFACE_PROPERTIES = [
'background-color',
'min-width',
@@ -593,14 +543,11 @@ const LABEL_PROPERTIES = [
]
const DOT_PROPERTIES = ['position', 'right', 'width', 'height', 'fill']
-/**
- * A style fingerprint of an open header dropdown: the surface, the selected row,
- * its label and its trailing dot. Two dropdowns whose styling really does come
- * from one shared module produce equal fingerprints -- which is a stronger claim
- * than each one separately matching the design, and it is the claim the user
- * actually made ("the language dropdown needs to look like the version
- * dropdown").
- */
+// An open header dropdown fingerprint covers the surface, selected row, label
+// and trailing dot.
+// Equal fingerprints prove the two menus share styling, not merely that each matches the design.
+// This tests the user-visible request: the language dropdown needs to look like
+// the version dropdown.
async function dropdownStyleFingerprint(menu: Locator, dropdown: HeaderDropdown) {
const selectedRow = menu.getByRole('menuitemradio', { name: dropdown.selectedRow, exact: true })
const read = (locator: Locator, properties: string[]) =>
@@ -614,8 +561,7 @@ async function dropdownStyleFingerprint(menu: Locator, dropdown: HeaderDropdown)
row: await read(selectedRow, ROW_PROPERTIES),
label: await read(selectedRow.getByTestId(dropdown.itemTestId), LABEL_PROPERTIES),
dot: await read(selectedRow.locator('svg.octicon-dot-fill'), DOT_PROPERTIES),
- // Brand's leading check slot is hidden structurally, so it has to be hidden
- // in both menus or one of them grows a check icon the other does not have.
+ // Structural hiding must match so one menu cannot grow a Brand check icon the other lacks.
leadingSlotDisplay: await selectedRow.evaluate((row) => {
const first = row.firstElementChild
return first && row.children.length > 1 ? getComputedStyle(first).display : null
@@ -644,8 +590,7 @@ async function expectDesktopHeaderSections(page: Page, signupVisible: boolean) {
const search = element.querySelector('[data-testid="toggle-search"]')!
const language = element.querySelector('[data-testid="language-picker"]')!
const signup = element.querySelector('[data-testid="header-signup"]')
- // Find the native section wrappers from stable Docs control anchors, not
- // Brand's private CSS class names or a hardcoded number of parent hops.
+ // Find section wrappers from stable Docs anchors, not Brand CSS hashes or parent-hop counts.
let sectionRow = search.parentElement!
while (!sectionRow.contains(language)) sectionRow = sectionRow.parentElement!
const sectionFor = (control: HTMLElement) => {
@@ -720,8 +665,7 @@ async function expectDesktopHeaderSections(page: Page, signupVisible: boolean) {
expect(section.rect.top).toBeCloseTo(layout.header.top, 0)
expect(section.rect.bottom).toBeCloseTo(layout.contentBottom, 0)
}
- // Search owns the full-height divider before Language. Language must not
- // double that border; Signup owns its own separate full-height left divider.
+ // Search owns the divider before Language; Signup owns its own left divider.
expect(layout.search.borderEnd).toBe('1px')
expect(layout.search.borderEndStyle).toBe('solid')
expect(layout.search.borderEndColor).not.toBe('rgba(0, 0, 0, 0)')
@@ -744,23 +688,21 @@ async function expectDocsSearchOpen(page: Page) {
await searchInput.click()
await expect(searchInput).toBeFocused()
await expect(page.getByRole('dialog')).toHaveCount(1)
- // Brand mounts its native dialog even while closed. Only the existing Docs
- // dialog may become modal; opening both would leave competing focus traps.
+ // Only Docs search may become modal; opening Brand's closed native dialog would add a focus trap.
const brandDialog = page.getByTestId('desktop-header').locator('dialog')
await expect(brandDialog).toHaveCount(1)
await expect(brandDialog).toHaveJSProperty('open', false)
await expect(page).toHaveURL((url) => url.searchParams.get('search-overlay-open') === 'true')
}
+// expectBackgroundIsolated includes Brand's skip link because it sits outside
+// the inert wrapper as a sibling before header, yet still targets #main-content
+// while the menu is open.
+// CSS avoids getByText strict-mode matches from the wrapped label and getByRole
+// misses after aria-hidden.
async function expectBackgroundIsolated(page: Page, isolated: boolean) {
for (const locator of [
page.getByText('Skip to main content', { exact: true }),
- // Brand's own skip link sits outside the inert wrapper (it renders as a
- // sibling before ) yet still targets #main-content, which is inert
- // while the menu is open. Matched by CSS rather than text or role: Brand
- // wraps the label in a span, so getByText resolves to both the and that
- // span -- a strict mode violation -- and aria-hidden removes it from the
- // accessibility tree that getByRole searches once isolated.
page.locator('[data-container="header"] a[href="#main-content"]'),
page.locator('#main-content'),
page.getByTestId('sidebar-mobile-toggle'),
@@ -777,8 +719,7 @@ async function expectBackgroundIsolated(page: Page, isolated: boolean) {
test.describe('Brand header', () => {
test.beforeEach(async ({ page }) => {
- // These regressions cover header coordination, not remote search quality.
- // Return empty suggestions so they also run without Elasticsearch or Copilot.
+ // Empty suggestions keep header coordination tests independent of Elasticsearch and Copilot.
await page.route('**/api/search/combined-search/v1?**', (route) =>
route.fulfill({
json: {
@@ -810,32 +751,18 @@ test.describe('Brand header', () => {
await page.reload()
}
- // Wait for account detection/desktop slots before measuring the pill:
- // Signup mounting must not shrink a name that only fit before hydration.
+ // Wait for account detection; Signup can mount after hydration and shrink the plan name.
await expectDesktopHeaderSections(page, !hasAccount)
await expectHeaderPlanPicker(page)
- // 1012px is where the two triggers compete for room with Signup, so it is
- // also where the flat language control is most likely to be "fixed" by
- // giving it the pill's class.
+ // At 1012px, Signup pressure exposes accidental pill styling on the language trigger.
await expectHeaderLanguageTrigger(page)
})
}
}
- /**
- * Brand renders its trailing slot whenever `trailingComponent` is not null,
- * so a `LanguagePicker` that returned `null` from inside the slot would still
- * leave the wrapper behind: an empty divided cell at the header's right edge
- * on desktop, and a full-width 16px-padded block in the narrow menu. Header.tsx
- * therefore withholds the prop itself rather than letting the picker opt out,
- * and that decision is invisible to every other test here -- they all run on
- * multi-language pages, where the slot is supposed to be present.
- *
- * Each absence is paired with the same assertion on a multi-language page.
- * Brand's class name is hashed, so `BRAND_TRAILING_SLOT` on its own would keep
- * passing the day Brand renames it; proving the selector still matches
- * something is what stops this from becoming a test of nothing.
- */
+ // Header.tsx omits trailingComponent because Brand keeps wrapper if LanguagePicker returns null.
+
+ // The multi-language assertion keeps BRAND_TRAILING_SLOT from passing after a Brand class rename.
test('the language slot is omitted, not left empty, when only English is available', async ({
page,
}) => {
@@ -849,15 +776,12 @@ test.describe('Brand header', () => {
await page.goto(ENGLISH_ONLY_ARTICLE)
await turnOffExperimentsInPage(page)
const header = page.getByTestId('desktop-header')
- // The plan picker still renders here, so an empty header would fail this
- // rather than passing as a trivially absent language control.
+ // Assert the plan picker first so an empty header cannot pass the absence checks below.
await expect(header.getByRole('button', { name: PLAN_LABEL, exact: false })).toBeVisible()
await expect(page.getByTestId('language-picker')).toHaveCount(0)
await expect(header.locator(BRAND_TRAILING_SLOT)).toHaveCount(0)
- // Independently of Brand's class names: every divided cell in the header's
- // section row still holds a control. An empty slot is exactly a cell that
- // does not, and it would carry its own gridline and margin.
+ // Every divided header cell must hold a control; an empty slot would add a gridline and margin.
await page.evaluate(() => document.fonts.ready)
await expect(async () => {
const sections = await header.evaluate((element) => {
@@ -882,8 +806,7 @@ test.describe('Brand header', () => {
expect(sections.lastReachesEdge).toBe(true)
}).toPass()
- // The narrow menu is where the leftover wrapper would be most visible: a
- // full-width padded block above Sign up rather than a thin cell.
+ // The narrow menu exposes a leftover wrapper as a full-width padded block above Sign up.
await page.setViewportSize({ width: 390, height: 800 })
await page.getByRole('button', { name: 'Menu', exact: true }).click()
await expect(page.getByTestId('header-signup')).toBeVisible()
@@ -897,38 +820,21 @@ test.describe('Brand header', () => {
page,
}) => {
await page.setViewportSize({ width: 1440, height: 800 })
- // No color_mode cookie, so colorModeScript resolves `auto` from this
- // emulation. Set before navigating so the first paint already uses it.
+ // Emulate color before navigation so colorModeScript resolves auto without a cookie.
await page.emulateMedia({ colorScheme })
await page.goto(ARTICLE)
await turnOffExperimentsInPage(page)
- // Each trigger resolves every color through tokens, so both are worth
- // re-checking in dark mode rather than only in the light-mode loop above.
- // The two triggers are intentionally different -- a filled pill for the
- // plan, a flat control for the language -- which is why only the dropdown
- // below them is shared.
+ // Recheck both token-based triggers in dark mode; only the dropdown below them is shared.
await dropdown.expectTrigger(page)
await expectHeaderDropdownDesign(page, colorScheme, dropdown)
})
}
}
- /**
- * The sticky ladder: header > Docs 2026 secondary bar > sticky table headers.
- *
- * Brand's ActionMenu is not portalled, so the plan and language dropdowns
- * render inside the header's stacking context and hang well below it, across
- * the secondary bar. The bar is sticky at every width and sits above sticky
- * table headers, so if the header does not outrank the bar, the bar paints a
- * band straight through the open menu and eats the clicks behind it -- which
- * is invisible to every other test here, because the menu still has the right
- * geometry, styling and roles while being covered.
- *
- * Asserted by hit-testing rather than by comparing z-index values: equal
- * z-index is resolved by DOM order, so the numbers alone do not say which
- * element a reader actually reaches.
- */
+ // The unportalled ActionMenu overlaps the sticky secondary bar, so the header must outrank it.
+
+ // Hit test overlap because DOM-order z-index ties and blocked clicks do not change geometry.
test('an open dropdown stays clickable where the secondary bar crosses it', async ({ page }) => {
await page.setViewportSize({ width: 1440, height: 800 })
await page.emulateMedia({ colorScheme: 'light' })
@@ -946,7 +852,6 @@ test.describe('Brand header', () => {
const b = bar.getBoundingClientRect()
const m = menuEl.getBoundingClientRect()
const crosses = m.bottom > b.top && m.top < b.bottom
- // Sample the full height of the band the two share.
const x = m.left + m.width / 2
const top = Math.max(m.top, b.top) + 2
const bottom = Math.min(m.bottom, b.bottom) - 2
@@ -955,7 +860,7 @@ test.describe('Brand header', () => {
const el = document.elementFromPoint(x, y)
if (!el || !el.closest('[role="menu"]')) covered.push(Math.round(y))
}
- // A row the bar crosses must receive its own clicks, not just paint above.
+ // A crossed row must receive clicks, not merely paint above the bar.
const row = [
...document.querySelectorAll('[data-testid="version-picker"] [role="menuitemradio"]'),
].find((candidate) => {
@@ -979,8 +884,7 @@ test.describe('Brand header', () => {
}
})
- // If the menu stopped overlapping the bar, this test would pass while
- // asserting nothing, so require the overlap it exists to check.
+ // Require actual overlap so this cannot pass after the menu stops crossing the bar.
expect(overlap.barFound).toBe(true)
expect(overlap.crosses).toBe(true)
expect(overlap.covered).toEqual([])
@@ -994,8 +898,7 @@ test.describe('Brand header', () => {
await page.goto(ARTICLE)
await turnOffExperimentsInPage(page)
- // Opened one at a time: Brand closes a menu as soon as the other trigger is
- // clicked, and both menus read their tokens from the same page and theme.
+ // Open one menu at a time because Brand closes the first; both read the same page theme.
const fingerprints: Record = {}
for (const dropdown of [PLAN_DROPDOWN, LANGUAGE_DROPDOWN]) {
const picker = page.getByTestId('desktop-header').getByTestId(dropdown.pickerTestId)
@@ -1009,13 +912,11 @@ test.describe('Brand header', () => {
expect(fingerprints[LANGUAGE_DROPDOWN.name]).toEqual(fingerprints[PLAN_DROPDOWN.name])
})
- // Below 1012px both pickers move inside SubdomainNavBar's narrow menu, which is a
- // scrolling panel. Brand's ActionMenu is absolutely positioned and — unlike the
- // @primer/react menu it replaced — is not portalled, so it regresses easily into
- // rendering outside that panel: cut off mid-list, or running past the viewport's
- // right edge. Both of those still satisfy toBeVisible(), so assert geometry. The
- // inline-flow rule that fixes it now lives in the shared module, so a change to it
- // moves both dropdowns at once and both are covered here.
+ // Below 1012px, SubdomainNavBar's scrolling narrow menu contains both pickers.
+
+ // Geometry catches an unportalled ActionMenu outside the panel while toBeVisible still passes.
+
+ // The shared module owns the inline-flow rule, so both dropdowns must prove the geometry.
for (const dropdown of [PLAN_DROPDOWN, LANGUAGE_DROPDOWN]) {
for (const width of [390, 1000]) {
test(`the ${dropdown.name} dropdown stays inside the narrow menu at ${width}px`, async ({
@@ -1033,7 +934,7 @@ test.describe('Brand header', () => {
await expect(page.getByRole('menu')).toBeVisible()
const layout = await page.getByRole('menu').evaluate((element) => {
- // The panel is found by its scrolling, not by Brand's hashed class name.
+ // Find the panel by scrolling behavior, not Brand's hashed class name.
let panel = element.parentElement
while (panel) {
const { overflowX, overflowY } = getComputedStyle(panel)
@@ -1053,11 +954,11 @@ test.describe('Brand header', () => {
})
expect(layout.panel).not.toBeNull()
- // Inside the panel, so no row is cut off...
+ // The menu stays inside the panel so no row gets cut off.
expect(layout.menu.bottom).toBeLessThanOrEqual(layout.panel.bottom + 1)
expect(layout.menu.right).toBeLessThanOrEqual(layout.panel.right + 1)
expect(layout.lastRowBottom).toBeLessThanOrEqual(layout.panel.bottom + 1)
- // ...and inside the viewport, so no row is sliced by the screen edge.
+ // The menu stays inside the viewport so no row is sliced by the screen edge.
expect(layout.menu.left).toBeGreaterThanOrEqual(-1)
expect(layout.menu.right).toBeLessThanOrEqual(layout.viewportWidth + 1)
expect(layout.scrollsHorizontally).toBe(false)
@@ -1075,13 +976,9 @@ test.describe('Brand header', () => {
}
}
- // Brand staggers the narrow menu's items in at 80ms per slot and hardcodes the
- // signup CTA's wrapper to slot 10 -- the moment ten `SubdomainNavBar.Link`
- // children would have finished cascading in. Docs passes zero links, so the
- // shipped 800ms is a dead second: the pickers ride the panel's fade and
- // "Sign up" trails them. Header.module.scss cuts it to a single slot, so assert
- // the computed delay rather than a wall clock, and assert that only the delay
- // moved -- duration and fill mode still have to be Brand's.
+ // Brand assigns signup to stagger slot 10, but Docs passes zero SubdomainNavBar.Link children.
+
+ // Header.module.scss cuts the 800ms delay to one 80ms slot; duration and fill mode stay Brand's.
test('signup follows the narrow menu pickers by one stagger step, not ten', async ({ page }) => {
await page.setViewportSize({ width: 390, height: 800 })
await page.goto(ARTICLE)
@@ -1090,9 +987,7 @@ test.describe('Brand header', () => {
const signup = page.getByTestId('header-signup')
await expect(signup).toBeVisible()
const animation = await signup.evaluate((element) => {
- // Brand hashes this class and exposes no test id for it, so match the
- // stable part of the name -- the same anchor the override in
- // Header.module.scss uses.
+ // Brand hashes class names, so match the stable SubdomainNavBar-button-area--visible part.
const area = element.closest('[class*="SubdomainNavBar-button-area--visible"]')
if (!area) throw new Error('Signup is not inside the narrow-menu button area')
const { animationDelay, animationDuration, animationFillMode } = getComputedStyle(area)
@@ -1103,7 +998,7 @@ test.describe('Brand header', () => {
}
})
- // Brand's untouched default is calc(10 * 80ms).
+ // Brand's untouched default delay equals 10 * 80ms.
expect(animation.delay).not.toBeCloseTo(0.8, 3)
// Still staggered after the pickers, but by one 80ms slot rather than ten.
expect(animation.delay).toBeGreaterThan(0)
@@ -1186,8 +1081,7 @@ test.describe('Brand header', () => {
'open',
false,
)
- // PRC restores focus during mousedown capture; the browser then transfers it
- // to the clicked backdrop. Persistent return focus is an Escape contract only.
+ // PRC restores focus on mousedown, but backdrop click moves it; Escape owns return focus.
await expect(searchTrigger).toBeVisible()
await expect(searchTrigger).toBeEnabled()
})
@@ -1197,7 +1091,7 @@ test.describe('Brand header', () => {
}) => {
await page.goto(ARTICLE)
await expect(page.getByTestId('toggle-search')).toBeVisible()
- // Use real DOM fields without depending on survey or search results data.
+ // Real DOM fields avoid survey or search-results data dependencies.
await page.locator('#main-content').evaluate((main) => {
const fields = document.createElement('div')
fields.innerHTML = `
@@ -1471,8 +1365,7 @@ test.describe('Brand header', () => {
const picker = page.getByTestId('desktop-header').getByTestId('version-picker')
const button = picker.getByRole('button')
const value = (await button.getByTestId('field').textContent())!
- // versionTitle is `${planTitle} ${release}` for a numbered release, so the
- // plan label would announce "Select your plan: Enterprise Server 3.19".
+ // A numbered release uses the version label instead of the plan label.
expect(value).toMatch(/^Enterprise Server [\d.]+$/)
await expect(picker.getByText(VERSION_LABEL, { exact: true })).toBeVisible()
await expect(button).toHaveAccessibleName(`${VERSION_LABEL} ${value}`)
diff --git a/src/fixtures/tests/playwright-secret-scanning.spec.ts b/src/fixtures/tests/playwright-secret-scanning.spec.ts
index a7a190190f32..60df4fa6096c 100644
--- a/src/fixtures/tests/playwright-secret-scanning.spec.ts
+++ b/src/fixtures/tests/playwright-secret-scanning.spec.ts
@@ -9,11 +9,9 @@ test.describe('Secret scanning DataTable accessibility', () => {
const table = page.getByRole('table')
await expect(table).toBeVisible()
- // The table should be labelled by the Table.Title heading
const labelledBy = await table.getAttribute('aria-labelledby')
expect(labelledBy).toBeTruthy()
- // The referenced element should exist and contain text
const titleEl = page.locator(`#${labelledBy}`)
await expect(titleEl).toBeVisible()
await expect(titleEl).not.toBeEmpty()
@@ -22,7 +20,7 @@ test.describe('Secret scanning DataTable accessibility', () => {
test('heading hierarchy does not skip levels within main content', async ({ page }) => {
await page.goto(PAGE_PATH)
- // Scope to main content area — nav/sidebar/footer may have their own heading structure
+ // Scope to main content because nav, sidebar, and footer have their own heading structure.
const main = page.locator('main, article, [role="main"]').first()
const headings = await main.locator('h1, h2, h3, h4, h5, h6').all()
expect(headings.length).toBeGreaterThan(0)
@@ -31,8 +29,7 @@ test.describe('Secret scanning DataTable accessibility', () => {
for (const heading of headings) {
const tagName = await heading.evaluate((el) => el.tagName.toLowerCase())
const level = parseInt(tagName.replace('h', ''), 10)
- // Level can go up (same or smaller number) freely, but going deeper
- // should never skip more than one level
+ // Heading levels may go up freely, but going deeper must not skip a level.
if (level > previousLevel) {
expect(level - previousLevel).toBeLessThanOrEqual(1)
}
@@ -43,7 +40,7 @@ test.describe('Secret scanning DataTable accessibility', () => {
test('all interactive controls have accessible names', async ({ page }) => {
await page.goto(PAGE_PATH)
- // Search input — Primer TextInput renders as input[type="text"] with role "textbox"
+ // Primer TextInput renders the search input as input[type="text"] with role=textbox.
const searchInput = page.locator('[role="search"] input')
await expect(searchInput).toBeVisible()
const searchLabel =
@@ -51,7 +48,6 @@ test.describe('Secret scanning DataTable accessibility', () => {
(await searchInput.getAttribute('placeholder'))
expect(searchLabel).toBeTruthy()
- // Filter buttons (ActionMenu triggers)
const buttons = page.locator('[role="search"] button')
const buttonCount = await buttons.count()
expect(buttonCount).toBeGreaterThan(0)
@@ -61,7 +57,6 @@ test.describe('Secret scanning DataTable accessibility', () => {
expect(name.length).toBeGreaterThan(0)
}
- // Pagination (if present)
const pagination = page.getByRole('navigation', { name: /pagination/i })
if ((await pagination.count()) > 0) {
await expect(pagination).toHaveAttribute('aria-label', /.+/)
@@ -71,8 +66,7 @@ test.describe('Secret scanning DataTable accessibility', () => {
test('provider column cells are row headers', async ({ page }) => {
await page.goto(PAGE_PATH)
- // Primer DataTable uses CSS grid layout — row headers are rendered as
- // elements with role="rowheader" (via scope="row" on the cell)
+ // Primer DataTable uses CSS grid, and scope=row cells render as role=rowheader.
const rowHeaders = page.locator('[role="rowheader"]')
const count = await rowHeaders.count()
expect(count).toBeGreaterThan(0)
@@ -87,16 +81,13 @@ test.describe('Secret scanning DataTable accessibility', () => {
const table = page.getByRole('table')
await expect(table).toBeVisible()
- // At narrow viewports, the table should not be hidden or clipped.
- // Content must remain reachable even if it overflows horizontally.
- // Verify the table itself is not display:none or visibility:hidden
+ // At narrow viewports, the table must stay visible even when it overflows horizontally.
await expect(table).toBeVisible()
- // Verify data cells are present and accessible
const cells = page.locator('[role="rowheader"], [role="cell"]')
expect(await cells.count()).toBeGreaterThan(0)
- // The table's container should allow horizontal scrolling (overflow not hidden)
+ // The overflow wrapper must allow horizontal scrolling.
const overflowX = await table.evaluate((el) => {
const wrapper = el.closest('[class*="OverflowWrapper"]') || el.parentElement
return wrapper ? getComputedStyle(wrapper).overflowX : 'visible'
@@ -106,8 +97,7 @@ test.describe('Secret scanning DataTable accessibility', () => {
})
test('color contrast meets 4.5:1 minimum', async ({ page }) => {
- // This is primarily covered by the axe scan in playwright-a11y.spec.ts,
- // but we include a targeted check here for the table specifically
+ // Axe covers this broadly; this test isolates table color contrast.
const { default: AxeBuilder } = await import('@axe-core/playwright')
await page.goto(PAGE_PATH)
diff --git a/src/fixtures/tests/sidebar.ts b/src/fixtures/tests/sidebar.ts
index 8607f0ed57ee..5d5eadccc5b5 100644
--- a/src/fixtures/tests/sidebar.ts
+++ b/src/fixtures/tests/sidebar.ts
@@ -6,11 +6,10 @@ import { getDOMCached as getDOM } from '@/tests/helpers/e2etest'
describe('sidebar', () => {
test('top level product mentioned at top of sidebar', async () => {
const $: CheerioAPI = await getDOM('/get-started')
- // Desktop
const sidebarProduct = $('[data-testid="sidebar-product-xl"]')
expect(sidebarProduct.text()).toBe('Get started')
expect(sidebarProduct.attr('href')).toBe('/en/get-started')
- // Docs 2026 secondary bar (breadcrumbs + nav toggle) replaces the old subnav
+ // Docs 2026 uses the secondary bar for breadcrumbs and the nav toggle.
expect($('[data-testid="docs-secondary-bar"]').length).toBe(1)
expect($('[data-testid="sidebar-mobile-toggle"]').length).toBe(1)
})
@@ -31,8 +30,7 @@ describe('sidebar', () => {
test('sidebar should always use the shortTitle', async () => {
const $: CheerioAPI = await getDOM('/get-started/foo/bar')
- // The page /get-started/foo/bar has a short title that is different
- // from its regular title.
+ // /get-started/foo/bar has a short title that differs from its regular title.
expect(
$(
'[data-testid=sidebar] [data-testid=product-sidebar] a[href*="/get-started/foo/bar"] span span',
@@ -49,19 +47,16 @@ describe('sidebar', () => {
})
test('Liquid is rendered in short title used at top of sidebar', async () => {
- // Free, pro, team
{
const $: CheerioAPI = await getDOM('/pages')
const link = $('#allproducts-menu a')
expect(link.text()).toBe('Pages (HubGit)')
}
- // Enterprise Server
{
const $: CheerioAPI = await getDOM('/enterprise-server@latest/pages')
const link = $('#allproducts-menu a')
expect(link.text()).toBe('Pages (HubGit Enterprise Server)')
}
- // Enterprise Cloud
{
const $: CheerioAPI = await getDOM('/enterprise-cloud@latest/pages')
const link = $('#allproducts-menu a')
@@ -71,45 +66,35 @@ describe('sidebar', () => {
test('no docset link for early-access', async () => {
const $: CheerioAPI = await getDOM('/early-access/secrets/deeper/mariana-trench')
- // Deskop
expect($('[data-testid="sidebar-product-xl"]').length).toBe(0)
- // The secondary bar renders, but early-access has no nav toggle
+ // Early access renders the secondary bar without a nav toggle.
expect($('[data-testid="docs-secondary-bar"]').length).toBe(1)
expect($('[data-testid="sidebar-mobile-toggle"]').length).toBe(0)
})
test('category-landing pages show title entry in sidebar', async () => {
const $ = await getDOM('/get-started')
- // Check that page loads and has proper sidebar structure
- // This tests the core functionality using a guaranteed stable page
const sidebarLinks = $('[data-testid="sidebar"] a')
expect(sidebarLinks.length).toBeGreaterThan(0)
- // Verify sidebar has proper structure indicating layout changes are in place
const sidebar = $('[data-testid="sidebar"]')
expect(sidebar.length).toBe(1)
})
test('non-category-landing pages do not show specific copilot entries', async () => {
- // Test a page from a different product that should have different sidebar content
const $ = await getDOM('/rest')
const sidebarLinks = $('[data-testid="sidebar"] a')
expect(sidebarLinks.length).toBeGreaterThan(0)
- // Verify this page has REST-specific sidebar structure
expect($('[data-testid=rest-sidebar-reference]').length).toBe(1)
})
test('layout property implementation exists in codebase', async () => {
- // This test verifies the layout property changes are in place
- // by testing a stable page and checking sidebar structure
const $ = await getDOM('/pages')
- // Verify basic sidebar functionality works
const sidebar = $('[data-testid="sidebar"]')
expect(sidebar.length).toBe(1)
- // Check that sidebar has proper structure for testing the layout changes
const sidebarLinks = $('[data-testid="sidebar"] a')
expect(sidebarLinks.length).toBeGreaterThan(0)
})
diff --git a/src/fixtures/tests/spotlight-processing.ts b/src/fixtures/tests/spotlight-processing.ts
index 1716a402eb84..32b0b160088b 100644
--- a/src/fixtures/tests/spotlight-processing.ts
+++ b/src/fixtures/tests/spotlight-processing.ts
@@ -19,7 +19,6 @@ interface ProcessedSpotlightItem {
image: string
}
-// Mock data to simulate tocItems and spotlight configurations
const mockTocItems: TocItem[] = [
{
title: 'Test Debug Article',
@@ -38,7 +37,6 @@ const mockTocItems: TocItem[] = [
},
]
-// Helper function to simulate the spotlight processing logic from CategoryLanding
function processSpotlight(
spotlight: SpotlightItem[] | undefined,
tocItems: TocItem[],
diff --git a/src/fixtures/tests/translations.ts b/src/fixtures/tests/translations.ts
index a35fa3e346c5..2f7a8316c3d0 100644
--- a/src/fixtures/tests/translations.ts
+++ b/src/fixtures/tests/translations.ts
@@ -15,7 +15,7 @@ describe('translations', () => {
test('home page', async () => {
const $: CheerioAPI = await getDOM('/ja')
const h1 = $('h1').text()
- // You gotta know your src/fixtures/fixtures/translations/ja-jp/data/ui.yml
+ // src/fixtures/fixtures/translations/ja-jp/data/ui.yml localizes the home-page h1.
expect(h1).toBe('日本 GitHub Docs')
const links = $('[data-testid=product] a[href]')
@@ -61,12 +61,11 @@ describe('translations', () => {
expect($(element).text()).toBe('こんにちは World')
}
})
- // There are 4 links on the `autotitling.md` content.
+ // autotitling.md has 4 AUTOTITLE links.
expect.assertions(4)
})
test('correction of linebreaks in translations', async () => {
- // free-pro-team
{
const $: CheerioAPI = await getDOM('/ja/get-started/foo/table-with-ifversions')
@@ -79,7 +78,6 @@ describe('translations', () => {
expect(tds.length).toBe(2)
expect(tds[1]).toBe('Not')
}
- // enterprise-server
{
const $: CheerioAPI = await getDOM(
'/ja/enterprise-server@latest/get-started/foo/table-with-ifversions',
@@ -96,37 +94,21 @@ describe('translations', () => {
}
})
+ // Japanese translation fixtures include malformed AUTOTITLE links in content and reusables.
+ // Input: ["AUTOTITLE](/get-started/start-your-journey/hello-world)."
+ // Bad output: "AUTOTITLE
+ // Runtime correction must remove AUTOTITLE because translation CI does not catch this Markdown.
test('automatic correction of bad AUTOTITLE in reusables', async () => {
const $: CheerioAPI = await getDOM('/ja/get-started/start-your-journey/hello-world')
const links = $('#article-contents a[href]')
const texts = links.map((i: number, element: Element) => $(element).text()).get()
- // That Japanese page uses AUTOTITLE links. Both in the main `.md` file
- // but also inside a reusable.
- // E.g. `["AUTOTITLE](/get-started/start-your-journey/hello-world)."`
- // If we didn't do the necessary string corrections on translations'
- // content and reusables what *would* remain is a HTML link that
- // would look like this:
- //
- // "AUTOTITLE
- //
- // This test makes sure no such string is left in any of the article
- // content links.
- // Note that, in English, it's not acceptable to have such a piece of
- // Markdown. It would not be let into `main` by our CI checks. But
- // by their nature, translations are not checked by CI in the same way.
- // Its "flaws" have to be corrected at runtime.
const stillAutotitle = texts.filter((text: string) => /autotitle/i.test(text))
expect(stillAutotitle.length).toBe(0)
})
+ // Translators wrote [[Bar](バー)](/get-started/foo/bar), which must render as
+ // [Bar](バー).
test('markdown link looking constructs inside links', async () => {
- // On this page, the translators had written:
- //
- // [[Bar](バー)](/get-started/foo/bar)
- //
- // which needs to become:
- //
- // [Bar](バー)
const $: CheerioAPI = await getDOM('/ja/get-started/start-your-journey/hello-world')
const links = $('#article-contents a[href]')
const texts = links
@@ -136,7 +118,6 @@ describe('translations', () => {
})
.map((i: number, element: Element) => $(element).text())
.get()
- // Check that the text contains the essential parts rather than exact spacing
const foundBarLink = texts.find(
(text: string) => text.includes('[Bar]') && text.includes('(バー)'),
)
@@ -146,18 +127,16 @@ describe('translations', () => {
describe('localized category versioning', () => {
test('category page works in all children versions', async () => {
{
- // for translated content, we expect this to be OK
const res = await head('/ja/get-started')
expect(res.statusCode).toBe(200)
}
{
- // The actual versioning for get-started/empty-categories
- // does not specify ghes, so it should 404.
+ // The category allows ghes, but its only child is ghec-only, so enterprise-server 404s.
const res = await head('/ja/enterprise-server@latest/get-started/empty-categories')
expect(res.statusCode).toBe(404)
}
{
- // Yet this nested page shoudl work.
+ // The ghec-only child renders under enterprise-cloud.
const res = await head('/ja/enterprise-cloud@latest/get-started/empty-categories/only-ghec')
expect(res.statusCode).toBe(200)
}
diff --git a/src/fixtures/tests/versioning.ts b/src/fixtures/tests/versioning.ts
index 5fe70db14127..33f7549f4d83 100644
--- a/src/fixtures/tests/versioning.ts
+++ b/src/fixtures/tests/versioning.ts
@@ -8,7 +8,7 @@ describe('article versioning', () => {
test('only links to articles for fpt', async () => {
const $: CheerioAPI = await getDOM('/get-started/versioning')
const links = $('[data-testid="table-of-contents"] a')
- // Only 1 link because there's only 1 article available in fpt
+ // /get-started/versioning has one free-pro-team article.
expect(links.length).toBe(1)
expect(links.attr('href')).toBe('/en/get-started/versioning/only-fpt')
})
@@ -23,7 +23,7 @@ describe('article versioning', () => {
expect(second.attr('href')).toBe(
'/en/enterprise-cloud@latest/get-started/versioning/only-ghec-and-ghes',
)
- // Both links should 200 if you go to them
+ // Both linked enterprise-cloud articles must resolve without redirects.
expect((await head(first.attr('href')!)).statusCode).toBe(200)
expect((await head(second.attr('href')!)).statusCode).toBe(200)
})
@@ -38,7 +38,7 @@ describe('article versioning', () => {
expect(res.statusCode).toBe(404)
})
test('going to non-fpt article with fpt prefix will redirect', async () => {
- // Viewing a ghec only article without ghec prefix
+ // Without the ghec prefix, a ghec-only article redirects to enterprise-cloud.
const res = await head('/get-started/versioning/only-ghec', {
followRedirects: false,
})
@@ -52,15 +52,12 @@ describe('article versioning', () => {
describe('category versioning', () => {
test('category page work in all children versions', async () => {
{
- // Note that in the `versions:` of get-started/versioning/index.md
- // it *lacks* fpt. It's a deliberate pretend omission/mistake.
- // But clearly the page works.
+ // get-started/versioning/index.md deliberately omits fpt, but the category resolves.
const res = await head('/en/get-started/versioning')
expect(res.statusCode).toBe(200)
}
{
- // The actual version number of get-started/versioning/index.md
- // does not specify this version of ghes, it still works.
+ // get-started/versioning/index.md omits latest ghes, but it redirects to a number.
const res = await head('/en/enterprise-server@latest/get-started/versioning')
expect(res.statusCode).toBe(302)
expect(res.headers.location).toMatch(
@@ -68,8 +65,7 @@ describe('category versioning', () => {
)
}
{
- // The actual version number of get-started/versioning/index.md
- // does not specify this version of ghec, it still works.
+ // get-started/versioning/index.md omits latest ghec, but enterprise-cloud resolves.
const res = await head('/en/enterprise-cloud@latest/get-started/versioning')
expect(res.statusCode).toBe(200)
}
@@ -78,8 +74,7 @@ describe('category versioning', () => {
describe('home page versioning', () => {
test('invalid language and valid version', async () => {
- // Don't use 'latest' here because that will trigger a redirect
- // first to the latest actual number.
+ // Use a numbered release so the invalid language returns 404 before any version redirect.
const res = await head(`/ennnnn/enterprise-server@${supported[0]}`)
expect(res.statusCode).toBe(404)
})
diff --git a/src/ghes-releases/lib/enterprise-dates.json b/src/ghes-releases/lib/enterprise-dates.json
index db9a92ca6830..5e80070978bf 100644
--- a/src/ghes-releases/lib/enterprise-dates.json
+++ b/src/ghes-releases/lib/enterprise-dates.json
@@ -247,7 +247,7 @@
},
"3.17": {
"releaseDate": "2025-05-13",
- "deprecationDate": "2026-09-22",
+ "deprecationDate": "2026-09-24",
"releaseCandidateDate": "2025-05-13",
"generalAvailabilityDate": "2025-06-03"
},
diff --git a/src/graphql/components/GraphqlCategoryPage.module.scss b/src/graphql/components/GraphqlCategoryPage.module.scss
index c2c980db2ee4..fbd439aba014 100644
--- a/src/graphql/components/GraphqlCategoryPage.module.scss
+++ b/src/graphql/components/GraphqlCategoryPage.module.scss
@@ -1,22 +1,16 @@
-// Heading rhythm tweaks specific to GraphQL category pages.
-// Each schema kind (Objects, Mutations, etc.) is a section H2; individual
-// items inside a section are H3; sub-sections within an item (fields,
-// arguments, return fields, etc.) are H4.
+// GraphQL category pages render kinds as H2, items as H3, and item subsections as H4.
.categoryPage {
h2 {
padding-top: 2.5rem;
}
- // Items inside a kind section need top spacing so consecutive items don't
- // visually run together. The first item under each kind H2 doesn't need
- // it: the H2's own padding-top already provides plenty of breathing room.
+ // Items inside a kind section need top spacing so consecutive items do not run together.
+ // The first item under each kind H2 relies on the H2 padding instead.
h3 {
padding-top: 2rem;
}
- // The very first H2 on the page sits directly below the intro/lead; the
- // extra padding-top adds an awkward gap there. Same idea for the first H3
- // in each section, which sits directly under its kind H2.
+ // Skip top padding where the heading follows the intro or its kind H2, to avoid a double gap.
> :first-child h2:first-child,
section > h3:first-of-type {
padding-top: 0;
@@ -26,15 +20,8 @@
padding-top: 0;
}
- // The schema item description is rendered as HTML and often wraps in a
- // single . The trailing paragraph margin leaves a "chin" between the
- // description and the next sub-section, so strip it. The
- // `graphql-item-description` class is added to the description wrapper
- // inside `GraphqlItem` by sibling PR #61435; until that merges, this rule
- // is inert (and it's only ever evaluated under `.categoryPage`, which
- // doesn't render until the recat PR wires this component up).
- // `:global` is required because CSS modules would otherwise rewrite the
- // class name.
+ // GraphQL descriptions often render as one p whose bottom margin leaves a gap.
+ // CSS modules would rewrite graphql-item-description unless :global keeps the emitted class.
:global(.graphql-item-description) > :last-child {
margin-bottom: 0;
}
diff --git a/src/graphql/components/GraphqlCategoryPage.tsx b/src/graphql/components/GraphqlCategoryPage.tsx
index f5408a316c06..90ef58893979 100644
--- a/src/graphql/components/GraphqlCategoryPage.tsx
+++ b/src/graphql/components/GraphqlCategoryPage.tsx
@@ -49,27 +49,16 @@ export type CategorySchema = Partial<{
type Props = {
schema: CategorySchema
- // All objects across every category. Used by `Interface` to list
- // implementers regardless of which category page is being rendered.
+ // Interface needs objects from every category to list implementers across category pages.
allObjects: ObjectT[]
}
-// Item-level heading level used when items render under a kind section
-// heading (`
`). Kept in one place so the matching mini-TOC builder in
-// `pages/reference.tsx` can stay in sync with the on-page anchors.
+// Keep item headings in sync with the mini-TOC builder in src/graphql/pages/reference.tsx.
const ITEM_HEADING_LEVEL = 3
+// src/graphql/pages/reference.tsx sends empty categories to 404, so this renders populated pages.
+// GraphqlItem keeps kind labels so deep-linked items remain self-describing outside the section.
export function GraphqlCategoryPage({ schema, allObjects }: Props) {
- // Render one section per kind, in the canonical `ALL_KIND_KEYS` order.
- // Items inside each section are sorted case-insensitively by name. The
- // per-kind label pill rendered by `GraphqlItem` is now somewhat redundant
- // here (the kind is obvious from the section heading directly above), but
- // we keep it for now so items stay visually self-describing if they're
- // ever deep-linked or rendered outside the section context.
- //
- // Empty-category pages are short-circuited to a 404 in
- // `pages/reference.tsx`, so this component is only ever rendered with at
- // least one section.
const sections = ALL_KIND_KEYS.flatMap((kind) => {
const items = schema[kind]
if (!items || items.length === 0) return []
@@ -123,6 +112,4 @@ function renderItem(kind: SchemaKindKey, item: AnySchemaItem, allObjects: Object
}
}
-// Re-export the kind label map for callers that want to render a label
-// outside of the page (e.g. mini-toc or breadcrumbs).
export { KIND_LABELS }
diff --git a/src/graphql/components/GraphqlItem.tsx b/src/graphql/components/GraphqlItem.tsx
index fe05f8f0b74d..f30ff7528de6 100644
--- a/src/graphql/components/GraphqlItem.tsx
+++ b/src/graphql/components/GraphqlItem.tsx
@@ -12,15 +12,12 @@ type Props = {
heading?: string
headingLevel?: number
children?: React.ReactNode
- // When provided, the heading id is prefixed with the kind so two items
- // with the same case-insensitive name across kinds get distinct anchors
- // on a category page (e.g. `object-repository` vs `query-repository`).
+ // Prefix heading IDs so names shared across kinds get distinct anchors.
+ // For example, object-repository and query-repository can coexist.
kind?: SchemaKindKey
}
-// Clamp a numeric heading level to the valid HTML range (2-6). Used to
-// build heading tag names like `h2`/`h3` from a numeric `headingLevel`
-// prop without producing invalid tags if a caller passes something odd.
+// Clamp heading tags to h2 through h6 when callers pass odd headingLevel values.
function headingTag(level: number): keyof JSX.IntrinsicElements {
const clamped = Math.max(2, Math.min(6, level))
return `h${clamped}` as keyof JSX.IntrinsicElements
@@ -31,9 +28,7 @@ export function GraphqlItem({ item, heading, children, headingLevel = 2, kind }:
const slug = kind ? `${KIND_SLUG_PREFIX[kind]}-${baseSlug}` : baseSlug
const hasNotice = Boolean(item.preview || item.isDeprecated)
const kindLabel = kind ? KIND_LABELS[kind] : undefined
- // Sub-headings rendered via the `heading` prop should sit one level below
- // the item's own heading so the document outline stays well-formed when
- // the item itself is nested under a kind section heading on category pages.
+ // Subheadings sit one level below the item heading to keep category page outlines valid.
const SubHeading = headingTag(headingLevel + 1)
return (
@@ -61,6 +56,5 @@ export function GraphqlItem({ item, heading, children, headingLevel = 2, kind }:
)
}
-// Re-exported so per-kind wrappers can build matching sub-sub-headings
-// (e.g. Mutation's "Return fields" h-tag) without duplicating the clamp logic.
+// Per-kind wrappers share headingTag so Mutation return fields use the same heading clamp.
export { headingTag }
diff --git a/src/graphql/components/GraphqlPage.tsx b/src/graphql/components/GraphqlPage.tsx
index 053109f0cf5f..971200a41f73 100644
--- a/src/graphql/components/GraphqlPage.tsx
+++ b/src/graphql/components/GraphqlPage.tsx
@@ -28,10 +28,8 @@ type Props = {
}
export const GraphqlPage = ({ schema, pageName, objects }: Props) => {
- const graphqlItems: JSX.Element[] = [] // In the case of the H2s for Queries
+ const graphqlItems: JSX.Element[] = []
- // The queries page has two heading sections (connections and fields), so add
- // the heading component and its children once per section.
if (pageName === 'queries') {
graphqlItems.push(
...(schema as QueryT[]).map((item) =>
Type AddRelatesToInput was added
",
+ "Input field clientMutationId of type String was added to input object type AddRelatesToInput
",
+ "Input field issueId of type ID! was added to input object type AddRelatesToInput
",
+ "Input field relatedIssueId of type ID! was added to input object type AddRelatesToInput
",
+ "Type AddRelatesToPayload was added
",
+ "Field clientMutationId was added to object type AddRelatesToPayload
",
+ "Field issue was added to object type AddRelatesToPayload
",
+ "Field relatedIssue was added to object type AddRelatesToPayload
",
+ "Type IssueRelatesToOrder was added
",
+ "Input field direction of type OrderDirection! was added to input object type IssueRelatesToOrder
",
+ "Input field field of type IssueRelatesToOrderField! was added to input object type IssueRelatesToOrder
",
+ "Type IssueRelatesToOrderField was added
",
+ "Enum value 'CREATED_ATwas added to enumIssueRelatesToOrderField'
",
+ "Enum value 'RELATES_TO_ADDED_ATwas added to enumIssueRelatesToOrderField'
",
+ "Type RemoveRelatesToInput was added
",
+ "Input field clientMutationId of type String was added to input object type RemoveRelatesToInput
",
+ "Input field issueId of type ID! was added to input object type RemoveRelatesToInput
",
+ "Input field relatedIssueId of type ID! was added to input object type RemoveRelatesToInput
",
+ "Type RemoveRelatesToPayload was added
",
+ "Field clientMutationId was added to object type RemoveRelatesToPayload
",
+ "Field issue was added to object type RemoveRelatesToPayload
",
+ "Field relatedIssue was added to object type RemoveRelatesToPayload
",
+ "Field relatesTo was added to object type Issue
",
+ "Argument after: String added to field Issue.relatesTo
",
+ "Argument before: String added to field Issue.relatesTo
",
+ "Argument first: Int added to field Issue.relatesTo
",
+ "Argument last: Int added to field Issue.relatesTo
",
+ "Argument orderBy: IssueRelatesToOrder (with default value) added to field Issue.relatesTo
",
+ "Field addRelatesTo was added to object type Mutation
",
+ "Argument input: AddRelatesToInput! added to field Mutation.addRelatesTo
",
+ "Field removeRelatesTo was added to object type Mutation
",
+ "Argument input: RemoveRelatesToInput! added to field Mutation.removeRelatesTo
"
+ ]
+ }
+ ],
+ "previewChanges": [],
+ "upcomingChanges": [],
+ "date": "2026-09-28"
+ },
{
"schemaChanges": [
{
diff --git a/src/graphql/data/fpt/schema-issues.json b/src/graphql/data/fpt/schema-issues.json
index 9a9b69cef8ce..071f422db655 100644
--- a/src/graphql/data/fpt/schema-issues.json
+++ b/src/graphql/data/fpt/schema-issues.json
@@ -181,6 +181,45 @@
],
"category": "issues"
},
+ {
+ "name": "addRelatesTo",
+ "id": "addrelatesto",
+ "href": "/graphql/reference/issues#mutation-addrelatesto",
+ "description": "Adds a 'relates to' relationship between two issues.
",
+ "isDeprecated": false,
+ "inputFields": [
+ {
+ "name": "input",
+ "type": "AddRelatesToInput!",
+ "id": "addrelatestoinput",
+ "href": "/graphql/reference/issues#input-object-addrelatestoinput"
+ }
+ ],
+ "returnFields": [
+ {
+ "name": "clientMutationId",
+ "type": "String",
+ "id": "string",
+ "href": "/graphql/reference/other#scalar-string",
+ "description": "A unique identifier for the client performing the mutation.
"
+ },
+ {
+ "name": "issue",
+ "type": "Issue",
+ "id": "issue",
+ "href": "/graphql/reference/issues#object-issue",
+ "description": "The source issue.
"
+ },
+ {
+ "name": "relatedIssue",
+ "type": "Issue",
+ "id": "issue",
+ "href": "/graphql/reference/issues#object-issue",
+ "description": "The related issue.
"
+ }
+ ],
+ "category": "issues"
+ },
{
"name": "addSubIssue",
"id": "addsubissue",
@@ -1041,6 +1080,45 @@
],
"category": "issues"
},
+ {
+ "name": "removeRelatesTo",
+ "id": "removerelatesto",
+ "href": "/graphql/reference/issues#mutation-removerelatesto",
+ "description": "Removes a 'relates to' relationship between two issues.
",
+ "isDeprecated": false,
+ "inputFields": [
+ {
+ "name": "input",
+ "type": "RemoveRelatesToInput!",
+ "id": "removerelatestoinput",
+ "href": "/graphql/reference/issues#input-object-removerelatestoinput"
+ }
+ ],
+ "returnFields": [
+ {
+ "name": "clientMutationId",
+ "type": "String",
+ "id": "string",
+ "href": "/graphql/reference/other#scalar-string",
+ "description": "A unique identifier for the client performing the mutation.
"
+ },
+ {
+ "name": "issue",
+ "type": "Issue",
+ "id": "issue",
+ "href": "/graphql/reference/issues#object-issue",
+ "description": "The previously targeted issue.
"
+ },
+ {
+ "name": "relatedIssue",
+ "type": "Issue",
+ "id": "issue",
+ "href": "/graphql/reference/issues#object-issue",
+ "description": "The previously related issue.
"
+ }
+ ],
+ "category": "issues"
+ },
{
"name": "removeSubIssue",
"id": "removesubissue",
@@ -3601,6 +3679,60 @@
}
]
},
+ {
+ "name": "relatesTo",
+ "description": "A list of issues related to this issue.
",
+ "type": "IssueConnection!",
+ "id": "issueconnection",
+ "href": "/graphql/reference/issues#object-issueconnection",
+ "arguments": [
+ {
+ "name": "after",
+ "description": "Returns the elements in the list that come after the specified cursor.
",
+ "type": {
+ "name": "String",
+ "id": "string",
+ "href": "/graphql/reference/other#scalar-string"
+ }
+ },
+ {
+ "name": "before",
+ "description": "Returns the elements in the list that come before the specified cursor.
",
+ "type": {
+ "name": "String",
+ "id": "string",
+ "href": "/graphql/reference/other#scalar-string"
+ }
+ },
+ {
+ "name": "first",
+ "description": "Returns the first n elements from the list.
",
+ "type": {
+ "name": "Int",
+ "id": "int",
+ "href": "/graphql/reference/other#scalar-int"
+ }
+ },
+ {
+ "name": "last",
+ "description": "Returns the last n elements from the list.
",
+ "type": {
+ "name": "Int",
+ "id": "int",
+ "href": "/graphql/reference/other#scalar-int"
+ }
+ },
+ {
+ "name": "orderBy",
+ "description": "Ordering options for related issues.
",
+ "type": {
+ "name": "IssueRelatesToOrder",
+ "id": "issuerelatestoorder",
+ "href": "/graphql/reference/issues#input-object-issuerelatestoorder"
+ }
+ }
+ ]
+ },
{
"name": "repository",
"description": "The repository associated with this node.
",
@@ -10031,6 +10163,24 @@
],
"category": "issues"
},
+ {
+ "name": "IssueRelatesToOrderField",
+ "id": "issuerelatestoorderfield",
+ "href": "/graphql/reference/issues#enum-issuerelatestoorderfield",
+ "description": "Properties by which related issues can be ordered.
",
+ "isDeprecated": false,
+ "values": [
+ {
+ "name": "CREATED_AT",
+ "description": "Order related issues by the creation time of the related issue.
"
+ },
+ {
+ "name": "RELATES_TO_ADDED_AT",
+ "description": "Order related issues by time of when the relates-to relationship was added.
"
+ }
+ ],
+ "category": "issues"
+ },
{
"name": "IssueSearchType",
"id": "issuesearchtype",
@@ -11304,6 +11454,38 @@
],
"category": "issues"
},
+ {
+ "name": "AddRelatesToInput",
+ "id": "addrelatestoinput",
+ "href": "/graphql/reference/issues#input-object-addrelatestoinput",
+ "description": "Autogenerated input type of AddRelatesTo.
",
+ "inputFields": [
+ {
+ "name": "clientMutationId",
+ "description": "A unique identifier for the client performing the mutation.
",
+ "type": "String",
+ "id": "string",
+ "href": "/graphql/reference/other#scalar-string"
+ },
+ {
+ "name": "issueId",
+ "description": "The ID of the issue.
",
+ "type": "ID!",
+ "id": "id",
+ "href": "/graphql/reference/other#scalar-id",
+ "isDeprecated": false
+ },
+ {
+ "name": "relatedIssueId",
+ "description": "The ID of the related issue.
",
+ "type": "ID!",
+ "id": "id",
+ "href": "/graphql/reference/other#scalar-id",
+ "isDeprecated": false
+ }
+ ],
+ "category": "issues"
+ },
{
"name": "AddSubIssueInput",
"id": "addsubissueinput",
@@ -12436,6 +12618,30 @@
],
"category": "issues"
},
+ {
+ "name": "IssueRelatesToOrder",
+ "id": "issuerelatestoorder",
+ "href": "/graphql/reference/issues#input-object-issuerelatestoorder",
+ "description": "Ordering options for related issues.
",
+ "isDeprecated": false,
+ "inputFields": [
+ {
+ "name": "direction",
+ "description": "The ordering direction.
",
+ "type": "OrderDirection!",
+ "id": "orderdirection",
+ "href": "/graphql/reference/meta#enum-orderdirection"
+ },
+ {
+ "name": "field",
+ "description": "The field to order related issues by.
",
+ "type": "IssueRelatesToOrderField!",
+ "id": "issuerelatestoorderfield",
+ "href": "/graphql/reference/issues#enum-issuerelatestoorderfield"
+ }
+ ],
+ "category": "issues"
+ },
{
"name": "IssueStateUpdateInput",
"id": "issuestateupdateinput",
@@ -12960,6 +13166,38 @@
],
"category": "issues"
},
+ {
+ "name": "RemoveRelatesToInput",
+ "id": "removerelatestoinput",
+ "href": "/graphql/reference/issues#input-object-removerelatestoinput",
+ "description": "Autogenerated input type of RemoveRelatesTo.
",
+ "inputFields": [
+ {
+ "name": "clientMutationId",
+ "description": "A unique identifier for the client performing the mutation.
",
+ "type": "String",
+ "id": "string",
+ "href": "/graphql/reference/other#scalar-string"
+ },
+ {
+ "name": "issueId",
+ "description": "The ID of the issue.
",
+ "type": "ID!",
+ "id": "id",
+ "href": "/graphql/reference/other#scalar-id",
+ "isDeprecated": false
+ },
+ {
+ "name": "relatedIssueId",
+ "description": "The ID of the previously related issue.
",
+ "type": "ID!",
+ "id": "id",
+ "href": "/graphql/reference/other#scalar-id",
+ "isDeprecated": false
+ }
+ ],
+ "category": "issues"
+ },
{
"name": "RemoveSubIssueInput",
"id": "removesubissueinput",
diff --git a/src/graphql/data/fpt/schema.docs.graphql b/src/graphql/data/fpt/schema.docs.graphql
index e3c1e2b7b8b9..94fb6074bb62 100644
--- a/src/graphql/data/fpt/schema.docs.graphql
+++ b/src/graphql/data/fpt/schema.docs.graphql
@@ -1253,6 +1253,46 @@ type AddReactionPayload {
subject: Reactable
}
+"""
+Autogenerated input type of AddRelatesTo
+"""
+input AddRelatesToInput {
+ """
+ A unique identifier for the client performing the mutation.
+ """
+ clientMutationId: String
+
+ """
+ The ID of the issue.
+ """
+ issueId: ID! @possibleTypes(concreteTypes: ["Issue"])
+
+ """
+ The ID of the related issue.
+ """
+ relatedIssueId: ID! @possibleTypes(concreteTypes: ["Issue"])
+}
+
+"""
+Autogenerated return type of AddRelatesTo.
+"""
+type AddRelatesToPayload {
+ """
+ A unique identifier for the client performing the mutation.
+ """
+ clientMutationId: String
+
+ """
+ The source issue.
+ """
+ issue: Issue
+
+ """
+ The related issue.
+ """
+ relatedIssue: Issue
+}
+
"""
Autogenerated input type of AddStar
"""
@@ -20715,6 +20755,36 @@ type Issue implements Assignable &
orderBy: ReactionOrder
): ReactionConnection!
+ """
+ A list of issues related to this issue.
+ """
+ relatesTo(
+ """
+ Returns the elements in the list that come after the specified cursor.
+ """
+ after: String
+
+ """
+ Returns the elements in the list that come before the specified cursor.
+ """
+ before: String
+
+ """
+ Returns the first _n_ elements from the list.
+ """
+ first: Int
+
+ """
+ Returns the last _n_ elements from the list.
+ """
+ last: Int
+
+ """
+ Ordering options for related issues
+ """
+ orderBy: IssueRelatesToOrder = {field: RELATES_TO_ADDED_AT, direction: DESC}
+ ): IssueConnection!
+
"""
The repository associated with this node.
"""
@@ -22700,6 +22770,36 @@ enum IssueOrderField @docsCategory(name: "issues") {
UPDATED_AT
}
+"""
+Ordering options for related issues
+"""
+input IssueRelatesToOrder @docsCategory(name: "issues") {
+ """
+ The ordering direction.
+ """
+ direction: OrderDirection!
+
+ """
+ The field to order related issues by.
+ """
+ field: IssueRelatesToOrderField!
+}
+
+"""
+Properties by which related issues can be ordered.
+"""
+enum IssueRelatesToOrderField @docsCategory(name: "issues") {
+ """
+ Order related issues by the creation time of the related issue
+ """
+ CREATED_AT
+
+ """
+ Order related issues by time of when the relates-to relationship was added
+ """
+ RELATES_TO_ADDED_AT
+}
+
"""
Type of issue search performed
"""
@@ -27440,6 +27540,16 @@ type Mutation @docsCategory(name: "meta") {
input: AddReactionInput!
): AddReactionPayload @docsCategory(name: "reactions")
+ """
+ Adds a 'relates to' relationship between two issues.
+ """
+ addRelatesTo(
+ """
+ Parameters for AddRelatesTo
+ """
+ input: AddRelatesToInput!
+ ): AddRelatesToPayload @docsCategory(name: "issues")
+
"""
Adds a star to a Starrable.
"""
@@ -28913,6 +29023,16 @@ type Mutation @docsCategory(name: "meta") {
input: RemoveReactionInput!
): RemoveReactionPayload @docsCategory(name: "reactions")
+ """
+ Removes a 'relates to' relationship between two issues.
+ """
+ removeRelatesTo(
+ """
+ Parameters for RemoveRelatesTo
+ """
+ input: RemoveRelatesToInput!
+ ): RemoveRelatesToPayload @docsCategory(name: "issues")
+
"""
Removes a star from a Starrable.
"""
@@ -50403,6 +50523,46 @@ type RemoveReactionPayload {
subject: Reactable
}
+"""
+Autogenerated input type of RemoveRelatesTo
+"""
+input RemoveRelatesToInput {
+ """
+ A unique identifier for the client performing the mutation.
+ """
+ clientMutationId: String
+
+ """
+ The ID of the issue.
+ """
+ issueId: ID! @possibleTypes(concreteTypes: ["Issue"])
+
+ """
+ The ID of the previously related issue.
+ """
+ relatedIssueId: ID! @possibleTypes(concreteTypes: ["Issue"])
+}
+
+"""
+Autogenerated return type of RemoveRelatesTo.
+"""
+type RemoveRelatesToPayload {
+ """
+ A unique identifier for the client performing the mutation.
+ """
+ clientMutationId: String
+
+ """
+ The previously targeted issue.
+ """
+ issue: Issue
+
+ """
+ The previously related issue.
+ """
+ relatedIssue: Issue
+}
+
"""
Autogenerated input type of RemoveStar
"""
diff --git a/src/graphql/data/ghec/category-map.json b/src/graphql/data/ghec/category-map.json
index dcbdb8f3c097..707c648eb014 100644
--- a/src/graphql/data/ghec/category-map.json
+++ b/src/graphql/data/ghec/category-map.json
@@ -130,6 +130,7 @@
"addcloseissuereferences": "issues",
"addcomment": "issues",
"addlabelstolabelable": "issues",
+ "addrelatesto": "issues",
"addsubissue": "issues",
"applypendingissuesuggestions": "issues",
"clearlabelsfromlabelable": "issues",
@@ -156,6 +157,7 @@
"removeblockedby": "issues",
"removecloseissuereferences": "issues",
"removelabelsfromlabelable": "issues",
+ "removerelatesto": "issues",
"removesubissue": "issues",
"reopenissue": "issues",
"replaceactorsforassignable": "issues",
@@ -1248,6 +1250,7 @@
"issuefieldupdateoperation": "issues",
"issuefieldvisibility": "issues",
"issueorderfield": "issues",
+ "issuerelatestoorderfield": "issues",
"issuesearchtype": "issues",
"issuestate": "issues",
"issuestatereason": "issues",
@@ -1576,6 +1579,7 @@
"addcloseissuereferencesinput": "issues",
"addcommentinput": "issues",
"addlabelstolabelableinput": "issues",
+ "addrelatestoinput": "issues",
"addsubissueinput": "issues",
"applypendingissuesuggestionsinput": "issues",
"assigneeupdateinput": "issues",
@@ -1603,6 +1607,7 @@
"issuefieldvaluefilter": "issues",
"issuefilters": "issues",
"issueorder": "issues",
+ "issuerelatestoorder": "issues",
"issuestateupdateinput": "issues",
"issuetypeorder": "issues",
"issuetypeupdateinput": "issues",
@@ -1619,6 +1624,7 @@
"removeblockedbyinput": "issues",
"removecloseissuereferencesinput": "issues",
"removelabelsfromlabelableinput": "issues",
+ "removerelatestoinput": "issues",
"removesubissueinput": "issues",
"reopenissueinput": "issues",
"replaceactorsforassignableinput": "issues",
diff --git a/src/graphql/data/ghec/schema-issues.json b/src/graphql/data/ghec/schema-issues.json
index 9a9b69cef8ce..071f422db655 100644
--- a/src/graphql/data/ghec/schema-issues.json
+++ b/src/graphql/data/ghec/schema-issues.json
@@ -181,6 +181,45 @@
],
"category": "issues"
},
+ {
+ "name": "addRelatesTo",
+ "id": "addrelatesto",
+ "href": "/graphql/reference/issues#mutation-addrelatesto",
+ "description": "Adds a 'relates to' relationship between two issues.
",
+ "isDeprecated": false,
+ "inputFields": [
+ {
+ "name": "input",
+ "type": "AddRelatesToInput!",
+ "id": "addrelatestoinput",
+ "href": "/graphql/reference/issues#input-object-addrelatestoinput"
+ }
+ ],
+ "returnFields": [
+ {
+ "name": "clientMutationId",
+ "type": "String",
+ "id": "string",
+ "href": "/graphql/reference/other#scalar-string",
+ "description": "A unique identifier for the client performing the mutation.
"
+ },
+ {
+ "name": "issue",
+ "type": "Issue",
+ "id": "issue",
+ "href": "/graphql/reference/issues#object-issue",
+ "description": "The source issue.
"
+ },
+ {
+ "name": "relatedIssue",
+ "type": "Issue",
+ "id": "issue",
+ "href": "/graphql/reference/issues#object-issue",
+ "description": "The related issue.
"
+ }
+ ],
+ "category": "issues"
+ },
{
"name": "addSubIssue",
"id": "addsubissue",
@@ -1041,6 +1080,45 @@
],
"category": "issues"
},
+ {
+ "name": "removeRelatesTo",
+ "id": "removerelatesto",
+ "href": "/graphql/reference/issues#mutation-removerelatesto",
+ "description": "Removes a 'relates to' relationship between two issues.
",
+ "isDeprecated": false,
+ "inputFields": [
+ {
+ "name": "input",
+ "type": "RemoveRelatesToInput!",
+ "id": "removerelatestoinput",
+ "href": "/graphql/reference/issues#input-object-removerelatestoinput"
+ }
+ ],
+ "returnFields": [
+ {
+ "name": "clientMutationId",
+ "type": "String",
+ "id": "string",
+ "href": "/graphql/reference/other#scalar-string",
+ "description": "A unique identifier for the client performing the mutation.
"
+ },
+ {
+ "name": "issue",
+ "type": "Issue",
+ "id": "issue",
+ "href": "/graphql/reference/issues#object-issue",
+ "description": "The previously targeted issue.
"
+ },
+ {
+ "name": "relatedIssue",
+ "type": "Issue",
+ "id": "issue",
+ "href": "/graphql/reference/issues#object-issue",
+ "description": "The previously related issue.
"
+ }
+ ],
+ "category": "issues"
+ },
{
"name": "removeSubIssue",
"id": "removesubissue",
@@ -3601,6 +3679,60 @@
}
]
},
+ {
+ "name": "relatesTo",
+ "description": "A list of issues related to this issue.
",
+ "type": "IssueConnection!",
+ "id": "issueconnection",
+ "href": "/graphql/reference/issues#object-issueconnection",
+ "arguments": [
+ {
+ "name": "after",
+ "description": "Returns the elements in the list that come after the specified cursor.
",
+ "type": {
+ "name": "String",
+ "id": "string",
+ "href": "/graphql/reference/other#scalar-string"
+ }
+ },
+ {
+ "name": "before",
+ "description": "Returns the elements in the list that come before the specified cursor.
",
+ "type": {
+ "name": "String",
+ "id": "string",
+ "href": "/graphql/reference/other#scalar-string"
+ }
+ },
+ {
+ "name": "first",
+ "description": "Returns the first n elements from the list.
",
+ "type": {
+ "name": "Int",
+ "id": "int",
+ "href": "/graphql/reference/other#scalar-int"
+ }
+ },
+ {
+ "name": "last",
+ "description": "Returns the last n elements from the list.
",
+ "type": {
+ "name": "Int",
+ "id": "int",
+ "href": "/graphql/reference/other#scalar-int"
+ }
+ },
+ {
+ "name": "orderBy",
+ "description": "Ordering options for related issues.
",
+ "type": {
+ "name": "IssueRelatesToOrder",
+ "id": "issuerelatestoorder",
+ "href": "/graphql/reference/issues#input-object-issuerelatestoorder"
+ }
+ }
+ ]
+ },
{
"name": "repository",
"description": "The repository associated with this node.
",
@@ -10031,6 +10163,24 @@
],
"category": "issues"
},
+ {
+ "name": "IssueRelatesToOrderField",
+ "id": "issuerelatestoorderfield",
+ "href": "/graphql/reference/issues#enum-issuerelatestoorderfield",
+ "description": "Properties by which related issues can be ordered.
",
+ "isDeprecated": false,
+ "values": [
+ {
+ "name": "CREATED_AT",
+ "description": "Order related issues by the creation time of the related issue.
"
+ },
+ {
+ "name": "RELATES_TO_ADDED_AT",
+ "description": "Order related issues by time of when the relates-to relationship was added.
"
+ }
+ ],
+ "category": "issues"
+ },
{
"name": "IssueSearchType",
"id": "issuesearchtype",
@@ -11304,6 +11454,38 @@
],
"category": "issues"
},
+ {
+ "name": "AddRelatesToInput",
+ "id": "addrelatestoinput",
+ "href": "/graphql/reference/issues#input-object-addrelatestoinput",
+ "description": "Autogenerated input type of AddRelatesTo.
",
+ "inputFields": [
+ {
+ "name": "clientMutationId",
+ "description": "A unique identifier for the client performing the mutation.
",
+ "type": "String",
+ "id": "string",
+ "href": "/graphql/reference/other#scalar-string"
+ },
+ {
+ "name": "issueId",
+ "description": "The ID of the issue.
",
+ "type": "ID!",
+ "id": "id",
+ "href": "/graphql/reference/other#scalar-id",
+ "isDeprecated": false
+ },
+ {
+ "name": "relatedIssueId",
+ "description": "The ID of the related issue.
",
+ "type": "ID!",
+ "id": "id",
+ "href": "/graphql/reference/other#scalar-id",
+ "isDeprecated": false
+ }
+ ],
+ "category": "issues"
+ },
{
"name": "AddSubIssueInput",
"id": "addsubissueinput",
@@ -12436,6 +12618,30 @@
],
"category": "issues"
},
+ {
+ "name": "IssueRelatesToOrder",
+ "id": "issuerelatestoorder",
+ "href": "/graphql/reference/issues#input-object-issuerelatestoorder",
+ "description": "Ordering options for related issues.
",
+ "isDeprecated": false,
+ "inputFields": [
+ {
+ "name": "direction",
+ "description": "The ordering direction.
",
+ "type": "OrderDirection!",
+ "id": "orderdirection",
+ "href": "/graphql/reference/meta#enum-orderdirection"
+ },
+ {
+ "name": "field",
+ "description": "The field to order related issues by.
",
+ "type": "IssueRelatesToOrderField!",
+ "id": "issuerelatestoorderfield",
+ "href": "/graphql/reference/issues#enum-issuerelatestoorderfield"
+ }
+ ],
+ "category": "issues"
+ },
{
"name": "IssueStateUpdateInput",
"id": "issuestateupdateinput",
@@ -12960,6 +13166,38 @@
],
"category": "issues"
},
+ {
+ "name": "RemoveRelatesToInput",
+ "id": "removerelatestoinput",
+ "href": "/graphql/reference/issues#input-object-removerelatestoinput",
+ "description": "Autogenerated input type of RemoveRelatesTo.
",
+ "inputFields": [
+ {
+ "name": "clientMutationId",
+ "description": "A unique identifier for the client performing the mutation.
",
+ "type": "String",
+ "id": "string",
+ "href": "/graphql/reference/other#scalar-string"
+ },
+ {
+ "name": "issueId",
+ "description": "The ID of the issue.
",
+ "type": "ID!",
+ "id": "id",
+ "href": "/graphql/reference/other#scalar-id",
+ "isDeprecated": false
+ },
+ {
+ "name": "relatedIssueId",
+ "description": "The ID of the previously related issue.
",
+ "type": "ID!",
+ "id": "id",
+ "href": "/graphql/reference/other#scalar-id",
+ "isDeprecated": false
+ }
+ ],
+ "category": "issues"
+ },
{
"name": "RemoveSubIssueInput",
"id": "removesubissueinput",
diff --git a/src/graphql/data/ghec/schema.docs.graphql b/src/graphql/data/ghec/schema.docs.graphql
index e3c1e2b7b8b9..94fb6074bb62 100644
--- a/src/graphql/data/ghec/schema.docs.graphql
+++ b/src/graphql/data/ghec/schema.docs.graphql
@@ -1253,6 +1253,46 @@ type AddReactionPayload {
subject: Reactable
}
+"""
+Autogenerated input type of AddRelatesTo
+"""
+input AddRelatesToInput {
+ """
+ A unique identifier for the client performing the mutation.
+ """
+ clientMutationId: String
+
+ """
+ The ID of the issue.
+ """
+ issueId: ID! @possibleTypes(concreteTypes: ["Issue"])
+
+ """
+ The ID of the related issue.
+ """
+ relatedIssueId: ID! @possibleTypes(concreteTypes: ["Issue"])
+}
+
+"""
+Autogenerated return type of AddRelatesTo.
+"""
+type AddRelatesToPayload {
+ """
+ A unique identifier for the client performing the mutation.
+ """
+ clientMutationId: String
+
+ """
+ The source issue.
+ """
+ issue: Issue
+
+ """
+ The related issue.
+ """
+ relatedIssue: Issue
+}
+
"""
Autogenerated input type of AddStar
"""
@@ -20715,6 +20755,36 @@ type Issue implements Assignable &
orderBy: ReactionOrder
): ReactionConnection!
+ """
+ A list of issues related to this issue.
+ """
+ relatesTo(
+ """
+ Returns the elements in the list that come after the specified cursor.
+ """
+ after: String
+
+ """
+ Returns the elements in the list that come before the specified cursor.
+ """
+ before: String
+
+ """
+ Returns the first _n_ elements from the list.
+ """
+ first: Int
+
+ """
+ Returns the last _n_ elements from the list.
+ """
+ last: Int
+
+ """
+ Ordering options for related issues
+ """
+ orderBy: IssueRelatesToOrder = {field: RELATES_TO_ADDED_AT, direction: DESC}
+ ): IssueConnection!
+
"""
The repository associated with this node.
"""
@@ -22700,6 +22770,36 @@ enum IssueOrderField @docsCategory(name: "issues") {
UPDATED_AT
}
+"""
+Ordering options for related issues
+"""
+input IssueRelatesToOrder @docsCategory(name: "issues") {
+ """
+ The ordering direction.
+ """
+ direction: OrderDirection!
+
+ """
+ The field to order related issues by.
+ """
+ field: IssueRelatesToOrderField!
+}
+
+"""
+Properties by which related issues can be ordered.
+"""
+enum IssueRelatesToOrderField @docsCategory(name: "issues") {
+ """
+ Order related issues by the creation time of the related issue
+ """
+ CREATED_AT
+
+ """
+ Order related issues by time of when the relates-to relationship was added
+ """
+ RELATES_TO_ADDED_AT
+}
+
"""
Type of issue search performed
"""
@@ -27440,6 +27540,16 @@ type Mutation @docsCategory(name: "meta") {
input: AddReactionInput!
): AddReactionPayload @docsCategory(name: "reactions")
+ """
+ Adds a 'relates to' relationship between two issues.
+ """
+ addRelatesTo(
+ """
+ Parameters for AddRelatesTo
+ """
+ input: AddRelatesToInput!
+ ): AddRelatesToPayload @docsCategory(name: "issues")
+
"""
Adds a star to a Starrable.
"""
@@ -28913,6 +29023,16 @@ type Mutation @docsCategory(name: "meta") {
input: RemoveReactionInput!
): RemoveReactionPayload @docsCategory(name: "reactions")
+ """
+ Removes a 'relates to' relationship between two issues.
+ """
+ removeRelatesTo(
+ """
+ Parameters for RemoveRelatesTo
+ """
+ input: RemoveRelatesToInput!
+ ): RemoveRelatesToPayload @docsCategory(name: "issues")
+
"""
Removes a star from a Starrable.
"""
@@ -50403,6 +50523,46 @@ type RemoveReactionPayload {
subject: Reactable
}
+"""
+Autogenerated input type of RemoveRelatesTo
+"""
+input RemoveRelatesToInput {
+ """
+ A unique identifier for the client performing the mutation.
+ """
+ clientMutationId: String
+
+ """
+ The ID of the issue.
+ """
+ issueId: ID! @possibleTypes(concreteTypes: ["Issue"])
+
+ """
+ The ID of the previously related issue.
+ """
+ relatedIssueId: ID! @possibleTypes(concreteTypes: ["Issue"])
+}
+
+"""
+Autogenerated return type of RemoveRelatesTo.
+"""
+type RemoveRelatesToPayload {
+ """
+ A unique identifier for the client performing the mutation.
+ """
+ clientMutationId: String
+
+ """
+ The previously targeted issue.
+ """
+ issue: Issue
+
+ """
+ The previously related issue.
+ """
+ relatedIssue: Issue
+}
+
"""
Autogenerated input type of RemoveStar
"""
diff --git a/src/graphql/lib/categories.ts b/src/graphql/lib/categories.ts
index ab62decb8d54..76eb35cd02c9 100644
--- a/src/graphql/lib/categories.ts
+++ b/src/graphql/lib/categories.ts
@@ -1,9 +1,4 @@
-// Canonical mapping of internal schema kinds to:
-// - urlKind: the URL/folder segment used in href anchors before categorization
-// (kept for backward-compat with `helpers.getFullLink` signature)
-// - slugPrefix: the kind-disambiguating slug prefix used on category pages
-// so two items sharing a case-insensitive name don't collide
-// - label: human-readable label rendered as a Primer Label next to each item
+// Schema kind tables keep legacy URL segments, category-page slug prefixes, and visible labels.
export type SchemaKindKey =
| 'queries'
@@ -26,8 +21,7 @@ export const KIND_LABELS: Record = {
scalars: 'Scalar',
}
-// Plural form of `KIND_LABELS`, used as the section heading (and mini-TOC
-// parent label) when a GraphQL category page groups its items by kind.
+// Plural labels appear in category-page sections and mini-TOC section entries.
export const KIND_LABELS_PLURAL: Record = {
queries: 'Queries',
mutations: 'Mutations',
@@ -39,10 +33,7 @@ export const KIND_LABELS_PLURAL: Record = {
scalars: 'Scalars',
}
-// Slug prefix used to disambiguate items across kinds on a category page.
-// For example, a `Repository` object and a `repository` query both have id
-// `repository`; on a category page they become `object-repository` and
-// `query-repository` respectively.
+// Category-page anchors prefix the kind, so Repository object and repository query stay distinct.
export const KIND_SLUG_PREFIX: Record = {
queries: 'query',
mutations: 'mutation',
@@ -54,9 +45,8 @@ export const KIND_SLUG_PREFIX: Record = {
scalars: 'scalar',
}
-// The "URL kind" / `pageType` value used by `helpers.getTypeKind` and
-// `helpers.getFullLink`. `inputObjects` (camelCase internal key) becomes
-// `input-objects` in URLs.
+// These URL segments match helpers.getTypeKind output and helpers.getFullLink input.
+// For example, inputObjects becomes input-objects.
export const KIND_URL_SEGMENT: Record = {
queries: 'queries',
mutations: 'mutations',
@@ -79,28 +69,22 @@ export const ALL_KIND_KEYS: SchemaKindKey[] = [
'scalars',
]
-// Reverse map from the URL-kind segment used in hrefs (e.g. `input-objects`)
-// to the slug prefix used to disambiguate items in category page anchors
-// (e.g. `input-object`). Derived from KIND_URL_SEGMENT + KIND_SLUG_PREFIX so
-// the three tables stay in sync automatically.
+// Derive URL-segment to slug-prefix mappings from the source tables so anchor helpers stay in sync.
+// For example, input-objects maps to input-object.
export const SLUG_PREFIX_BY_URL_SEGMENT: Record = Object.fromEntries(
ALL_KIND_KEYS.map((k) => [KIND_URL_SEGMENT[k], KIND_SLUG_PREFIX[k]]),
)
-// Given a URL-kind segment (as returned by helpers.getTypeKind, e.g.
-// `objects`, `input-objects`), return the slug prefix used to disambiguate
-// items in category page anchors. Falls back to the input for unknown kinds.
+// Unknown URL-kind segments fall back to themselves so callers can handle future kinds.
export function slugPrefixForUrlKind(urlKind: string): string {
return SLUG_PREFIX_BY_URL_SEGMENT[urlKind] ?? urlKind
}
-// Bucket all items that don't have an upstream `@docsCategory` directive.
+// Unannotated upstream schema items fall into the other category.
export const OTHER_CATEGORY = 'other'
-// Canonical list of categories emitted by the upstream `docs_category` DSL.
-// Keep this list in sync with the allowlist in
-// `github/github`'s `app/platform/objects/base/docs_category.rb`.
-// `other` is a docs-internal bucket for un-annotated types.
+// github/github app/platform/objects/base/docs_category.rb must allow each upstream category here.
+// The other category belongs to docs-internal for unannotated types.
export const CATEGORIES = [
'actions',
'activity',
@@ -153,7 +137,6 @@ export function isValidCategory(slug: string): slug is CategorySlug {
return (CATEGORIES as readonly string[]).includes(slug)
}
-// Human-readable display title for a category. Falls back to slug.
export function categoryTitle(slug: string): string {
switch (slug) {
case 'apps':
diff --git a/src/graphql/lib/index.ts b/src/graphql/lib/index.ts
index 96a7818f113c..8f0cfd8a8da2 100644
--- a/src/graphql/lib/index.ts
+++ b/src/graphql/lib/index.ts
@@ -13,27 +13,23 @@ import type {
} from '@/graphql/components/types'
import { ALL_KIND_KEYS, CATEGORIES, isValidCategory, type SchemaKindKey } from './categories'
-// GraphqlContext describes the per-request context object that getMiniToc and
-// getGraphqlSchema read language/version from.
export interface GraphqlContext {
currentLanguage: string
currentVersion: string
[key: string]: unknown
}
-// The GraphQL schema JSON is keyed by member type (e.g. "queries", "objects",
-// "enums"), each holding a list of schema members.
+// GraphQL schema JSON groups members by schema kind.
type GraphqlSchemaData = Record
export const GRAPHQL_DATA_DIR = 'src/graphql/data'
-/* ADD LANGUAGE KEY */
const previews = new Map()
const upcomingChanges = new Map()
const changelog = new Map()
const changelogMiniTocs = new Map()
-// Per-category schema files. Key: `${graphqlVersion}:${category}` → bucket.
+// Per-category schema cache keys combine graphqlVersion and category.
const graphqlCategorySchemas = new Map()
-// All objects across categories (for interface implementer lookup).
+// Interface renderers need object items from every category to list implementers.
const allObjectsByVersion = new Map()
const miniTocs = new Map>>()
@@ -41,9 +37,7 @@ for (const language of Object.keys(languages)) {
miniTocs.set(language, new Map())
}
-// Returns the per-category schema bucket `{queries, mutations, ...}` for a
-// given category slug (e.g. 'repos', 'issues'). Throws via the loader if the
-// category slug is not valid for this version.
+// Reject invalid category slugs before the loader reads a missing schema file.
export function getGraphqlSchema(version: string, category: string): GraphqlSchemaData {
if (!isValidCategory(category)) {
throw new Error(`Invalid GraphQL category: ${category}`)
@@ -65,9 +59,7 @@ function getGraphqlSchemaByCategory(graphqlVersion: string, category: string): G
return graphqlCategorySchemas.get(key)!
}
-// Returns all object-kind items across every category for the given version.
-// Used by the interface renderer to list implementers regardless of which
-// category page is being rendered.
+// Interface renderers need objects from every category to list implementers.
export function getAllGraphqlObjects(version: string): GraphqlT[] {
const graphqlVersion: string = getGraphqlVersion(version)
if (!allObjectsByVersion.has(graphqlVersion)) {
@@ -81,7 +73,6 @@ export function getAllGraphqlObjects(version: string): GraphqlT[] {
return allObjectsByVersion.get(graphqlVersion)!
}
-// Returns the canonical render order of kinds within a category page.
export function getKindOrder(): SchemaKindKey[] {
return ALL_KIND_KEYS
}
@@ -100,17 +91,11 @@ export function getGraphqlChangelog(version: string): ChangelogItemT[] {
return changelog.get(graphqlVersion)!
}
-/**
- * Return changelog entries filtered by year.
- */
export function getGraphqlChangelogByYear(version: string, year: number): ChangelogItemT[] {
const all = getGraphqlChangelog(version)
return all.filter((entry) => entry.date.startsWith(String(year)))
}
-/**
- * Return the distinct years present in the changelog, sorted descending (newest first).
- */
export function getGraphqlChangelogYears(version: string): number[] {
const all = getGraphqlChangelog(version)
const years = new Set()
diff --git a/src/graphql/lib/validator.ts b/src/graphql/lib/validator.ts
index 89283c83e5a5..eac02970e166 100644
--- a/src/graphql/lib/validator.ts
+++ b/src/graphql/lib/validator.ts
@@ -1,5 +1,4 @@
-// the tests in tests/graphql.ts use this schema to ensure the integrity
-// of the data in src/graphql/data/*.json
+// src/graphql/tests/validate-schema.ts reads these schemas to validate generated data.
interface JSONSchema {
type?: string
@@ -79,7 +78,6 @@ export const upcomingChangesValidator: ValidatorSchema = {
},
}
-// many GraphQL schema members have these core properties
const coreProps: JSONSchema = {
properties: {
name: {
@@ -107,7 +105,6 @@ const coreProps: JSONSchema = {
},
}
-// some GraphQL schema members have the core properties plus an 'args' object
const corePropsPlusArgs = dup(coreProps)
corePropsPlusArgs.properties!.args = {
@@ -118,7 +115,6 @@ corePropsPlusArgs.properties!.args = {
},
}
-// the args object can have defaultValue prop
corePropsPlusArgs.properties!.args.items!.properties!.defaultValue = {
type: 'boolean',
}
diff --git a/src/graphql/pages/breaking-changes.tsx b/src/graphql/pages/breaking-changes.tsx
index b39033517190..1f960e9c4760 100644
--- a/src/graphql/pages/breaking-changes.tsx
+++ b/src/graphql/pages/breaking-changes.tsx
@@ -49,9 +49,7 @@ export const getServerSideProps: GetServerSideProps = async (context) =>
const schema = getGraphqlBreakingChanges(currentVersion)
if (!schema) throw new Error(`No graphql breaking changes schema found for ${currentVersion}`)
- // Gets the miniTocItems in the article context. At this point it will only
- // include miniTocItems that exist in Markdown pages in
- // content/graphql/reference/*
+ // Start from the current page's Markdown headings, then append the generated ones.
const automatedPageContext = getAutomatedPageContextFromRequest(req)
const slugger = new GithubSlugger()
const headings = Object.fromEntries(
@@ -69,7 +67,6 @@ export const getServerSideProps: GetServerSideProps = async (context) =>
)
const titles = Object.values(headings).map((heading) => heading.title)
const changelogMiniTocItems = await getAutomatedPageMiniTocItems(titles, req.context!, 2)
- // Update the existing context to include the miniTocItems from GraphQL
automatedPageContext.miniTocItems.push(...changelogMiniTocItems)
return {
diff --git a/src/graphql/pages/changelog.tsx b/src/graphql/pages/changelog.tsx
index 1aee0f1921bd..74b9bfb9f9e5 100644
--- a/src/graphql/pages/changelog.tsx
+++ b/src/graphql/pages/changelog.tsx
@@ -69,10 +69,7 @@ export const getServerSideProps: GetServerSideProps = async (context) =>
}
}
-/**
- * Strip wrapping `` tags from HTML change descriptions to allow
- * rendering as `
` content without nested block elements.
- */
+// Strip wrapping p tags so list items do not contain nested block elements.
export function stripParagraphWrappers(schema: ChangelogItemT[]) {
for (const item of schema) {
for (const group of [item.schemaChanges, item.previewChanges, item.upcomingChanges]) {
diff --git a/src/graphql/pages/reference.tsx b/src/graphql/pages/reference.tsx
index bef78e785891..77dba5be2983 100644
--- a/src/graphql/pages/reference.tsx
+++ b/src/graphql/pages/reference.tsx
@@ -40,10 +40,7 @@ export default function GraphqlReferencePage({
allObjects,
categorySlug,
}: Props) {
- // Key the schema content by category slug. Without this, client-side
- // navigation between category pages reuses the same React tree and
- // dangerouslySetInnerHTML descriptions from a previous category can stick
- // around in the DOM. Keying forces a clean unmount/remount on route change.
+ // Keying by categorySlug prevents navigation from reusing stale dangerouslySetInnerHTML content.
const content =
@@ -74,12 +71,7 @@ export const getServerSideProps: GetServerSideProps = async (context) =>
const schema = getGraphqlSchema(currentVersion, page) as CategorySchema
const allObjects = getAllGraphqlObjects(currentVersion) as ObjectT[]
- // If a category has no types in the current version, 404 the page rather
- // than render an empty document. Empty buckets typically happen when a
- // category exists in fpt/ghec but not in GHES (or vice versa). The content
- // .md files for categories that are empty in every version are removed
- // from `content/graphql/reference/`, but the dynamic [page].tsx route would
- // still serve them otherwise. This guard makes the response a real 404.
+ // Return 404 when a version has no types because the route can serve categories without files.
const hasAnyTypes = ALL_KIND_KEYS.some((kind) => {
const items = (schema as Record)[kind]
return Array.isArray(items) && items.length > 0
@@ -88,12 +80,7 @@ export const getServerSideProps: GetServerSideProps = async (context) =>
return { notFound: true }
}
- // Build a two-level mini-TOC mirroring the page's kind sections. Top-level
- // entries are kind labels (e.g. "Objects") pointing at the matching section
- // heading; nested entries are the items inside each section. The mini-TOC
- // React component (`MiniTocs`) renders `item.items` recursively, so pushing
- // a nested structure here yields a two-level sidebar even though the
- // default heading-collection path is capped at one level globally.
+ // Build nested kind entries because MiniTocs recurses and default collection stops at one level.
const automatedPageContext = getAutomatedPageContextFromRequest(req)
for (const kind of ALL_KIND_KEYS) {
const kindItems = (schema as Record>)[kind]
diff --git a/src/graphql/pages/schema-previews.tsx b/src/graphql/pages/schema-previews.tsx
index f2eda650a55f..23d781ae7cfc 100644
--- a/src/graphql/pages/schema-previews.tsx
+++ b/src/graphql/pages/schema-previews.tsx
@@ -45,13 +45,10 @@ export const getServerSideProps: GetServerSideProps = async (context) =>
const schema = getPreviews(currentVersion) as PreviewT[]
if (!schema) throw new Error(`No graphql preview schema found for ${currentVersion}`)
- // Gets the miniTocItems in the article context. At this point it will only
- // include miniTocItems that exist in Markdown pages in
- // content/graphql/reference/*
+ // Start from the current page's Markdown headings, then append the generated ones.
const automatedPageContext = getAutomatedPageContextFromRequest(req)
const titles = schema.map((item) => item.title)
const changelogMiniTocItems = await getAutomatedPageMiniTocItems(titles, req.context!, 2)
- // Update the existing context to include the miniTocItems from GraphQL
automatedPageContext.miniTocItems.push(...changelogMiniTocItems)
const mainContext = await getMainContext(req, res as unknown as Response)
diff --git a/src/graphql/scripts/build-changelog.ts b/src/graphql/scripts/build-changelog.ts
index 0b645a4e7f52..319c2a5d6a8b 100644
--- a/src/graphql/scripts/build-changelog.ts
+++ b/src/graphql/scripts/build-changelog.ts
@@ -60,10 +60,7 @@ interface IgnoredChangesSummary {
let lastIgnoredChanges: Change[] = []
-/**
- * Tag `changelogEntry` with `date: YYYY-mm-dd`, then prepend it to the JSON
- * structure written to `targetPath`. (`changelogEntry` and that file are modified in place.)
- */
+// Add today's date to changelogEntry and prepend it to the JSON array at targetPath.
export function prependDatedEntry(changelogEntry: ChangelogEntry, targetPath: string): void {
const todayString = new Date().toISOString().slice(0, 10)
changelogEntry.date = todayString
@@ -73,17 +70,13 @@ export function prependDatedEntry(changelogEntry: ChangelogEntry, targetPath: st
previousChangelog.unshift(changelogEntry)
fs.writeFileSync(targetPath, JSON.stringify(previousChangelog, null, 2))
- // Ensure a content page exists for this entry's year
const year = todayString.slice(0, 4)
ensureYearPage(year)
}
const DEFAULT_CHANGELOG_CONTENT_DIR = nodePath.join('content', 'graphql', 'overview', 'changelog')
-/**
- * If a year-specific content page doesn't exist yet (e.g. 2027.md),
- * create it and prepend it to the children list in index.md.
- */
+// Create a missing year page and prepend it to the changelog index when its first entry arrives.
export function ensureYearPage(
year: string,
contentDir: string = DEFAULT_CHANGELOG_CONTENT_DIR,
@@ -110,12 +103,6 @@ export function ensureYearPage(
fs.writeFileSync(indexPath, updated)
}
-/**
- * Compare `oldSchemaString` to `newSchemaString`, and if there are any
- * changes that warrant a changelog entry, return a changelog entry.
- * Based on the parsed `previews`, identify changes that are under a preview.
- * Otherwise, return null.
- */
export async function createChangelogEntry(
oldSchemaString: string,
newSchemaString: string,
@@ -159,8 +146,7 @@ export async function createChangelogEntry(
)
const addedUpcomingChanges = newUpcomingChanges.filter(function (change): boolean {
- // Manually check each of `newUpcomingChanges` for an equivalent entry
- // in `oldUpcomingChanges`.
+ // Match upcoming changes by location, date, and description.
return !oldUpcomingChanges.find(function (oldChange) {
return (
oldChange.location === change.location &&
@@ -189,7 +175,6 @@ export async function createChangelogEntry(
)
const schemaChange: ChangelogSchemaChange = {
title: 'The GraphQL schema includes these changes:',
- // Replace single quotes which wrap field/argument/type names with backticks
changes: renderedScheamChanges,
}
changelogEntry.schemaChanges.push(schemaChange)
@@ -236,9 +221,7 @@ export async function createChangelogEntry(
}
}
-/**
- * Prepare the preview title from github/github source for the docs.
- */
+// github/github preview titles need docs-style wording before rendering.
export function cleanPreviewTitle(title: string): string {
if (title === 'UpdateRefsPreview') {
title = 'Update refs preview'
@@ -250,10 +233,7 @@ export function cleanPreviewTitle(title: string): string {
return title
}
-/**
- * Turn the given title into an HTML-ready anchor.
- * (ported from graphql-docs/lib/graphql_docs/update_internal_developer/change_log.rb#L281)
- */
+// Anchor generation matches the changelog URL format.
export function previewAnchor(previewTitle: string): string {
return previewTitle
.toLowerCase()
@@ -261,29 +241,18 @@ export function previewAnchor(previewTitle: string): string {
.replace(/[^\w-]/g, '')
}
-/**
- * Turn changes from graphql-inspector into messages for the HTML changelog.
- */
export function cleanMessagesFromChanges(changes: Change[]): string[] {
return changes.map(function (change): string {
- // replace single quotes around graphql names with backticks,
- // to match previous behavior from graphql-schema-comparator
+ // Wrap quoted GraphQL names in Markdown code spans for changelog rendering.
return change.message.replace(/'([a-zA-Z. :!]+)'/g, '`$1`')
})
}
-/**
- * Split `changesToReport` into two parts,
- * one for changes in the main schema,
- * and another for changes that are under preview.
- * (Ported from /graphql-docs/lib/graphql_docs/update_internal_developer/change_log.rb#L230)
- */
+// Preview-toggled paths and their ancestors move changes out of the main schema section.
export function segmentPreviewChanges(
changesToReport: Change[],
previews: Preview[],
): SegmentedChanges {
- // Build a map of `{ path => previewTitle` }
- // for easier lookup of change to preview
const pathToPreview: Record = {}
for (const preview of previews) {
for (const path of preview.toggled_on) {
@@ -294,8 +263,7 @@ export function segmentPreviewChanges(
const changesByPreview: Record = {}
for (const change of changesToReport) {
- // For each change, see if its path _or_ one of its ancestors
- // is covered by a preview. If it is, mark this change as belonging to a preview
+ // Preview ownership applies when the change path or an ancestor path is toggled on.
const pathParts = change.path?.split('.') || []
let testPath: string | null = null
let previewTitle: string | null = null
@@ -303,8 +271,6 @@ export function segmentPreviewChanges(
while (pathParts.length > 0 && !previewTitle) {
testPath = pathParts.join('.')
previewTitle = pathToPreview[testPath]
- // If that path didn't find a match, then we'll
- // check the next ancestor.
pathParts.pop()
}
if (previewTitle) {
@@ -322,11 +288,8 @@ export function segmentPreviewChanges(
return { schemaChangesToReport: schemaChanges, previewChangesToReport: changesByPreview }
}
-// We only want to report changes to schema structure.
-// Deprecations are covered by "upcoming changes."
-// By listing the changes explicitly here, we can make sure that,
-// if the library changes, we don't miss publishing anything that we mean to.
-// This was originally ported from graphql-docs/lib/graphql_docs/update_internal_developer/change_log.rb#L35-L103
+// Report only schema-structure changes; deprecations come from upcoming changes.
+// Unknown change types log for review instead of appearing in the changelog.
const CHANGES_TO_REPORT = [
ChangeType.FieldArgumentDefaultChanged,
ChangeType.FieldArgumentTypeChanged,
@@ -354,9 +317,6 @@ const CHANGES_TO_REPORT = [
ChangeType.DirectiveUsageFieldDefinitionRemoved,
]
-// Anything not in CHANGES_TO_REPORT is logged as ignored rather than reported,
-// so a new change type added upstream cannot break this script.
-
export function getLastIgnoredChanges(): Change[] {
return lastIgnoredChanges
}
diff --git a/src/graphql/scripts/sync.ts b/src/graphql/scripts/sync.ts
index 9bcc3257bb38..d7c6b8e2e06b 100755
--- a/src/graphql/scripts/sync.ts
+++ b/src/graphql/scripts/sync.ts
@@ -58,17 +58,13 @@ const dataFilenames = JSON.parse(
await fs.readFile('src/graphql/scripts/utils/data-filenames.json', 'utf8'),
)
-// check for required PAT
if (!process.env.GITHUB_TOKEN) {
throw new Error('Error! You must have a GITHUB_TOKEN set in an .env file to run this script.')
}
const versionsToBuild = Object.keys(allVersions)
-// Tracks, per category, the set of docs versions in which the category has at
-// least one type. Populated inside the per-version loop and consumed after it
-// to manage the per-category content pages. Declared before `main()` runs so
-// the loop never reads it in the temporal dead zone.
+// Declare categoryPresence before the main() call so the loop never reads it in the temporal dead zone.
const categoryPresence: CategoryPresence = new Map()
main()
@@ -77,12 +73,9 @@ const allIgnoredChanges: IgnoredChange[] = []
async function main() {
for (const version of versionsToBuild) {
- // Get the relevant GraphQL name for the current version.
- // For example, free-pro-team@latest corresponds to dotcom,
- // enterprise-server@2.22 corresponds to ghes-2.22.
+ // Examples: free-pro-team@latest maps to dotcom; enterprise-server@2.22 maps to ghes-2.22.
const graphqlVersion = allVersions[version].openApiVersionName
- // 1. UPDATE PREVIEWS
const previewsPath = getDataFilepath('previews', graphqlVersion)
const rawPreviews = load(
await getRemoteRawContent(previewsPath, graphqlVersion),
@@ -94,7 +87,6 @@ async function main() {
path.join(graphqlStaticDir, graphqlVersion, 'previews.json'),
)
- // 2. UPDATE UPCOMING CHANGES
const upcomingChangesPath = getDataFilepath('upcomingChanges', graphqlVersion)
const previousUpcomingChanges = load(
await fs.readFile(upcomingChangesPath, 'utf8'),
@@ -107,8 +99,6 @@ async function main() {
path.join(graphqlStaticDir, graphqlVersion, 'upcoming-changes.json'),
)
- // 3. UPDATE SCHEMAS
- // note: schemas live in separate files per version
const previewFilePath = getDataFilepath('schemas', graphqlVersion)
const previousSchemaString = await fs.readFile(previewFilePath, 'utf8')
const latestSchema = await getRemoteRawContent(previewFilePath, graphqlVersion)
@@ -117,11 +107,7 @@ async function main() {
...preview,
toggled_by: [preview.toggled_by].flat(),
}))
- // Fallback category source for GHES versions that pre-date the upstream
- // `@docsCategory` DSL (DSL landed on master 2026-05-07; GHES 3.16-3.21
- // were cut at the 3.21 freeze 2026-03-19). Without this, every type on
- // those versions gets bucketed as "other". GHES 3.22+ is expected to
- // include the DSL natively so it's excluded from the fallback.
+ // GHES schemas before 3.22 lack @docsCategory, so fall back to the fpt category map.
let fallbackCategoryMap: Record> | undefined
const ghesMatch = /^ghes-(\d+)\.(\d+)$/.exec(graphqlVersion)
if (ghesMatch) {
@@ -134,8 +120,7 @@ async function main() {
)
console.log(`Using fpt/category-map.json as @docsCategory fallback for ${graphqlVersion}`)
} catch {
- // fpt hasn't been processed yet (shouldn't happen given iteration
- // order, but stay defensive). Without it, ghes types fall to "other".
+ // fpt runs first; if category-map.json is unavailable, GHES types fall back to other.
}
}
}
@@ -144,18 +129,13 @@ async function main() {
previewsForSchema,
fallbackCategoryMap,
{ currentLanguage: 'en', currentVersion: version },
- ) // This is slow!
+ )
- // Split the schema by category so the runtime can lazily load only the
- // bucket it needs for a given page request. The monolithic `schema.json`
- // is no longer written; per-category files are the only on-disk format.
+ // processSchemas is slow; per-category files are the only on-disk format for scoped loads.
const perCategoryFiles = bucketSchemaByCategory(schemaJsonPerVersion)
await writeCategoryFiles(path.join(graphqlStaticDir, graphqlVersion), perCategoryFiles)
- // Record which categories have at least one type in this version so the
- // content pages and their `versions` frontmatter can be managed after the
- // loop. `version` is the docs version key (e.g. `enterprise-server@3.22`),
- // which is the format `convertVersionsToFrontmatter` expects.
+ // Store docs version keys so convertVersionsToFrontmatter can update pages after the loop.
for (const [cat, bucket] of perCategoryFiles.entries()) {
const hasTypes = ALL_KIND_KEYS.some((kind) => (bucket[kind]?.length ?? 0) > 0)
if (!hasTypes) continue
@@ -163,9 +143,8 @@ async function main() {
categoryPresence.get(cat)!.add(version)
}
- // 4. UPDATE CHANGELOG
if (allVersions[version].nonEnterpriseDefault) {
- // The changelog is only built for free-pro-team@latest
+ // Build the changelog only for free-pro-team@latest.
const changelogEntry = await createChangelogEntry(
previousSchemaString,
latestSchema,
@@ -180,7 +159,6 @@ async function main() {
)
}
- // Capture ignored changes for potential workflow notifications
const ignoredSummary = getIgnoredChangesSummary()
if (ignoredSummary) {
allIgnoredChanges.push({
@@ -191,15 +169,11 @@ async function main() {
}
}
- // Manage the per-category content pages (create new categories, delete
- // emptied ones, narrow `versions` frontmatter) plus the reference index
- // children and disappearance redirects, based on the presence collected above.
+ // Sync category pages, index children, and disappearance redirects after all versions run.
await syncCategoryContentFiles(categoryPresence)
- // Run the YAML linter before anything is checked in.
execSync('npx prettier -w "**/*.{yml,yaml}"')
- // Output ignored changes for GitHub Actions
if (allIgnoredChanges.length > 0) {
const totalIgnored = allIgnoredChanges.reduce((sum, item) => sum + item.totalCount, 0)
const uniqueTypes = [
@@ -221,14 +195,12 @@ async function main() {
}
}
-// get latest from github/github
async function getRemoteRawContent(filepath: string, graphqlVersion: string) {
const options: GitHubRepoOptions = {
owner: 'github',
repo: 'github',
}
- // find the relevant branch in github/github and set it as options.ref
let t0 = new Date().getTime()
options.ref = await getBranchAsRef(options, graphqlVersion)
let took = new Date().getTime() - t0
@@ -244,11 +216,10 @@ async function getRemoteRawContent(filepath: string, graphqlVersion: string) {
return contents
}
-// find the relevant filepath in src/graphql/scripts/util/data-filenames.json
function getDataFilepath(id: string, graphqlVersion: string) {
const versionType = getVersionName(graphqlVersion)
- // for example, dataFilenames['schema']['ghes'] = schema.docs-enterprise.graphql
+ // Example: dataFilenames.schema.ghes maps to schema.docs-enterprise.graphql.
const filename = dataFilenames[id][versionType]
return path.join(graphqlStaticDir, graphqlVersion, filename)
@@ -268,15 +239,12 @@ async function getBranchAsRef(
ghes: `enterprise-${graphqlVersion.replace('ghes-', '')}-release`,
}
- // the first time this runs, it uses the branch found for the version above
if (!branch) branch = branches[versionType]
const ref = `heads/${branch}`
- // check whether the branch can be found in github/github
const exists = await hasMatchingRef(options.owner, options.repo, ref)
- // if ref is not found, the branch cannot be found, so try a fallback
if (!exists) {
const fallbackBranch = defaultBranch
return await getBranchAsRef(options, graphqlVersion, fallbackBranch)
@@ -284,8 +252,7 @@ async function getBranchAsRef(
return ref
}
-// given a GraphQL version like `ghes-2.22`, return `ghes`;
-// given a GraphQL version like `dotcom`, return as is
+// Examples: ghes-2.22 returns ghes; dotcom returns dotcom.
function getVersionName(graphqlVersion: string) {
return graphqlVersion.split('-')[0]
}
@@ -296,8 +263,7 @@ async function updateFile(filepath: string, content: string) {
return fs.writeFile(filepath, content, 'utf8')
}
-// JSON data from GraphQL schema processing - complex nested structures
-// Serialize unknown shapes because the structure varies (arrays, objects, nested schemas, etc.)
+// Serialize unknown GraphQL shapes because schema processing returns nested arrays and objects.
async function updateStaticFile(json: unknown, filepath: string) {
console.log(`Updating static file ${filepath}`)
const jsonString = JSON.stringify(json, null, 2)
diff --git a/src/graphql/scripts/utils/bucket-by-category.ts b/src/graphql/scripts/utils/bucket-by-category.ts
index 794f1b85ea73..78c47487aada 100644
--- a/src/graphql/scripts/utils/bucket-by-category.ts
+++ b/src/graphql/scripts/utils/bucket-by-category.ts
@@ -9,15 +9,12 @@ import {
type SchemaKindKey,
} from '@/graphql/lib/categories'
-// Item shape from process-schemas; we only need the `category` field here so
-// we keep this loose to avoid pulling all the precise interfaces.
+// Keep this loose so bucket-by-category does not import every process-schemas interface.
type CategorizedItem = { category?: string; name?: string; id?: string }
export type CategoryBuckets = Map>>
-// Matches the legacy href format that process-schemas emits, e.g.
-// `/graphql/reference/objects#repository`. Captures the url-kind segment
-// and the id so the bucketer can rewrite into the category-aware form.
+// Example: /graphql/reference/objects#repository captures url kind objects and id repository.
const LEGACY_HREF_RE = /^\/graphql\/reference\/([a-z][a-z-]*)#([a-z0-9-]+)$/
type CategoryLookup = Map>
@@ -46,10 +43,7 @@ function rewriteHref(href: string, lookup: CategoryLookup): string {
return `/graphql/reference/${category}#${slugPrefixForUrlKind(urlKind)}-${id}`
}
-// Walk a processed item recursively, rewriting any string value that looks
-// like a legacy `/graphql/reference/#` href into the
-// category-aware form. Mutates in place; the monolithic schema.json has
-// already been written to disk before this runs.
+// rewriteHrefsInPlace mutates processed items so category files link to sibling files.
function rewriteHrefsInPlace(value: unknown, lookup: CategoryLookup): void {
if (Array.isArray(value)) {
for (const v of value) rewriteHrefsInPlace(v, lookup)
@@ -68,9 +62,6 @@ function rewriteHrefsInPlace(value: unknown, lookup: CategoryLookup): void {
}
}
-// Group a processed schema (one big `{queries, mutations, ...}` object) into
-// one bucket per category. Each bucket only contains the kinds that have
-// items in that category.
export function bucketSchemaByCategory(
schema: Record,
): CategoryBuckets {
@@ -89,12 +80,7 @@ export function bucketSchemaByCategory(
}
}
- // After grouping, rewrite cross-reference hrefs from the legacy
- // `/graphql/reference/#` form into the category-aware
- // `/graphql/reference/#-` form so per-category
- // files link to their sibling files. The monolithic `schema.json` is
- // serialized to disk before this runs (see sync.ts), so it keeps the
- // legacy hrefs the existing runtime expects.
+ // Rewriting happens after buckets exist, so hrefs can point to sibling category files.
const lookup = buildCategoryLookup(buckets)
for (const bucket of buckets.values()) {
rewriteHrefsInPlace(bucket, lookup)
@@ -103,13 +89,10 @@ export function bucketSchemaByCategory(
return buckets
}
-// Write `schema-.json` files into `dir`. Categories with no items
-// for this version get an empty file so the loader has a deterministic file
-// to consume (rather than relying on filesystem stat).
+// Emit every schema-.json file so the loader never stats missing categories.
export async function writeCategoryFiles(dir: string, buckets: CategoryBuckets): Promise {
await fs.mkdir(dir, { recursive: true })
- // First, delete any stale schema-*.json files so a category that becomes
- // empty in a new sync doesn't leave behind a stale file.
+ // Remove schema-*.json files before writing, so categories with no items keep no data.
let existing: string[] = []
try {
existing = await fs.readdir(dir)
@@ -121,7 +104,7 @@ export async function writeCategoryFiles(dir: string, buckets: CategoryBuckets):
try {
await fs.unlink(path.join(dir, file))
} catch {
- // ignore
+ // Keep writing other category files if one stale file cannot be removed.
}
}
}
@@ -132,8 +115,7 @@ export async function writeCategoryFiles(dir: string, buckets: CategoryBuckets):
await fs.writeFile(filepath, JSON.stringify(bucket, null, 2), 'utf8')
}
- // Also emit a small category-map.json used at runtime by the GraphQL
- // category redirect middleware. Shape: { [kindKey]: { [id]: category } }
+ // category-map.json shape is { [kindKey]: { [id]: category } } for GraphQL redirects.
const categoryMap: Partial>> = {}
for (const kind of ALL_KIND_KEYS) {
const byId: Record = {}
diff --git a/src/graphql/scripts/utils/process-previews.ts b/src/graphql/scripts/utils/process-previews.ts
index 4a9197faa42f..8563ec9f7ccb 100644
--- a/src/graphql/scripts/utils/process-previews.ts
+++ b/src/graphql/scripts/utils/process-previews.ts
@@ -22,19 +22,17 @@ const inputOrPayload = /(Input|Payload)$/m
export default function processPreviews(previews: RawPreview[]): ProcessedPreview[] {
return previews.map((raw) => {
let title = sentenceCase(raw.title)
- .replace(/ -.+/, '') // remove any extra info that follows a hyphen
- .replace('it hub', 'itHub') // fix overcorrected `git hub` from sentenceCasing
- .replace(' s ', "'s ") // sentenceCase replaces apostrophes with spaces
+ .replace(/ -.+/, '')
+ .replace('it hub', 'itHub') // sentenceCase rewrites GitHub as Git hub.
+ .replace(' s ', "'s ") // sentenceCase replaces apostrophes with spaces.
- // Add `preview` to the end of titles if needed
title = title.endsWith('preview') ? title : `${title} preview`
- // filter out schema members that end in `Input` or `Payload`
+ // Preview pages omit generated Input and Payload members.
const toggled_on = raw.toggled_on.filter(
(schemaMember: string) => !inputOrPayload.test(schemaMember),
)
- // remove unnecessary leading colon
const toggled_by = raw.toggled_by.replace(':', '')
const accept_header = `application/vnd.github.${toggled_by}+json`
@@ -42,7 +40,6 @@ export default function processPreviews(previews: RawPreview[]): ProcessedPrevie
slugger.reset()
const href = `/graphql/overview/schema-previews#${slugger.slug(title)}`
- // Preserve all original properties except announcement/updates
return {
title,
description: raw.description,
diff --git a/src/graphql/scripts/utils/process-schemas.ts b/src/graphql/scripts/utils/process-schemas.ts
index 9abfd2995ee2..034691b44248 100755
--- a/src/graphql/scripts/utils/process-schemas.ts
+++ b/src/graphql/scripts/utils/process-schemas.ts
@@ -22,7 +22,6 @@ interface PreviewInfo {
toggled_by: string[]
}
-// Interface for arguments returned by helpers.getArguments()
interface FieldArgumentInfo {
name: string
// GraphQL scalar default values come through the AST as a string or boolean.
@@ -207,23 +206,16 @@ interface ProcessedSchemaData {
scalars: ScalarInfo[]
}
-// All processed items get an optional `category` field once the schema has
-// been categorized. Using `& { category: string }` at the type level would
-// require touching every interface, so we keep it loose here and rely on the
-// runtime guarantee that every emitted item has a category.
-
+// Category stays loose on emitted items to avoid duplicating it across every output interface.
const externalScalarsJSON: Array<{ name: string; description: string }> = JSON.parse(
await fs.readFile(path.join(process.cwd(), './src/graphql/lib/non-schema-scalars.json'), 'utf-8'),
)
const externalScalars: ScalarInfo[] = await Promise.all(
externalScalarsJSON.map(async (scalar): Promise => {
- // These live in a local JSON file rather than the versioned schema, and
- // their only link is external, so they need no version context.
+ // Local non-schema scalars have external links and need no docs version context.
const description = await baseHelpers.getDescription(scalar.description)
const id = baseHelpers.getId(scalar.name)
- // External scalars (e.g. Date, URI) are not annotated upstream and live
- // in the "other" bucket. Emit the legacy href; bucket-by-category will
- // rewrite it to the category-aware form for per-category files.
+ // External scalars like Date and URI start in other with hrefs for bucket rewriting.
const href = baseHelpers.getFullLink('scalars', id)
return {
name: scalar.name,
@@ -235,38 +227,40 @@ const externalScalars: ScalarInfo[] = await Promise.all(
}),
)
-// Shape of the per-version `category-map.json` used both at runtime by the
-// redirect middleware and (here) at build time as a fallback source of
-// categories when a schema lacks `@docsCategory` directives.
+// category-map.json supplies runtime redirects and build-time fallback.
+// The fallback covers schemas without @docsCategory.
type CategoryMapFallback = Partial>>
-// Selects and formats the schema data the docs need. Runs in the build step.
+// processSchemas assigns GraphQL categories before rendering. Explicit @docsCategory wins.
+// category-map.json fills GHES schemas without directives. Mutation inputs inherit their owning
+// mutation. Connection and Edge types come from graphql-ruby Relay pagination and inherit from
+// node, nodes, or edges. Unannotated enum, union, and input object targets inherit only when
+// every referrer resolves to one category. Interfaces do not contribute because they are
+// cross-cutting. Input object candidates propagate through nested inputs.
+// Examples include IssueTimelineItemsItemType, PullRequestTimelineItemsItemType,
+// RepositoryRuleType, RuleParameters, and RuleParametersInput. Candidate sets make derivation
+// order-independent and retain later conflicting referrers.
+// Input-suffixed objects stay included because docs pages exist outside the v4 sidebar.
+// https://developer.github.com/v4/input_object/acceptenterpriseadministratorinvitationinput/
+// Categories missing from CATEGORIES in src/graphql/lib/categories.ts normalize to other.
export default async function processSchemas(
idl: Buffer | string,
previewsPerVersion: PreviewInfo[],
- // Optional fallback used when the IDL itself has no `@docsCategory`
- // directives (e.g. GHES branches cut before the upstream DSL existed).
- // Lookups for type-level categories use the type id; mutations look up
- // by mutation field name under the `mutations` key.
+ // Optional fallback for schemas without @docsCategory.
+ // Type ids are keys, and the mutations map uses field names.
fallbackCategoryMap?: CategoryMapFallback,
- // The docs version being generated, e.g. `enterprise-server@3.22`. Without
- // it, links inside schema descriptions render without a version segment.
+ // Context carries the docs version key so schema description links get a version segment.
context: Context = {},
): Promise {
const helpers = createSchemaHelpers(context)
const schemaAST: DocumentNode = parse(idl.toString())
const schema: GraphQLSchema = buildASTSchema(schemaAST)
- // list of objects is used when processing mutations
const objectsInSchema = schemaAST.definitions.filter(
(def): def is ObjectTypeDefinitionNode => def.kind === 'ObjectTypeDefinition',
)
- // PASS 1: Build a typeId -> category map by reading the @docsCategory
- // directive on every categorizable definition. Queries derive their
- // category from the return type's category; mutations are annotated on
- // each Mutation root field rather than on a type, so we collect those
- // separately.
+ // Read @docsCategory before deriving fallback categories.
const typeCategoryMap = new Map()
const mutationFieldCategoryMap = new Map()
@@ -295,51 +289,32 @@ export default async function processSchemas(
}
}
- // Build a flat fallback id -> cat map across every type-level kind. (We
- // exclude queries: query categories are derived from the return type.
- // Mutations are kept separately since they're keyed by field name.)
+ // Fallback type categories skip queries and keep mutations keyed by field name.
const fallbackTypeMap: Record = {}
if (fallbackCategoryMap) {
for (const kind of Object.keys(fallbackCategoryMap)) {
if (kind === 'queries' || kind === 'mutations') continue
const sub = fallbackCategoryMap[kind] || {}
for (const id of Object.keys(sub)) {
- // First write wins; in practice ids don't collide across kinds.
+ // First write wins; ids do not collide across kinds in practice.
if (!(id in fallbackTypeMap)) fallbackTypeMap[id] = sub[id]
}
}
}
const fallbackMutationMap = fallbackCategoryMap?.mutations || {}
- // PASS 1.5: derive categories for types that github/github cannot annotate
- // directly. Two rules apply, both run before fallback / OTHER assignment so
- // they take effect for fpt and ghec (where the IDL has the annotations) and
- // also propagate into the per-version category-map.json that GHES <3.22
- // consumes as its fallback.
- //
- // (a) Input objects inherit from their owning mutation. The DSL can mark
- // a mutation field with @docsCategory but the generated *Input type
- // isn't annotated; we copy the mutation's category onto each input
- // argument's named type.
- // (b) Connection / Edge types inherit from their underlying type. These
- // are emitted by graphql-ruby's Relay pagination and never get a
- // hand-written docs_category. We walk `node`/`nodes`/`edges` to the
- // referenced object type and copy its category.
- //
- // Explicit annotations always win; derivation only fills gaps.
+ // Derive missing categories before fallback and other assignment so GHES fallback inherits them.
const lookupCat = (id: string): string | undefined =>
typeCategoryMap.get(id) ?? fallbackTypeMap[id]
const getMutationCat = (mutFieldName: string): string | undefined =>
mutationFieldCategoryMap.get(mutFieldName) ?? fallbackMutationMap[mutFieldName.toLowerCase()]
- // Walk through a TypeNode chain (NonNull/List wrappers) to the NamedType.
const namedTypeName = (typeNode: TypeNode): string | undefined => {
let t: TypeNode = typeNode
while ('type' in t) t = t.type
return t.kind === 'NamedType' ? t.name.value : undefined
}
- // (a) input objects from mutation field args
const mutationDef = schemaAST.definitions.find(
(def): def is ObjectTypeDefinitionNode =>
def.kind === 'ObjectTypeDefinition' && def.name.value === 'Mutation',
@@ -362,9 +337,7 @@ export default async function processSchemas(
}
}
- // (b) Connection / Edge types from their underlying type. Run multiple
- // passes so an XConnection that points at XEdge can still resolve after
- // XEdge itself has been derived (Connection -> Edge -> object).
+ // Multiple passes let Connection to Edge to object chains inherit the object category.
const objectDefs = schemaAST.definitions.filter(
(def): def is ObjectTypeDefinitionNode => def.kind === 'ObjectTypeDefinition',
)
@@ -378,7 +351,7 @@ export default async function processSchemas(
if (!isEdge && !isConn) continue
const id = helpers.getId(name)
if (lookupCat(id)) continue
- // Edge: walk `node`. Connection: prefer `nodes` (direct), else `edges`.
+ // Edge types use node; Connection types prefer nodes, then edges.
const fields = def.fields || []
let underlyingName: string | undefined
if (isEdge) {
@@ -402,39 +375,7 @@ export default async function processSchemas(
if (!changed) break
}
- // (c) General reference-based inheritance. An un-annotated enum, union, or
- // input object inherits the category of the type(s) that reference it, but
- // only when every referrer resolves to a single category; ambiguous types
- // (referrers disagree, or a referrer is itself ambiguous) stay in `other`.
- // This is the derived successor to a static exception list: it catches
- // generated/indirect types that github/github never annotates directly while
- // still letting the upstream team own the outcome via the parent type's
- // `docs_category`.
- //
- // Examples this resolves today:
- // - `IssueTimelineItemsItemType` / `PullRequestTimelineItemsItemType`:
- // runtime-generated enums used only as the `itemTypes` argument on
- // `Issue.timelineItems` (issues) / `PullRequest.timelineItems` (pulls).
- // - `RepositoryRuleType` (enum) and `RuleParameters` (union): referenced
- // from the annotated `RepositoryRule` object (repos).
- // - `RuleParametersInput` (input): referenced from the annotated
- // `RepositoryRuleInput` input object (repos).
- //
- // A "referrer category" is the category of:
- // - the owning object type, for a field's return type or a field argument's
- // type (interfaces are intentionally excluded: they are cross-cutting and
- // make coincidental single-category matches likely);
- // - the Mutation root field, for that field's arguments;
- // - the owning input object, for an input field's type. Input objects can
- // themselves be uncategorized-but-derivable, so this rule propagates
- // transitively through nested inputs.
- //
- // Implemented as a monotone fixpoint over candidate category *sets* rather
- // than committing categories as we go: a type is only assigned once its
- // candidate set has stopped growing, so the result is independent of
- // definition/derivation order and a later-discovered conflicting referrer
- // can never be missed. Explicit annotations and derivations (a)/(b) always
- // win: we only compute candidates for ids `lookupCat` still can't resolve.
+ // Reference-based inheritance assigns a category only when referrers resolve to one category.
const derivableTargets = schemaAST.definitions.filter(
(
def,
@@ -457,10 +398,7 @@ export default async function processSchemas(
const candidates = new Map>()
for (const id of targetIds) candidates.set(id, new Set())
- // Categories a type contributes when it appears as a referrer. Annotated /
- // fallback types contribute their single category; an uncommitted derivable
- // referrer (only ever an input object here) contributes its current
- // candidate set so ambiguity propagates downstream.
+ // Uncommitted input object referrers contribute candidate sets so ambiguity propagates.
const contribution = (referrerId: string): Iterable => {
const explicit = lookupCat(referrerId)
if (explicit) return [explicit]
@@ -481,8 +419,7 @@ export default async function processSchemas(
return grew
}
- // Bounded by the worst-case propagation depth; each pass only adds to sets,
- // so this terminates well before the cap.
+ // Each pass only adds candidates, so maxPasses bounds the propagation depth.
const maxPasses = targetIds.size + 2
for (let pass = 0; pass < maxPasses; pass++) {
let changed = false
@@ -492,8 +429,7 @@ export default async function processSchemas(
if (name === 'Query') continue
const isMutation = name === 'Mutation'
for (const field of def.fields || []) {
- // Mutation fields carry their own category and their payload return
- // type is already annotated, so (like rule (a)) we only walk args.
+ // Mutation categories apply to args; payload return types already carry categories.
const fieldCats: Iterable = isMutation
? ((c) => (c ? [c] : []))(getMutationCat(field.name.value))
: contribution(helpers.getId(name))
@@ -514,33 +450,18 @@ export default async function processSchemas(
if (!changed) break
}
- // Assign only the targets whose final candidate set is unambiguous.
for (const [id, cats] of candidates) {
if (cats.size === 1) typeCategoryMap.set(id, [...cats][0])
}
}
- // Populates the top-level `.category` field on every processed item. The
- // bucketer reads `.category` to split the schema into per-category files and
- // to rewrite cross-reference hrefs.
- //
- // Unknown categories (e.g. `:checks`, `:search`, `:packages`,
- // `:security_advisories`) normalize to `other`. The upstream gh/gh allowlist
- // permits many categories that docs-internal has not built per-category
- // landing pages for; without this fallback those types would be silently
- // dropped by `writeCategoryFiles` (which only emits files for slugs in
- // CATEGORIES) and their redirects would 404. Once a page exists for a
- // category, add it to CATEGORIES in src/graphql/lib/categories.ts and types
- // will move out of `other` on the next sync.
+ // Unknown categories normalize to other so writeCategoryFiles does not drop types or redirects.
const resolveCategory = (typeId: string): string => {
const cat = typeCategoryMap.get(typeId) ?? fallbackTypeMap[typeId] ?? OTHER_CATEGORY
return isValidCategory(cat) ? cat : OTHER_CATEGORY
}
- // process-schemas emits legacy `/graphql/reference/#` hrefs
- // throughout so the monolithic `schema.json` stays compatible with the
- // existing runtime loader. The bucketer rewrites these to the
- // category-aware form when emitting per-category schema files.
+ // linkTo emits reference hrefs; bucket-by-category rewrites only per-category schema files.
const linkTo = (urlKind: string, id: string): string => helpers.getFullLink(urlKind, id)
const data: ProcessedSchemaData = {
@@ -635,10 +556,7 @@ export default async function processSchemas(
mutation.name = field.name.value
mutation.id = helpers.getId(mutation.name)
- // Mutation fields carry @docsCategory at the field level on the
- // Mutation root, not on the payload type, so use the field map.
- // Normalize via isValidCategory so an upstream-only category
- // doesn't produce hrefs/buckets we don't ship pages for.
+ // Mutation fields carry @docsCategory on the field, not the payload type.
const rawMutationCategory =
mutationFieldCategoryMap.get(mutation.name) ??
fallbackMutationMap[mutation.name.toLowerCase()] ??
@@ -661,7 +579,7 @@ export default async function processSchemas(
previewsPerVersion,
)
- // there is only ever one input field argument, but loop anyway
+ // Mutation fields have one input argument in practice, but the schema exposes an array.
await Promise.all(
(field.arguments || []).map(async (arg: InputValueDefinitionNode) => {
const inputField: Partial = {}
@@ -679,8 +597,7 @@ export default async function processSchemas(
mutation.inputFields = sortBy(inputFields, 'name')
- // get return fields
- // first get the payload, then find payload object's fields. these are the mutation's return fields.
+ // Mutation return fields come from the payload object's fields.
const returnType = helpers.getType(field)
if (!returnType) return
const mutationReturnFields = objectsInSchema.find(
@@ -731,8 +648,7 @@ export default async function processSchemas(
}
if (def.kind === 'ObjectTypeDefinition') {
- // objects ending with 'Payload' are only used to derive mutation values
- // they are not included in the objects docs
+ // Payload objects provide mutation return fields and stay out of the object docs.
if (def.name.value.endsWith('Payload')) return
const object: Partial = {}
@@ -756,8 +672,7 @@ export default async function processSchemas(
previewsPerVersion,
)
- // an object's interfaces render in the `Implements` section
- // interfaces do not have directives so they cannot be under preview/deprecated
+ // Implements links carry only name, id, and href, without preview or deprecation data.
if (def.interfaces && def.interfaces.length) {
await Promise.all(
def.interfaces.map(async (graphqlInterface) => {
@@ -771,7 +686,7 @@ export default async function processSchemas(
)
}
- // an object's fields render in the `Fields` section
+ // Object fields render under Fields.
if (def.fields && def.fields.length) {
await Promise.all(
def.fields.map(async (field: FieldDefinitionNode) => {
@@ -835,7 +750,7 @@ export default async function processSchemas(
previewsPerVersion,
)
- // an interface's fields render in the "Fields" section
+ // Interface fields render under Fields.
if (def.fields && def.fields.length) {
await Promise.all(
def.fields.map(async (field: FieldDefinitionNode) => {
@@ -938,7 +853,7 @@ export default async function processSchemas(
previewsPerVersion,
)
- // union types do not have directives so cannot be under preview/deprecated
+ // Union member links carry no preview or deprecation state.
await Promise.all(
(def.types || []).map(async (type) => {
const possibleType: PossibleTypeInfo = {
@@ -956,10 +871,7 @@ export default async function processSchemas(
return
}
- // INPUT OBJECTS
- // NOTE: input objects ending with `Input` are NOT included in the v4 input objects sidebar
- // but they are still present in the docs (e.g., https://developer.github.com/v4/input_object/acceptenterpriseadministratorinvitationinput/)
- // so we will include them here
+ // Include Input-suffixed objects; docs pages exist outside the v4 sidebar.
if (def.kind === 'InputObjectTypeDefinition') {
const inputObject: Partial = {}
const inputFields: InputFieldDetailInfo[] = []
@@ -1048,7 +960,6 @@ export default async function processSchemas(
}),
)
- // add non-schema scalars and sort all scalars alphabetically
data.scalars = sortBy(data.scalars.concat(externalScalars), 'name')
data.queries = sortBy(data.queries, 'name')
diff --git a/src/graphql/scripts/utils/schema-helpers.ts b/src/graphql/scripts/utils/schema-helpers.ts
index f7914cd309c4..4cbf514a967b 100644
--- a/src/graphql/scripts/utils/schema-helpers.ts
+++ b/src/graphql/scripts/utils/schema-helpers.ts
@@ -54,13 +54,11 @@ const graphqlTypes: GraphQLTypeInfo[] = JSON.parse(
const singleQuotesInsteadOfBackticks = / '(\S+?)' /
-// Upstream schema descriptions link with a `${externalDocsUrl}` placeholder,
-// but nothing in this pipeline expands it. It ships percent-encoded as
-// `href="$%7BexternalDocsUrl%7D/code-security/..."`, which the browser
-// resolves against the current page and 404s. Dropping the placeholder leaves
-// a root-relative link, which `getDescription` then versions using the
-// `context` handed to `createSchemaHelpers`, so a GHES reader stays on GHES.
-// The bare `helpers` export has no context and leaves links unversioned.
+// Upstream schema descriptions include ${externalDocsUrl}, but this pipeline never expands it.
+// It ships as href="$%7BexternalDocsUrl%7D/code-security/..." and resolves against the
+// current page, causing 404s.
+// Dropping it leaves a root-relative link that getDescription versions with createSchemaHelpers.
+// The bare helpers export has no context and leaves links unversioned.
const unexpandedExternalDocsUrl = /\$\{externalDocsUrl\}(?=\/)/g
function addPeriod(string: string): string {
@@ -84,15 +82,12 @@ async function getArguments(
arg.defaultValue && 'value' in arg.defaultValue ? arg.defaultValue.value : undefined
newArg.description = arg.description ? await getDescription(arg.description.value, context) : ''
const typeName = getType(arg)
- if (!typeName) continue // Skip if type cannot be determined
+ if (!typeName) continue
type.name = typeName
type.id = getId(typeName)
const typeKind = getTypeKind(typeName, schema)
- if (!typeKind) continue // Skip if type kind cannot be determined
- // process-schemas always emits legacy `/graphql/reference/#`
- // hrefs. bucket-by-category rewrites them into the category-aware form
- // when splitting into per-category files, so monolithic schema.json stays
- // byte-stable with what the existing runtime expects.
+ if (!typeKind) continue
+ // getFullLink keeps reference hrefs stable; bucket-by-category rewrites only category files.
type.href = getFullLink(typeKind, type.id!)
newArg.type = type as TypeInfo
newArgs.push(newArg as ArgumentInfo)
@@ -101,9 +96,7 @@ async function getArguments(
return newArgs
}
-// Build a category-aware anchor link for a type, e.g.
-// `/graphql/reference/repos#object-repository`. Exposed for the bucketer's
-// href-rewrite pass; process-schemas itself uses the legacy `getFullLink`.
+// buildCategoryHref returns anchors like /graphql/reference/repos#object-repository for rewrites.
export function buildCategoryHref(category: string, urlKind: string, id: string): string {
return `/graphql/reference/${category}#${slugPrefixForUrlKind(urlKind)}-${id}`
}
@@ -115,10 +108,10 @@ async function getDeprecationReason(
): Promise {
if (!schemaMember.isDeprecated) return
- // it's possible for a schema member to be deprecated and under preview
+ // Deprecated and preview can both apply to one schema member.
const deprecationDirective = directives.filter((dir) => dir.name.value === 'deprecated')
- // catch any schema members that have more than one deprecation (none currently)
+ // Multiple deprecation directives indicate upstream schema data needs review.
if (deprecationDirective.length > 1)
console.log(`more than one deprecation found for ${schemaMember.name}`)
@@ -146,8 +139,6 @@ function getFullLink(baseType: string, id: string): string {
return `/graphql/reference/${baseType}#${id}`
}
-// Extract the `@docsCategory(name: "...")` value from a directive list.
-// Returns undefined when the directive is absent.
function getDocsCategory(directives: readonly ConstDirectiveNode[]): string | undefined {
const directive = directives.find((dir) => dir.name.value === 'docsCategory')
if (!directive) return
@@ -162,7 +153,7 @@ function getId(typeName: string): string {
return removeMarkers(typeName).toLowerCase()
}
-// e.g., given `ObjectTypeDefinition`, get `objects`
+// Example: ObjectTypeDefinition maps to objects.
function getKind(type: string): string {
return graphqlTypes.find((graphqlType) => graphqlType.type === type)!.kind
}
@@ -174,15 +165,15 @@ async function getPreview(
): Promise {
if (!directives.length) return
- // it's possible for a schema member to be deprecated and under preview
+ // Deprecated and preview can both apply to one schema member.
const previewDirective = directives.filter((dir) => dir.name.value === 'preview')
if (!previewDirective.length) return
- // catch any schema members that are under more than one preview (none currently)
+ // Log multiple preview directives from the schema AST; the script expects at most one.
if (previewDirective.length > 1)
console.log(`more than one preview found for ${schemaMember.name}`)
- // an input object's input field may have a ListValue directive that is not relevant to previews
+ // Ignore ListValue preview directives on input fields because previews use string values.
const firstArg = previewDirective[0]?.arguments?.[0]
if (!firstArg) return
const argValue = firstArg.value
@@ -196,48 +187,40 @@ async function getPreview(
return preview
}
-// the docs use brackets to denote list types: `[foo]`
-// and an exclamation mark to denote non-nullable types: `foo!`
-// both single items and lists can be non-nullable
-// so the permutations are:
-// 1. single items: `foo`, `foo!`
-// 2. nullable lists: `[foo]`, `[foo!]`
-// 3. non-null lists: `[foo]!`, `[foo!]!`
-// see https://github.com/rmosolgo/graphql-ruby/blob/master/guides/type_definitions/lists.md#lists-nullable-lists-and-lists-of-nulls
+// GraphQL list and non-null wrappers combine as foo, foo!, [foo], [foo!], [foo]!,
+// and [foo!]!.
+// See https://github.com/rmosolgo/graphql-ruby/blob/master/guides/type_definitions/lists.md#lists-nullable-lists-and-lists-of-nulls
function getType(field: FieldNode): string | undefined {
- // 1. single items
if (field.type.kind !== 'ListType') {
- // nullable item, e.g. `license` query has `License` type
+ // Nullable item example: license query has License type.
if (field.type.kind === 'NamedType') {
return field.type.name.value
}
- // non-null item, e.g. `meta` query has `GitHubMetadata!` type
+ // Non-null item example: meta query has GitHubMetadata! type.
if (field.type.kind === 'NonNullType' && field.type.type.kind === 'NamedType') {
return `${field.type.type.name.value}!`
}
}
- // 2. nullable lists
if (field.type.kind === 'ListType') {
- // nullable items, e.g. `codesOfConduct` query has `[CodeOfConduct]` type
+ // Nullable list example: codesOfConduct query has [CodeOfConduct] type.
if (field.type.type.kind === 'NamedType') {
return `[${field.type.type.name.value}]`
}
- // non-null items, e.g. `severities` arg has `[SecurityAdvisorySeverity!]` type
+ // Nullable list example: severities arg has [SecurityAdvisorySeverity!] type.
if (field.type.type.kind === 'NonNullType' && field.type.type.type.kind === 'NamedType') {
return `[${field.type.type.type.name.value}!]`
}
}
- // 3. non-null lists
if (field.type.kind === 'NonNullType' && field.type.type.kind === 'ListType') {
- // nullable items, e.g. `licenses` query has `[License]!` type
+ // Non-null list example: licenses query has [License]! type.
if (field.type.type.type.kind === 'NamedType') {
return `[${field.type.type.type.name.value}]!`
}
- // non-null items, e.g. `marketplaceCategories` query has `[MarketplaceCategory!]!` type
+ // Non-null list example: marketplaceCategories query has [MarketplaceCategory!]! type.
if (
field.type.type.type.kind === 'NonNullType' &&
field.type.type.type.type.kind === 'NamedType'
@@ -296,12 +279,9 @@ const helpers = {
getTypeKind,
}
-// The three helpers that render Markdown need to know which docs version they
-// are rendering for, otherwise `rewrite-local-links` bails out and root-relative
-// links ship without a language or version segment. Binding the context once
-// here keeps the ~30 call sites in `process-schemas` unchanged, and keeps the
-// context per-call rather than in module state, so two versions can never
-// render against each other's context.
+// Markdown helpers need docs version context, or rewrite-local-links omits language and version.
+// Binding context once keeps the ~30 process-schemas call sites unchanged and per-call.
+// Per-call context prevents concurrent versions from rendering against each other.
export function createSchemaHelpers(context: Context): typeof helpers {
return {
...helpers,
diff --git a/src/graphql/scripts/utils/sync-category-content.ts b/src/graphql/scripts/utils/sync-category-content.ts
index 02a58d150c5d..5e71068971e5 100644
--- a/src/graphql/scripts/utils/sync-category-content.ts
+++ b/src/graphql/scripts/utils/sync-category-content.ts
@@ -10,36 +10,27 @@ import {
} from '@/automated-pipelines/lib/update-markdown'
import { CATEGORIES, OTHER_CATEGORY, categoryTitle } from '@/graphql/lib/categories'
-// Default directory holding the per-category GraphQL reference content pages.
-// Overridable via options for tests; production always uses this path.
+// Tests can override the content directory; production uses content/graphql/reference.
const DEFAULT_CONTENT_DIR = path.join('content', 'graphql', 'reference')
-// Value of the `autogenerated` frontmatter on managed category pages. The
-// content-directory helper uses this to know which files it owns (and may
-// therefore delete when a category empties).
+// updateContentDirectory deletes only pages whose autogenerated frontmatter matches graphql.
const AUTOGENERATED_TYPE = 'graphql'
// Breadcrumb category the reference pages sit under in the sidebar.
const CATEGORY_BREADCRUMB = 'Explore the schema reference'
-// Maps a category slug to the set of docs version keys (e.g.
-// `free-pro-team@latest`, `enterprise-server@3.22`) in which the category has
-// at least one type. Built by sync.ts from the per-version buckets.
+// Maps each category slug to docs version keys where at least one type exists; sync.ts builds it.
export type CategoryPresence = Map>
const categoryUrlPath = (cat: string) => `/graphql/reference/${cat}`
-// Matches a bare category reference URL (no fragment), e.g.
-// `/graphql/reference/code-scanning`. Kind pages like
-// `/graphql/reference/queries` also match this shape but are filtered out
-// because their slug is not in CATEGORIES.
+// CATEGORY_URL_RE matches bare category URLs like /graphql/reference/code-scanning.
+// Kind pages like /graphql/reference/queries match this shape but get filtered later.
const CATEGORY_URL_RE = /^\/graphql\/reference\/([a-z][a-z0-9-]*)$/
function isPresentInAnyVersion(presence: CategoryPresence, cat: string): boolean {
return (presence.get(cat)?.size ?? 0) > 0
}
-// Read the `redirect_from` of every managed category page before the content
-// helper potentially deletes those files, so redirect chains aren't lost when a
-// category disappears. Returns a map of category slug -> redirect_from entries.
+// Capture managed category redirects before deletion so disappearance redirects keep resolving.
async function captureCategoryRedirects(contentDir: string): Promise> {
const captured = new Map()
let files: string[] = []
@@ -72,10 +63,7 @@ function normalizeRedirects(value: unknown): string[] {
return []
}
-// Build the `sourceContent` map the content-directory helper expects:
-// `{ : { data: , content: } }`. Only categories
-// that are non-empty in at least one version get a page; emptied categories are
-// omitted so the helper deletes their stale files.
+// buildSourceContent omits empty categories so updateContentDirectory deletes stale pages.
async function buildSourceContent(presence: CategoryPresence, contentDir: string) {
const sourceContent: Record; content: string }> = {}
for (const cat of CATEGORIES) {
@@ -84,9 +72,7 @@ async function buildSourceContent(presence: CategoryPresence, contentDir: string
const versions = await convertVersionsToFrontmatter([...versionsSet])
const title = categoryTitle(cat)
const file = path.join(contentDir, `${cat}.md`)
- // For pages that already exist, the helper only refreshes `versions` and the
- // autogenerated body, preserving any writer edits to title/intro/category.
- // These values therefore only seed brand-new category pages.
+ // Managed pages keep writer-edited title, intro, and category; values apply only at creation.
sourceContent[file] = {
data: {
title,
@@ -102,10 +88,8 @@ async function buildSourceContent(presence: CategoryPresence, contentDir: string
return sourceContent
}
-// Reconcile the reference index `redirect_from` so that a bare category URL
-// redirects to the reference root when (and only when) that category is empty in
-// every version. Categories present in at least one version must NOT have a
-// redirect, otherwise a still-valid versioned page would be shadowed.
+// Empty categories redirect to the reference root only when every version lacks that category.
+// Present categories must not redirect, or they shadow still-valid versioned pages.
async function reconcileIndexRedirects(
presence: CategoryPresence,
capturedRedirects: Map,
@@ -120,9 +104,7 @@ async function reconcileIndexRedirects(
const { data, content } = matter(raw)
const existing = normalizeRedirects(data.redirect_from)
- // Drop redirects for managed categories that are now present (e.g. a category
- // that previously emptied and has since come back). Leave kind-page redirects
- // (queries, mutations, ...) and non-category redirects (/v4/reference) intact.
+ // Drop redirects for categories that returned; keep kind-page and non-category redirects intact.
const next = existing.filter((entry) => {
const match = CATEGORY_URL_RE.exec(entry)
if (!match) return true
@@ -131,16 +113,13 @@ async function reconcileIndexRedirects(
return !isPresentInAnyVersion(presence, cat)
})
- // Add a root redirect for every managed category that is empty in all
- // versions. `other` is always present (un-annotated types), so it never
- // disappears, but guard against it defensively.
+ // Add root redirects for categories empty in all versions; other never disappears.
for (const cat of CATEGORIES) {
if (cat === OTHER_CATEGORY) continue
if (isPresentInAnyVersion(presence, cat)) continue
const url = categoryUrlPath(cat)
if (!next.includes(url)) next.push(url)
- // Preserve any redirect_from the deleted category page carried so existing
- // inbound redirect chains keep resolving.
+ // Carry redirect_from from deleted category pages so inbound chains keep resolving.
for (const inherited of capturedRedirects.get(cat) ?? []) {
if (!next.includes(inherited)) next.push(inherited)
}
@@ -151,10 +130,8 @@ async function reconcileIndexRedirects(
await fs.writeFile(indexFile, matter.stringify(content, data))
}
-// Entry point used by sync.ts after it has bucketed every version. Creates,
-// updates, and deletes the per-category content pages, refreshes the reference
-// index children, and reconciles disappearance redirects. `contentDir` is
-// overridable for tests; production uses the default reference directory.
+// Creates, updates, and deletes category pages after sync.ts buckets every version.
+// Also refreshes index children and disappearance redirects; tests can override contentDir.
export async function syncCategoryContentFiles(
presence: CategoryPresence,
options: { contentDir?: string } = {},
diff --git a/src/graphql/tests/build-changelog.ts b/src/graphql/tests/build-changelog.ts
index 420d659ec06d..ed140730d217 100644
--- a/src/graphql/tests/build-changelog.ts
+++ b/src/graphql/tests/build-changelog.ts
@@ -59,8 +59,6 @@ describe('creating a changelog from old schema and new schema', () => {
})
test('ignores unknown change types without throwing errors', async () => {
- // Create a minimal test that would generate an unknown change type
- // This test ensures the system gracefully handles new change types
const oldSchemaString = `
type Query {
field: String
@@ -76,8 +74,7 @@ describe('creating a changelog from old schema and new schema', () => {
}
`
- // This should generate TypeDescriptionAdded change type
- // which should be silently ignored if not in CHANGES_TO_REPORT
+ // CHANGES_TO_REPORT omits TypeDescriptionAdded, so createChangelogEntry ignores it.
const entry: ChangelogEntry | null = await createChangelogEntry(
oldSchemaString,
newSchemaString,
@@ -86,14 +83,10 @@ describe('creating a changelog from old schema and new schema', () => {
[],
)
- // Should return null since TypeDescriptionAdded is not in CHANGES_TO_REPORT
- // and will be silently ignored without throwing an error
expect(entry).toBeNull()
})
test('handles new directive usage change types gracefully', async () => {
- // Test that verifies the system can handle new directive-related change types
- // that were previously causing errors in the pipeline
const oldSchemaString = `
directive @example on FIELD_DEFINITION
@@ -110,8 +103,7 @@ describe('creating a changelog from old schema and new schema', () => {
}
`
- // This should generate DirectiveUsage* change types that are not in CHANGES_TO_REPORT
- // The system should silently ignore these and not throw errors
+ // CHANGES_TO_REPORT omits added field directives, so createChangelogEntry ignores this one.
const entry: ChangelogEntry | null = await createChangelogEntry(
oldSchemaString,
newSchemaString,
@@ -120,7 +112,6 @@ describe('creating a changelog from old schema and new schema', () => {
[],
)
- // Should return null since directive usage changes are typically ignored
expect(entry).toBeNull()
})
@@ -219,12 +210,10 @@ upcoming_changes:
describe('Preparing preview links', () => {
test('fixes preview names', () => {
- // These two are special cases
+ // UpdateRefsPreview and MergeInfoPreview are hand-written title exceptions.
expect(cleanPreviewTitle('UpdateRefsPreview')).toEqual('Update refs preview')
expect(cleanPreviewTitle('MergeInfoPreview')).toEqual('Merge info preview')
- // Previews that don't end in " preview" have it added
expect(cleanPreviewTitle('something interesting')).toEqual('something interesting preview')
- // Other things are left as-is
expect(cleanPreviewTitle('nice preview')).toEqual('nice preview')
})
@@ -253,7 +242,6 @@ describe('updating the changelog file', () => {
prependDatedEntry(exampleEntry, testTargetPath)
const newContents: string = await fs.readFile(testTargetPath, 'utf8')
- // reset the file:
await fs.writeFile(testTargetPath, previousContents.toString())
expect(exampleEntry).toEqual({
@@ -299,10 +287,8 @@ describe('ensureYearPage', () => {
ensureYearPage('2026', tmpDir)
- // Should not modify the existing file
const yearPage = await fs.readFile(`${tmpDir}/2026.md`, 'utf8')
expect(yearPage).toContain('title: existing')
- // index.md should be unchanged
const updatedIndex = await fs.readFile(`${tmpDir}/index.md`, 'utf8')
expect(updatedIndex).toBe(indexContent)
})
@@ -325,7 +311,7 @@ describe('ignored changes tracking', () => {
}
`
- // This should generate a TypeDescriptionAdded change type that gets ignored
+ // Ignored-change tracking records TypeDescriptionAdded.
await createChangelogEntry(oldSchemaString, newSchemaString, [], [], [])
const ignoredChanges: IgnoredChange[] = getLastIgnoredChanges() as unknown as IgnoredChange[]
@@ -350,7 +336,7 @@ describe('ignored changes tracking', () => {
}
`
- // This should generate multiple DirectiveUsage changes that get ignored
+ // Ignored-change summary groups multiple DirectiveUsage changes under one type.
await createChangelogEntry(oldSchemaString, newSchemaString, [], [], [])
const summary = getIgnoredChangesSummary()
@@ -369,7 +355,6 @@ describe('ignored changes tracking', () => {
}
`
- // No changes should be generated
await createChangelogEntry(schemaString, schemaString, [], [], [])
const summary = getIgnoredChangesSummary()
diff --git a/src/graphql/tests/derive-categories.ts b/src/graphql/tests/derive-categories.ts
index fa3138ef1baa..4a0b675d88b8 100644
--- a/src/graphql/tests/derive-categories.ts
+++ b/src/graphql/tests/derive-categories.ts
@@ -2,14 +2,12 @@ import { describe, expect, test } from 'vitest'
import processSchemas from '../scripts/utils/process-schemas'
-// Minimal `@docsCategory` directive declaration so `buildASTSchema` can parse
-// the fixtures below. Mirrors the real declaration emitted by github/github.
+// Minimal @docsCategory lets buildASTSchema parse fixtures and mirrors github/github output.
const DIRECTIVE = `
directive @docsCategory(name: String!) on ENUM | FIELD_DEFINITION | INPUT_OBJECT | INTERFACE | OBJECT | UNION
`
-// Run processSchemas over an inline IDL and return a flat name -> category map
-// across every kind, so tests can assert where a type landed.
+// A flat name to category map lets each fixture assert where every kind landed.
async function categoriesFor(idl: string): Promise> {
const data = await processSchemas(`${DIRECTIVE}\n${idl}`, [])
const out: Record = {}
@@ -21,7 +19,7 @@ async function categoriesFor(idl: string): Promise> {
return out
}
-// Every fixture needs a Query root; a `viewer` field keeps it non-empty.
+// GraphQL fixtures need a non-empty Query root.
const QUERY = `
type Query {
viewer: String
@@ -53,7 +51,7 @@ describe('reference-based category derivation (PASS 1.5 rule c)', () => {
})
test('enum inherits from an annotated object field argument', async () => {
- // Mirrors Issue.timelineItems(itemTypes: [IssueTimelineItemsItemType!]).
+ // Mirrors Issue.timelineItems with itemTypes [IssueTimelineItemsItemType!].
const cats = await categoriesFor(`
${QUERY}
type Issue @docsCategory(name: "issues") {
@@ -74,7 +72,7 @@ describe('reference-based category derivation (PASS 1.5 rule c)', () => {
input NestedParametersInput { value: String }
`)
expect(cats.RuleParametersInput).toBe('repos')
- // Propagates another hop through the still-uncategorized input chain.
+ // NestedParametersInput proves inheritance propagates through an uncategorized input chain.
expect(cats.NestedParametersInput).toBe('repos')
})
@@ -117,8 +115,7 @@ describe('reference-based category derivation (PASS 1.5 rule c)', () => {
})
test('interfaces are not a referrer source', async () => {
- // Interface is annotated and references the enum, but no object does, so
- // the enum must not inherit the interface's category.
+ // Interfaces are not referrer sources, so their referenced enums stay in other.
const cats = await categoriesFor(`
${QUERY}
interface Rulable @docsCategory(name: "repos") {
diff --git a/src/graphql/tests/description-links.ts b/src/graphql/tests/description-links.ts
index 19a20e1ea6b2..1f07d84e5aca 100644
--- a/src/graphql/tests/description-links.ts
+++ b/src/graphql/tests/description-links.ts
@@ -13,8 +13,7 @@ describe('GraphQL description links', () => {
test('strips the unexpanded externalDocsUrl placeholder', async () => {
const rendered = await helpers.getDescription(PLACEHOLDER_LINK)
- // Left in place the placeholder percent-encodes into the href and the
- // browser resolves it against the current page, which 404s.
+ // An unexpanded placeholder becomes a page-relative href and 404s.
expect(rendered).not.toContain('externalDocsUrl')
expect(rendered).toContain('href="/code-security/code-scanning#levels"')
})
diff --git a/src/graphql/tests/server-rendering.ts b/src/graphql/tests/server-rendering.ts
index 177bb94f37d3..deace3a4f0fe 100644
--- a/src/graphql/tests/server-rendering.ts
+++ b/src/graphql/tests/server-rendering.ts
@@ -23,12 +23,9 @@ describe('server rendering certain GraphQL pages', () => {
expect.assertions(hrefs.length + 1)
})
+ // Request the changelog twice because github-slugger state can add suffixes on the second render.
+ // The mini-TOC hrefs must match those heading IDs.
test('minitoc hrefs on changelog match and verify slugger behavior', async () => {
- // Testing the minitoc links match the heading ids but also validating
- // slugger behavior see docs-engineering/issues#5792.
- // Little funky because we need to make 2 requests to the page to test
- // the problem behavior where slugger state accumulates across
- // requests, it won't fail the first time around.
await getDOM('/graphql/overview/changelog')
const $ = await getDOM('/graphql/overview/changelog')
const links = $('[data-testid="minitoc"] a[href]')
diff --git a/src/graphql/tests/sync-category-content.ts b/src/graphql/tests/sync-category-content.ts
index ef3690219664..6de96d3afc5b 100644
--- a/src/graphql/tests/sync-category-content.ts
+++ b/src/graphql/tests/sync-category-content.ts
@@ -17,8 +17,7 @@ const GHEC = 'enterprise-cloud@latest'
const REFERENCE_DIR = path.join('content', 'graphql', 'reference')
-// The full set of categories present in a steady-state fixture. Returned as a
-// fresh Map each call so tests never share mutable state.
+// A fresh steady-state presence map prevents tests from sharing mutable state.
function steadyPresence(): CategoryPresence {
return new Map([
['actions', new Set([FPT, GHEC])],
@@ -28,7 +27,6 @@ function steadyPresence(): CategoryPresence {
])
}
-// Write an autogenerated category page with the given versions frontmatter.
async function writeCategoryFile(
root: string,
cat: string,
@@ -104,28 +102,23 @@ describe('syncCategoryContentFiles', () => {
await writeCategoryFile(root, 'other', { fpt: '*', ghec: '*' })
const presence = steadyPresence()
- presence.delete('code-scanning') // emptied -> absent in all versions
+ presence.delete('code-scanning') // A missing category means it has no types in any version.
await syncCategoryContentFiles(presence, { contentDir })
- // Emptied category file is deleted; populated ones remain.
expect(existsSync(path.join(root, REFERENCE_DIR, 'code-scanning.md'))).toBe(false)
expect(existsSync(path.join(root, REFERENCE_DIR, 'actions.md'))).toBe(true)
expect(existsSync(path.join(root, REFERENCE_DIR, 'other.md'))).toBe(true)
- // Versions are narrowed to where the category actually has types.
const sponsors = matter(await readFile(path.join(root, REFERENCE_DIR, 'sponsors.md'), 'utf8'))
expect(sponsors.data.versions).toEqual({ fpt: '*' })
const index = await readIndex(root)
- // Sidebar children drop the emptied category.
expect(index.data.children).not.toContain('/code-scanning')
expect(index.data.children).toContain('/actions')
expect(index.data.children).toContain('/sponsors')
expect(index.data.children).toContain('/other')
- // Disappeared category gets a root redirect; unrelated redirects are kept;
- // present categories are never redirected.
expect(index.data.redirect_from).toContain('/graphql/reference/code-scanning')
expect(index.data.redirect_from).toContain('/v4/reference')
expect(index.data.redirect_from).toContain('/graphql/reference/queries')
@@ -133,8 +126,7 @@ describe('syncCategoryContentFiles', () => {
})
test('recreates a reappearing category and removes its stale redirect', async () => {
- // Seed the post-deletion state: code-scanning has no page and carries a
- // disappearance redirect on the index.
+ // Seed the post-deletion state: code-scanning has no page but redirects to the index.
await writeIndex(
root,
['/actions', '/sponsors', '/other'],
@@ -150,7 +142,6 @@ describe('syncCategoryContentFiles', () => {
const index = await readIndex(root)
expect(index.data.children).toContain('/code-scanning')
expect(index.data.redirect_from).not.toContain('/graphql/reference/code-scanning')
- // Unrelated redirects survive the reconciliation.
expect(index.data.redirect_from).toContain('/v4/reference')
})
@@ -161,10 +152,8 @@ describe('syncCategoryContentFiles', () => {
await writeCategoryFile(root, 'code-scanning', { fpt: '*', ghec: '*' })
await writeCategoryFile(root, 'other', { fpt: '*', ghec: '*' })
- // First run establishes the canonical steady state for this presence.
await syncCategoryContentFiles(steadyPresence(), { contentDir })
const before = await snapshotReferenceDir(root)
- // Second run with identical input must be a no-op.
await syncCategoryContentFiles(steadyPresence(), { contentDir })
const after = await snapshotReferenceDir(root)
diff --git a/src/graphql/tests/validate-schema.ts b/src/graphql/tests/validate-schema.ts
index 55ce99991162..19ee80a23e54 100644
--- a/src/graphql/tests/validate-schema.ts
+++ b/src/graphql/tests/validate-schema.ts
@@ -20,13 +20,11 @@ const upcomingChangesValidate = getJsonValidator(upcomingChangesValidator)
describe('graphql json files', () => {
vi.setConfig({ testTimeout: 3 * 60 * 1000 })
- // The typeObj is repeated thousands of times across the per-category files
- // so cache validated objects to speed this test up significantly.
+ // typeObj repeats thousands of times across category files.
+ // Cache validated objects to keep this test fast.
const typeObjsTested = new Set()
for (const version of graphqlVersions) {
- // Merge every per-category schema-*.json into one in-memory shape
- // mirroring the legacy monolithic schema.json so the rest of the test
- // logic stays unchanged.
+ // Merge category schema files into the monolithic shape that schemaValidator expects.
const schemaJsonPerVersion: Record> = {}
for (const type of graphqlTypes) schemaJsonPerVersion[type] = []
for (const category of CATEGORIES) {
@@ -83,7 +81,6 @@ describe('graphql json files', () => {
`${GRAPHQL_DATA_DIR}/${version}/upcoming-changes.json`,
) as Record
for (const changes of Object.values(upcomingChanges)) {
- // each object value is an array of changes
for (const changeObj of changes) {
const isValid = upcomingChangesValidate(changeObj)
let errors: string | undefined
diff --git a/src/landings/components/ArticleList.module.scss b/src/landings/components/ArticleList.module.scss
index 4bd4497feaea..11f743e8ba81 100644
--- a/src/landings/components/ArticleList.module.scss
+++ b/src/landings/components/ArticleList.module.scss
@@ -12,7 +12,6 @@
}
}
-// Muted secondary copy (article intro, published date) under each link title.
.textMuted {
color: var(--brand-color-text-muted);
}
diff --git a/src/landings/components/CategoryLanding.tsx b/src/landings/components/CategoryLanding.tsx
index 995261e8edd7..7b4909bee7cc 100644
--- a/src/landings/components/CategoryLanding.tsx
+++ b/src/landings/components/CategoryLanding.tsx
@@ -17,12 +17,11 @@ export const CategoryLanding = () => {
const router = useRouter()
const { title, intro, tocItems, spotlight, filters } = useCategoryLandingContext()
- // The category filter is always shown. Surface and complexity are opt-in via
- // the `filters` frontmatter array on the landing page.
+ // Always show the category filter; filters frontmatter controls surface and complexity.
const showSurface = filters ? filters.includes('surface') : true
const showComplexity = filters ? filters.includes('complexity') : false
- // tocItems contains directories and its children, we only want the child articles
+ // Category landing cards use only child articles, not directory nodes.
const onlyFlatItems: ArticleCardItems = tocItems.flatMap((item) => item.childTocItems || [])
const [searchQuery, setSearchQuery] = useState('')
@@ -112,8 +111,7 @@ export const CategoryLanding = () => {
`: `Link` paints its own `color`, so it does not inherit.
+// The accent color belongs on the Primer React Link because h3 cannot override Link color.
.linkAccent {
color: var(--brand-color-text-link-rest);
}
-// The leading octicon's tinted circle. Replaces primer/css's
-// `bgColor-accent-muted`, which resolves to Primer's blue wash (#388bfd1a in
-// dark) behind a brand-blue icon. Brand ships no accent-muted surface token, so
-// tint brand's own link blue — the same token the icon itself is painted with.
+// bgColor-accent-muted resolves to Primer's blue wash, #388bfd1a in dark mode,
+// behind a brand-blue icon. Tint with brand link blue to match the icon.
.iconBackdrop {
background-color: color-mix(
in srgb,
@@ -45,7 +41,6 @@
);
}
-// The card's description copy.
.textMuted {
color: var(--brand-color-text-muted);
}
diff --git a/src/landings/components/HomePageHero.module.scss b/src/landings/components/HomePageHero.module.scss
index 7c558975a89d..3e57f1577931 100644
--- a/src/landings/components/HomePageHero.module.scss
+++ b/src/landings/components/HomePageHero.module.scss
@@ -1,5 +1,5 @@
-// Docs 2026 homepage hero: a left-aligned title block stacked above a muted
-// search band, both inside bordered rails. No background image — the mountains
+// The homepage hero stacks a left-aligned title block above a muted search band,
+// both inside bordered rails. No background image; the mountains
// graphic is a separate, out-of-scope band.
.hero {
border-bottom: var(--brand-borderWidth-thin, 1px) solid
@@ -26,9 +26,8 @@
margin: 1rem 0 0;
}
-// Search entry that mimics an input but opens the shared SearchOverlay on
-// activation. A full-width muted band with the placeholder on the left and a
-// trailing "/" key hint, separated from the title by a top border.
+// The search entry mimics an input but opens the shared SearchOverlay on activation.
+// The slash key hint sits in a muted band separated from the title by a top border.
.searchRow {
display: flex;
align-items: center;
diff --git a/src/landings/components/ProductSelectionCard.module.scss b/src/landings/components/ProductSelectionCard.module.scss
index 5a621578ae8a..422d1f6f5126 100644
--- a/src/landings/components/ProductSelectionCard.module.scss
+++ b/src/landings/components/ProductSelectionCard.module.scss
@@ -1,10 +1,7 @@
// A single "All Docs" grid cell: category heading at the top, product links
-// top-aligned directly beneath it. Cells in a row still stretch to the tallest
-// cell's height via the grid. Internal dividers are the cell's left
-// border (skipped on the first column of each row, per breakpoint, so they don't
-// double the container rail) plus a bottom border for row dividers. The column
-// count steps 1 -> 2 -> 3 -> 4, so each breakpoint re-applies the left border to
-// every cell and then clears it on the new first-of-row.
+// top-aligned directly beneath it. Internal dividers come from the cell's left
+// border, skipped on the first column of each row, plus bottom row dividers.
+// Each breakpoint re-applies the left border and clears its first column.
.cell {
display: flex;
flex-direction: column;
@@ -14,11 +11,9 @@
border-bottom: var(--brand-borderWidth-thin, 1px) solid
var(--brand-color-border-muted);
- // Mobile: single column — no internal vertical dividers.
border-left: 0;
@media (min-width: 34rem) {
- // 2 columns.
border-left: var(--brand-borderWidth-thin, 1px) solid
var(--brand-color-border-muted);
@@ -28,7 +23,6 @@
}
@media (min-width: 63.25rem) {
- // 3 columns.
&:nth-child(n) {
border-left: var(--brand-borderWidth-thin, 1px) solid
var(--brand-color-border-muted);
@@ -40,7 +34,6 @@
}
@media (min-width: 80rem) {
- // 4 columns.
&:nth-child(n) {
border-left: var(--brand-borderWidth-thin, 1px) solid
var(--brand-color-border-muted);
diff --git a/src/landings/components/ProductSelectionCard.tsx b/src/landings/components/ProductSelectionCard.tsx
index b052b97e68e4..9e0edeb2b35b 100644
--- a/src/landings/components/ProductSelectionCard.tsx
+++ b/src/landings/components/ProductSelectionCard.tsx
@@ -10,7 +10,7 @@ type ProductSelectionCardProps = {
}
export const ProductSelectionCard = ({ group }: ProductSelectionCardProps) => {
- // Don't display the group if it has no children due to versioning
+ // Versioning can remove every child, so hide empty groups.
if (!group.children || group.children.length === 0) {
return null
}
diff --git a/src/landings/components/ProductSelections.module.scss b/src/landings/components/ProductSelections.module.scss
index 4ec64165ad96..6a30d4a823d4 100644
--- a/src/landings/components/ProductSelections.module.scss
+++ b/src/landings/components/ProductSelections.module.scss
@@ -1,8 +1,6 @@
-// "All Docs" section — bordered gridline layout mirroring the Docs 2026 design.
+// The All Docs section uses a bordered gridline layout.
// The container supplies the outer left/right rails; internal dividers are drawn
-// by each cell (left border, skipping the first column) and each row (bottom
-// border). There is deliberately no divider directly under the "All Docs"
-// heading.
+// by each cell and row. There is no divider directly under the All Docs heading.
.section {
max-width: 82rem;
margin: 0 auto;
diff --git a/src/landings/components/SidebarProduct.module.scss b/src/landings/components/SidebarProduct.module.scss
index b4d073687e20..842f5e4184c9 100644
--- a/src/landings/components/SidebarProduct.module.scss
+++ b/src/landings/components/SidebarProduct.module.scss
@@ -1,15 +1,10 @@
-// The tree's typography now comes from @primer/react-brand NavList's own scale
-// (level-1 section headers at size-200, leaves at size-100). This class is kept
-// as the wrapper hook for `data-testid="sidebar"`.
+// Brand NavList owns the tree typography: level-1 section headers use size-200,
+// and leaves use size-100. This class stays as the data-testid sidebar hook.
.sidebar {
- // Brand's NavList draws the active accent bar only on nested leaves — its rule
- // explicitly excludes level-1 (`.item--leaf:not(.item--level-1) .link[aria-current]`),
- // so a top-level article like "Quickstart" gets only the subtle background pill
- // and no green bar. Restore the bar for active top-level leaves to match nested
- // items. Brand's classes are hashed, so match on the stable substrings.
- //
- // This also covers `[data-pending]` — the optimistic marker on a just-clicked item
- // (see the pending rules below) — so a clicked top-level leaf gets the bar too.
+ // Brand NavList excludes level-1 leaves from its active accent bar rule, so
+ // top-level articles such as Quickstart get only the subtle background pill.
+ // Match stable hashed-class substrings and restore the bar for aria-current.
+ // Cover data-pending too, so clicked top-level leaves get the same visual bar.
:global([class*="NavList__item--leaf"][class*="NavList__item--level-1"])
:global([class*="NavList__link"])[aria-current]:not(
[aria-current="false"]
@@ -18,34 +13,26 @@
:global([class*="NavList__link"])[data-pending]::before {
content: "";
position: absolute;
- // Level-1 links are shorter (~24px) than nested leaves, so use a small inset
- // to keep the bar roughly the height of the background pill rather than the
- // larger inset brand applies to taller nested items.
+ // Level-1 links are about 24px tall, so a small inset matches the background pill height.
inset-block: var(--base-size-2);
// Level-1 leaves have no inline padding, so pull the bar into the NavList's
- // own inline padding gutter to sit left of the label — matching where brand
- // places the level-1 indicator on expandable section toggles.
+ // inline padding gutter where Brand places level-1 expandable indicators.
inset-inline-start: calc(-1 * var(--base-size-8));
width: var(--base-size-4);
border-radius: var(--base-size-2);
background-color: var(--brand-NavList-activeIndicator-color);
}
- // Optimistic "pending" highlight. Article pages are getServerSideProps routes and can
- // be slow to load, so router.asPath — and thus aria-current — only updates once the
- // page has loaded. We keep aria-current on the *loaded* page (assistive tech must not
- // be told a still-loading destination is the current page), and instead mark the
- // just-clicked link with `data-pending` for a VISUAL-ONLY accent. Brand couples its
- // active styling to `[aria-current]`, so mirror that treatment here for `[data-pending]`.
- // data-pending is only set while a *different* page is loading, so it never collides
- // with the real aria-current item.
+ // Article pages can load slowly through getServerSideProps, so aria-current stays
+ // on the loaded page and data-pending carries a visual-only accent for the clicked
+ // destination. data-pending is only set for a different loading page, so it never
+ // collides with the real aria-current item.
:global([class*="NavList__link"])[data-pending] {
color: var(--brand-color-text-default);
background-color: var(--brand-color-canvas-subtle);
}
- // Nested leaves: brand moves the background pill onto the label and adds a leading
- // accent bar (mirrors `.item--leaf:not(.item--level-1) .link[aria-current]`).
+ // Nested leaves need the same background pill and leading bar as Brand's active rule.
:global([class*="NavList__item--leaf"]:not([class*="NavList__item--level-1"]))
:global([class*="NavList__link"])[data-pending] {
background-color: transparent;
diff --git a/src/landings/components/SidebarProduct.tsx b/src/landings/components/SidebarProduct.tsx
index 5e66cf0f056d..030a03070d31 100644
--- a/src/landings/components/SidebarProduct.tsx
+++ b/src/landings/components/SidebarProduct.tsx
@@ -22,10 +22,8 @@ import { flattenDescendants, MAX_NAVLIST_LEVEL } from './sidebar-navlist-depth'
import styles from './SidebarProduct.module.scss'
-// The nearest ancestor that actually scrolls vertically. Brand's NavList.SubNav
-// wrappers use `overflow-y: hidden`, so match only auto/scroll to skip past them
-// and land on the sidebar's own overflow container. Returns null when the rail is
-// hidden (below the xxl breakpoint it is `display: none`, so nothing scrolls).
+// Brand NavList.SubNav wrappers use overflow-y hidden, so match only auto or scroll
+// to find the sidebar's own overflow container. Hidden rails return null.
function findScrollableAncestor(element: Element): HTMLElement | null {
let node = element.parentElement
while (node) {
@@ -40,12 +38,10 @@ function findScrollableAncestor(element: Element): HTMLElement | null {
type Router = ReturnType
-// Brand NavList.Item renders a plain (its `as` prop only accepts 'a' | 'button',
-// not next/link), so intercept clicks to restore next/link-style client-side
-// navigation. Modifier/middle clicks fall through to the browser so open-in-new-tab
-// still works, and the keeps links crawlable for SSR. Mirrors Breadcrumbs.tsx.
-// Returns true when it performed a client-side navigation (so the caller can move the
-// optimistic selection), false when the click was left to the browser.
+// Brand NavList.Item renders a plain anchor, not next/link, so intercept plain
+// left-clicks to restore client-side navigation. Modified clicks fall through for
+// separate tabs, the href keeps links crawlable for server-side rendering, and true
+// tells the caller to move the optimistic selection. Mirrors Breadcrumbs.tsx.
function handleNavClick(router: Router, event: MouseEvent, href: string): boolean {
if (
event.defaultPrevented ||
@@ -59,24 +55,18 @@ function handleNavClick(router: Router, event: MouseEvent, href: st
return false
}
event.preventDefault()
- // hrefs already include the locale prefix (e.g. /en/...), so disable Next.js
- // locale handling to avoid double-prefixing.
+ // Locale-prefixed hrefs need locale false so Next.js does not add the locale twice.
router.push(href, undefined, { locale: false })
return true
}
-// The sidebar renders the full product tree (hundreds of nodes) and fully remounts
-// on every navigation (key={asPath} in SidebarNav). To keep per-item cost down we
-// subscribe to the router ONCE here and hand items a stable routePath plus stable
-// navigate/prefetch callbacks, instead of every item calling useRouter itself.
+// The sidebar renders hundreds of nodes and remounts on every navigation. Subscribe
+// once here so each item gets stable routePath, navigate, and prefetch values instead
+// of calling useRouter itself.
type SidebarNavValue = {
- // The real loaded route. Drives aria-current (the semantic "current page") and the
- // auto-expanded active ancestor chain. Both must reflect the page actually loaded.
+ // The loaded route drives aria-current and the auto-expanded active ancestor chain.
routePath: string
- // The in-flight click target, or null. Drives a VISUAL-ONLY optimistic accent bar
- // (via data-pending) so the click feels acknowledged before the slow
- // getServerSideProps page loads, without lying to assistive tech about the current
- // page. Once navigation completes, the keyed remount clears it and routePath catches up.
+ // The in-flight click target drives a visual-only data-pending accent during slow loads.
pendingHref: string | null
navigate: (event: MouseEvent, href: string) => void
prefetch: (href: string) => void
@@ -91,10 +81,8 @@ function useSidebarNav(): SidebarNavValue {
return value
}
-// Props for a leaf link's : aria-current tracks the loaded page (semantics), while
-// data-pending marks the in-flight click so CSS can move the accent bar optimistically
-// without changing what screen readers announce as current. data-pending is only set
-// while a *different* page is loading, so it never double-marks the already-current item.
+// Leaf links keep aria-current on the loaded page and data-pending on a different
+// in-flight destination, so screen readers do not hear a loading page as current.
function leafLinkProps(nav: SidebarNavValue, href: string) {
return {
'aria-current': (nav.routePath === href ? 'page' : false) as 'page' | false,
@@ -102,9 +90,8 @@ function leafLinkProps(nav: SidebarNavValue, href: string) {
}
}
-// Separate context for the REST-only scroll-spy state (full asPath with query+hash,
-// and query). Kept out of SidebarNavValue so its per-navigation identity churn
-// doesn't invalidate the memoized common items — only RestNavListItem consumes it.
+// Keep REST-only scroll-spy state out of SidebarNavValue so its per-navigation
+// identity churn invalidates only RestNavListItem.
type RestNavValue = {
asPath: string
query: ReturnType['query']
@@ -119,7 +106,6 @@ function useRestNav(): RestNavValue {
return value
}
-// Hover/focus handlers for a leaf link: warm the destination so the click is fast.
function prefetchHandlers(prefetch: (href: string) => void, href: string) {
return {
onMouseEnter: () => prefetch(href),
@@ -127,12 +113,13 @@ function prefetchHandlers(prefetch: (href: string) => void, href: string) {
}
}
+// pendingHref survives slow getServerSideProps navigations because SidebarNav remounts
+// only after asPath changes. aria-current stays on the loaded route.
export const SidebarProduct = () => {
const router = useRouter()
const {
currentProduct,
- // For the sidebar we only need the short titles so we can use the
- // more "compressed" tree that is as light as possible.
+ // The sidebar only needs short titles, so MainContext supplies the compressed tree.
sidebarTree,
sidebarExpanded,
} = useMainContext()
@@ -141,20 +128,14 @@ export const SidebarProduct = () => {
const { asPath, locale, query } = router
const routePath = `/${locale}${asPath.split('?')[0].split('#')[0]}`
- // Optimistic selection: the href of an in-flight click. Used to move the accent bar
- // visually (data-pending) the instant a link is clicked, even while the destination
- // page is still loading. This SidebarProduct instance persists during the pending
- // fetch (SidebarNav keys it on asPath, which only changes once navigation completes),
- // so the state survives the wait and is discarded by the keyed remount when the new
- // route lands. aria-current is NOT derived from this: it stays on the loaded route.
+ // pendingHref moves only the visual accent while aria-current stays on the loaded route.
const [pendingHref, setPendingHref] = useState(null)
const prefetchHref = usePrefetchOnInteraction()
- // Stable callbacks so memoized items don't re-render on unrelated changes.
+ // Stable callbacks keep memoized items from re-rendering on unrelated changes.
const navigate = useCallback(
(event: MouseEvent, href: string) => {
- // Only move the optimistic highlight on a real client-side nav, not on a
- // modifier/middle click that opens a new tab (the current page stays put).
+ // Move the optimistic highlight only for client-side navigation, not modified clicks.
if (handleNavClick(router, event, href)) setPendingHref(href)
},
[router],
@@ -168,10 +149,7 @@ export const SidebarProduct = () => {
const rootRef = useRef(null)
useEffect(() => {
- // Clear the optimistic highlight if a navigation genuinely fails, so it doesn't
- // stick on a page that never loaded. Skip cancellations (err.cancelled): those
- // fire when a second click supersedes the first, and pendingHref already points at
- // that newer target, which we want to keep highlighted.
+ // Failed navigations clear pendingHref; cancellations keep the newer click highlighted.
const clearPending = (err: { cancelled?: boolean }) => {
if (!err?.cancelled) setPendingHref(null)
}
@@ -180,27 +158,19 @@ export const SidebarProduct = () => {
}, [router.events])
useEffect(() => {
- // Skip all sidebar scroll adjustments when the URL carries landing-page
- // article filters (search/category/page). Those are shallow same-page
- // updates that must not move the reader (the article grid manages its own
- // scroll position).
+ // Article filter query params are shallow same-page updates; the grid manages their scroll.
if (/[?&]articles-(filter|category|page)=/.test(router.asPath)) return
- // Brand NavList auto-expands the whole ancestor chain of the active item, so
- // scroll to the item marked aria-current="page" (the active article) rather
- // than the top-most expanded section.
+ // Brand expands every active ancestor, so scroll to the aria-current page item.
const activeArticle = rootRef.current?.querySelector('[aria-current="page"]')
if (!activeArticle) return
- // Scroll the sidebar's own overflow container by hand. `scrollIntoView` would
- // scroll every scrollable ancestor, including the document, which cancels the
- // browser's scroll to a #anchor on load and leaves the reader at the top of
- // the article. See BreadcrumbsScroller for the same approach.
+ // Scroll by hand to preserve hash anchors; BreadcrumbsScroller does the same.
const container = findScrollableAncestor(activeArticle)
if (!container) return
const containerRect = container.getBoundingClientRect()
const activeRect = activeArticle.getBoundingClientRect()
- // Setting to the top doesn't give enough context of surrounding categories
+ // Centering shows surrounding categories.
const delta =
activeRect.top - containerRect.top - (container.clientHeight - activeRect.height) / 2
container.scrollBy({ top: delta, behavior: 'instant' })
@@ -263,9 +233,8 @@ export const SidebarProduct = () => {
)
}
-// Wraps a brand NavList expandable item (renders as a