diff --git a/src/ai-tools/lib/auth-utils.ts b/src/ai-tools/lib/auth-utils.ts index 9639c6fc35b0..c85ebaba54e5 100644 --- a/src/ai-tools/lib/auth-utils.ts +++ b/src/ai-tools/lib/auth-utils.ts @@ -1,7 +1,8 @@ import { execSync } from 'child_process' /** - * Ensure GitHub token is available, exiting process if not found + * Falls back to the gh CLI token, setting process.env.GITHUB_TOKEN. + * Exits the process if neither is available. */ export function ensureGitHubToken(): void { if (!process.env.GITHUB_TOKEN) { diff --git a/src/ai-tools/lib/call-models-api.ts b/src/ai-tools/lib/call-models-api.ts index 70e2a90b6f58..db8883f7c536 100644 --- a/src/ai-tools/lib/call-models-api.ts +++ b/src/ai-tools/lib/call-models-api.ts @@ -42,7 +42,6 @@ export async function callModelsApi( ): Promise { let aiResponse: ChatCompletionChoice - // Set default model if none specified if (!promptWithContent.model) { promptWithContent.model = DEFAULT_MODEL if (verbose) { @@ -51,7 +50,6 @@ export async function callModelsApi( } try { - // Create an AbortController for timeout handling const controller = new AbortController() const timeoutId = setTimeout(() => controller.abort(), API_TIMEOUT_MS) @@ -82,7 +80,6 @@ export async function callModelsApi( if (!response.ok) { let errorMessage = `HTTP error! status: ${response.status} - ${response.statusText}` - // Try to get more detailed error information try { const errorBody = await response.json() if (errorBody.error && errorBody.error.message) { @@ -92,7 +89,6 @@ export async function callModelsApi( // If we can't parse error body, continue with basic error } - // Add helpful hints for common errors if (response.status === 401) { errorMessage += ' (Check your GITHUB_TOKEN)' } else if (response.status === 400) { @@ -135,9 +131,7 @@ export async function callModelsApi( return cleanAIResponse(aiResponse.message.content) } -// Helper function to clean up AI response content function cleanAIResponse(content: string): string { - // Remove markdown code blocks return content .replace(/^```[\w]*\n/gm, '') // Remove opening code blocks .replace(/\n```$/gm, '') // Remove closing code blocks at end diff --git a/src/ai-tools/lib/file-utils.ts b/src/ai-tools/lib/file-utils.ts index d113396793bd..63cbe92a10e7 100644 --- a/src/ai-tools/lib/file-utils.ts +++ b/src/ai-tools/lib/file-utils.ts @@ -6,9 +6,6 @@ import { schema } from '@/frame/lib/frontmatter' const MAX_DIRECTORY_DEPTH = 20 -/** - * Enhanced recursive markdown file finder with symlink, depth, and root path checks - */ export function findMarkdownFiles( dir: string, rootDir: string, @@ -33,7 +30,6 @@ export function findMarkdownFiles( return [] } visited.add(realDir) - // Prevent excessive depth if (depth > maxDepth) { return [] } @@ -71,9 +67,8 @@ interface FrontmatterProperties { } /** - * Function to merge new frontmatter properties into existing file while preserving formatting. - * Uses surgical replacement to only modify the specific field(s) being updated, - * preserving all original YAML formatting for unchanged fields. + * Replaces only the fields being updated, so the original YAML formatting + * of every other field survives. */ export function mergeFrontmatterProperties(filePath: string, newPropertiesYaml: string): string { const content = fs.readFileSync(filePath, 'utf8') @@ -90,7 +85,7 @@ export function mergeFrontmatterProperties(filePath: string, newPropertiesYaml: } try { - // Clean up the AI response - remove markdown code blocks if present + // The model often wraps its output in a code fence. let cleanedYaml = newPropertiesYaml.trim() cleanedYaml = cleanedYaml.replace(/^```ya?ml\s*\n/i, '') cleanedYaml = cleanedYaml.replace(/\n```\s*$/i, '') @@ -111,12 +106,10 @@ export function mergeFrontmatterProperties(filePath: string, newPropertiesYaml: }), ) - // Split content into lines for surgical replacement const lines = content.split('\n') let inFrontmatter = false let frontmatterEndIndex = -1 - // Find frontmatter boundaries for (let i = 0; i < lines.length; i++) { if (lines[i].trim() === '---') { if (!inFrontmatter) { @@ -128,17 +121,14 @@ export function mergeFrontmatterProperties(filePath: string, newPropertiesYaml: } } - // Replace each field value while preserving everything else for (const [key, value] of Object.entries(sanitizedProperties)) { const formattedValue = typeof value === 'string' ? `'${value.replace(/'/g, "''")}'` : value - // Find the line with this field let foundField = false for (let i = 1; i < frontmatterEndIndex; i++) { const line = lines[i] if (line.startsWith(`${key}:`)) { foundField = true - // Simple replacement: keep the field name and spacing, replace the value const colonIndex = line.indexOf(':') const leadingSpace = line.substring(colonIndex + 1, colonIndex + 2) // Usually a space lines[i] = `${key}:${leadingSpace}${formattedValue}` @@ -153,7 +143,6 @@ export function mergeFrontmatterProperties(filePath: string, newPropertiesYaml: } } - // If field doesn't exist, add it before the closing --- if (!foundField && frontmatterEndIndex > 0) { lines.splice(frontmatterEndIndex, 0, `${key}: ${formattedValue}`) frontmatterEndIndex++ diff --git a/src/ai-tools/lib/prompt-utils.ts b/src/ai-tools/lib/prompt-utils.ts index 1715d7bc2202..7d5f875ba348 100644 --- a/src/ai-tools/lib/prompt-utils.ts +++ b/src/ai-tools/lib/prompt-utils.ts @@ -17,17 +17,11 @@ export interface PromptData { max_tokens?: number } -/** - * Get the prompts directory path - */ export function getPromptsDir(): string { const __dirname = path.dirname(fileURLToPath(import.meta.url)) return path.join(__dirname, '../prompts') } -/** - * Dynamically discover available editor types from prompt files - */ export function getAvailableEditorTypes(promptDir: string): string[] { const editorTypes: string[] = [] @@ -46,16 +40,10 @@ export function getAvailableEditorTypes(promptDir: string): string[] { return editorTypes } -/** - * Get formatted description of available refinement types - */ export function getRefinementDescriptions(editorTypes: string[]): string { return editorTypes.join(', ') } -/** - * Enrich context for intro prompt on index.md files - */ export function enrichIndexContext(filePath: string, content: string): string { if (!filePath.endsWith('index.md')) return content @@ -72,7 +60,6 @@ export function enrichIndexContext(filePath: string, content: string): string { .join(' ') : '' - // Get child article titles const titles: string[] = [] if (data.children && Array.isArray(data.children)) { const dir = path.dirname(filePath) @@ -94,7 +81,6 @@ export function enrichIndexContext(filePath: string, content: string): string { } } - // Build context note const parts: string[] = [] if (productName) parts.push(`Product: ${productName}`) if (titles.length > 0) parts.push(`Child articles: ${titles.join(', ')}`) @@ -114,24 +100,19 @@ export function enrichIndexContext(filePath: string, content: string): string { return content } -/** - * Call an editor with the given content and options - */ export async function callEditor( editorType: string, content: string, promptDir: string, writeMode: boolean, verbose = false, - promptContent?: string, // Optional: use this instead of reading from file + promptContent?: string, // Use this instead of reading a prompt file ): Promise { let markdownPrompt: string if (promptContent) { - // Use provided prompt content (e.g., from Copilot Space) markdownPrompt = promptContent } else { - // Read from file const markdownPromptPath = path.join(promptDir, `${editorType}.md`) if (!fs.existsSync(markdownPromptPath)) { @@ -144,7 +125,6 @@ export async function callEditor( const prompt = load(fs.readFileSync(promptTemplatePath, 'utf8')) as PromptData - // Validate the prompt template has required properties if (!prompt.messages || !Array.isArray(prompt.messages)) { throw new Error('Invalid prompt template: missing or invalid messages array') } @@ -152,7 +132,8 @@ export async function callEditor( for (const msg of prompt.messages) { msg.content = msg.content.replace('{{markdownPrompt}}', markdownPrompt) msg.content = msg.content.replace('{{input}}', content) - // Replace writeMode template variable with simple string replacement + // Resolve the write-mode markers, then strip whatever they marked + // for removal. msg.content = msg.content.replace( //g, writeMode ? '' : '', @@ -166,7 +147,6 @@ export async function callEditor( writeMode ? '' : '', ) - // Remove sections marked for removal msg.content = msg.content.replace(/[\s\S]*?/g, '') } diff --git a/src/ai-tools/lib/spaces-utils.ts b/src/ai-tools/lib/spaces-utils.ts index e5859ec5b338..e0f993b3b046 100644 --- a/src/ai-tools/lib/spaces-utils.ts +++ b/src/ai-tools/lib/spaces-utils.ts @@ -1,6 +1,3 @@ -/** - * Copilot Space API response types - */ import { fetchWithRetry, readBodyWithTimeout } from '@/frame/lib/fetch-utils' export interface SpaceResource { @@ -25,9 +22,6 @@ export interface SpaceData { updated_at: string } -/** - * Parse a Copilot Space URL to extract org and space ID - */ export function parseSpaceUrl(url: string): { org: string; id: string } { // Expected format: https://api.github.com/orgs/{org}/copilot-spaces/{id} const match = url.match(/\/orgs\/([^/]+)\/copilot-spaces\/(\d+)/) @@ -44,9 +38,6 @@ export function parseSpaceUrl(url: string): { org: string; id: string } { } } -/** - * Fetch a Copilot Space from the GitHub API - */ export async function fetchCopilotSpace(spaceUrl: string): Promise { const { org, id } = parseSpaceUrl(spaceUrl) const apiUrl = `https://api.github.com/orgs/${org}/copilot-spaces/${id}` @@ -88,26 +79,20 @@ export async function fetchCopilotSpace(spaceUrl: string): Promise { return (await readBodyWithTimeout(response, () => response.json(), 30_000)) as SpaceData } -/** - * Convert a Copilot Space to a markdown prompt file - */ export function convertSpaceToPrompt(space: SpaceData): string { const timestamp = new Date().toISOString() const lines: string[] = [] - // Header with metadata lines.push(``) lines.push(``) lines.push(``) lines.push('') - // General instructions if (space.general_instructions) { lines.push(space.general_instructions.trim()) lines.push('') } - // Add each resource as a context section if (space.resources_attributes && space.resources_attributes.length > 0) { for (const resource of space.resources_attributes) { if (resource.resource_type === 'free_text' && resource.metadata) { diff --git a/src/ai-tools/scripts/ai-tools.ts b/src/ai-tools/scripts/ai-tools.ts index 62f2d3a04cfb..2c66e8c72938 100644 --- a/src/ai-tools/scripts/ai-tools.ts +++ b/src/ai-tools/scripts/ai-tools.ts @@ -19,7 +19,6 @@ dotenv.config({ quiet: true }) const promptDir = getPromptsDir() -// Ensure GitHub token is available ensureGitHubToken() const editorTypes = getAvailableEditorTypes(promptDir) @@ -68,7 +67,7 @@ program .option('-f, --files ', 'One or more content file paths in the content directory') .action((options: CliOptions) => { ;(async () => { - // Handle export-space workflow (standalone, doesn't process files) + // The export-space workflow is standalone and processes no files. if (options.exportSpace) { if (!options.output) { console.error('Error: --export-space requires --output option') @@ -95,13 +94,11 @@ program } } - // Validate mutually exclusive options if (options.space && options.prompt) { console.error('Error: Cannot use both --space and --prompt options') process.exit(1) } - // Files are required for processing workflows if (!options.files || options.files.length === 0) { console.error('Error: --files option is required (unless using --export-space)') process.exit(1) @@ -113,7 +110,7 @@ program let prompts: string[] = [] let promptContent: string | undefined - // Handle Space workflow (in-memory) + // Build the prompt in memory instead of reading a prompt file. if (options.space) { try { spinner.text = 'Fetching Copilot Space...' @@ -130,7 +127,6 @@ program process.exit(1) } } else { - // Handle local prompt workflow prompts = options.prompt || options.refine || [] if (prompts.length === 0) { @@ -140,7 +136,7 @@ program } } - // Validate local prompt types exist (skip for Space workflow) + // A Space prompt is not a local file, so there is nothing to validate. if (!options.space) { const availableEditors = editorTypes for (const editor of prompts) { @@ -168,16 +164,13 @@ program continue } - // Check if it's a directory const isDirectory = fs.statSync(filePath).isDirectory() for (const editorType of prompts) { try { - // For other editor types, process individual files const filesToProcess: string[] = [] if (isDirectory) { - // Find all markdown files in the directory recursively // Use process.cwd() as the root directory for safety const rootDir = fs.realpathSync(process.cwd()) filesToProcess.push(...findMarkdownFiles(filePath, rootDir)) @@ -197,7 +190,7 @@ program const relativePath = path.relative(process.cwd(), fileToProcess) spinner.text = `Processing: ${relativePath}` try { - // Expand Liquid references before processing + // Capture the intro before Liquid expansion rewrites it. let originalIntro = '' if (editorType === 'intro') { const originalContent = fs.readFileSync(fileToProcess, 'utf8') @@ -212,7 +205,6 @@ program let content = fs.readFileSync(fileToProcess, 'utf8') - // For intro prompt, add original intro and enrich context if (editorType === 'intro') { if (originalIntro) { content = `\n\n---\nOriginal intro (unresolved): ${originalIntro}\n---\n\n${content}` @@ -220,7 +212,6 @@ program content = enrichIndexContext(fileToProcess, content) } - // For content-type prompt, skip files that already have contentType if (editorType === 'content-type' && content.includes('contentType:')) { spinner.stop() console.log(`ā­ļø Skipping ${relativePath} (already has contentType)`) @@ -240,24 +231,22 @@ program if (options.write) { if (editorType === 'intro' || editorType === 'content-type') { - // For frontmatter addition/modification, merge properties instead of overwriting entire file + // Merge frontmatter instead of overwriting the whole file. const updatedContent = mergeFrontmatterProperties(fileToProcess, answer) fs.writeFileSync(fileToProcess, updatedContent, 'utf8') console.log(`āœ… Added frontmatter properties to: ${relativePath}`) } else { - // For other editor types, write the full result back to the original file fs.writeFileSync(fileToProcess, answer, 'utf8') console.log(`āœ… Updated: ${relativePath}`) } } else { - // Just output to console (current behavior) if (filesToProcess.length > 1) { console.log(`\n=== ${relativePath} ===`) } console.log(answer) } - // Always restore Liquid references after processing (even in non-write mode) + // Restore Liquid references even when not writing. if (options.verbose) { console.log(`Restoring Liquid references in: ${relativePath}`) } @@ -271,7 +260,6 @@ program try { runLiquidTagsScript('restore', [fileToProcess], false) } catch (restoreError) { - // Log restore failures in verbose mode for debugging if (options.verbose) { console.error(`Warning: Failed to restore Liquid references: ${restoreError}`) } @@ -291,7 +279,6 @@ program spinner.stop() - // Exit with appropriate code based on whether any errors occurred if (process.exitCode) { process.exit(process.exitCode) } @@ -300,9 +287,6 @@ program program.parse(process.argv) -/** - * Run liquid-tags command on specified file paths - */ function runLiquidTagsScript( command: 'expand' | 'restore', filePaths: string[], @@ -314,7 +298,6 @@ function runLiquidTagsScript( } try { - // Run liquid-tags script via tsx const liquidTagsScriptPath = path.join( process.cwd(), 'src/content-render/scripts/liquid-tags.ts', @@ -330,7 +313,6 @@ function runLiquidTagsScript( } } -// Handle graceful shutdown process.on('SIGINT', () => { console.log('\n\nšŸ›‘ Process interrupted by user') process.exit(0) diff --git a/src/archives/middleware/archived-enterprise-versions.ts b/src/archives/middleware/archived-enterprise-versions.ts index d7501fb082e8..6a678383e7c8 100644 --- a/src/archives/middleware/archived-enterprise-versions.ts +++ b/src/archives/middleware/archived-enterprise-versions.ts @@ -105,7 +105,6 @@ export default async function archivedEnterpriseVersions( const { isArchived, requestedVersion } = isArchivedVersion(req) if (!isArchived || !requestedVersion) return next() - // Skip asset paths if (patterns.assetPaths.test(req.path)) return next() const redirectCode = pathLanguagePrefixed(req.path) ? 301 : 302 @@ -176,8 +175,6 @@ export default async function archivedEnterpriseVersions( // URLs like this only need to redirect the original `req.path` // didn't already have a language if (newPath !== undefined && (newPath || !language)) { - // Construct the new URL by combining the new language and the - // new destination. const redirect = `/${language || 'en'}${newPath || withoutLanguagePath}` cacheAggressively(res) return res.safeRedirect(redirectCode, redirect) @@ -258,8 +255,6 @@ export default async function archivedEnterpriseVersions( ])() const responseTime = Date.now() - startTime - // Log warnings for slow responses to help identify degraded performance - // A response time over half the timeout indicates potential issues if (responseTime > WARN_RESPONSE_THRESHOLD) { logger.warn('Slow response from archived enterprise content', { version: requestedVersion, @@ -270,14 +265,14 @@ export default async function archivedEnterpriseVersions( }) } - // Log non-200 responses — use warn for 404s (expected for missing archived - // pages) and error for genuine upstream failures (5xx, timeouts). + // Warn on 404s, which are expected for missing archived pages. + // Everything else is a genuine upstream failure. if (r.status !== 200) { let upstreamBody: string | undefined try { upstreamBody = await readBodyWithTimeout(r, () => r.text(), timeoutConfiguration.response) } catch { - // ignore — body reading failure shouldn't affect error handling + // A body we cannot read should not change how we handle the error. } const level = r.status === 404 ? 'warn' : 'error' logger[level]('Failed to fetch archived enterprise content', { @@ -369,7 +364,6 @@ export default async function archivedEnterpriseVersions( ) } - // Continue with remaining replacements modifiedBody = modifiedBody.replaceAll( /="(\.\.\/)*assets/g, `="${ENTERPRISE_GH_PAGES_URL_PREFIX}${requestedVersion}/assets`, @@ -465,7 +459,7 @@ function getProxyPath(reqPath: string, requestedVersion: string) { } // Module-level global cache object. -// Get's populated lazily inside getFallbackRedirect(). +// Gets populated lazily inside getFallbackRedirect(). const fallbackRedirectLookups = new Map() function getFallbackRedirect(req: ExtendedRequest) { @@ -543,8 +537,8 @@ function getEarlyNotFoundReason(reqPath: string, version: string): string | null return 'double-slash' } - // Duplicated "/developer/developer/" segment — these are broken - // crawler URLs from the old developer.github.com site. + // A duplicated "/developer/developer/" segment means a broken crawler URL + // from the old developer.github.com site. if (reqPath.includes('/developer/developer/')) { return 'developer-developer' } diff --git a/src/article-api/lib/get-all-toc-items.ts b/src/article-api/lib/get-all-toc-items.ts index bf3e94824931..985bb2667c1c 100644 --- a/src/article-api/lib/get-all-toc-items.ts +++ b/src/article-api/lib/get-all-toc-items.ts @@ -19,11 +19,6 @@ interface TocItem extends LinkData { * Recursively gathers all TOC items from a page and its descendants. * This mirrors the behavior of getTocItems() in the generic-toc middleware * but works with the page.children frontmatter property. - * - * @param page - The page to gather TOC items from - * @param context - The rendering context - * @param options - Configuration options - * @returns Array of TocItems with nested childTocItems */ export async function getAllTocItems( page: Page, @@ -41,7 +36,6 @@ export async function getAllTocItems( return [] } - // Get the page's pathname for resolving children const pagePermalink = page.permalinks.find( (p) => p.languageCode === languageCode && p.pageVersion === context.currentVersion, ) @@ -92,10 +86,6 @@ export async function getAllTocItems( /** * Flattens nested TOC items into a single array. * Only includes leaf nodes (items without children) or all items based on options. - * - * @param tocItems - The nested TOC items to flatten - * @param options - Configuration options - * @returns Flat array of LinkData items */ export function flattenTocItems( tocItems: TocItem[], @@ -125,7 +115,6 @@ export function flattenTocItems( } } - // Recurse into children if (hasChildren) { recurse(item.childTocItems!) } diff --git a/src/article-api/lib/get-link-data.ts b/src/article-api/lib/get-link-data.ts index f9c33c74608e..45749ec06a0e 100644 --- a/src/article-api/lib/get-link-data.ts +++ b/src/article-api/lib/get-link-data.ts @@ -5,15 +5,9 @@ import type { LinkData } from '@/article-api/transformers/types' * Resolves link data (title, href, intro) for a given href and page * * This helper is used by landing page transformers to build link lists. - * It resolves the page from an href, renders its title and intro, and + * It resolves the page from an href (relative or absolute), renders its title + * and intro, and * returns the canonical permalink. - * - * @param href - The href to resolve (can be relative or absolute) - * @param languageCode - The language code for the current page - * @param pathname - The current page's pathname (for relative resolution) - * @param context - The rendering context - * @param resolvePath - Function to resolve an href to a Page object - * @returns LinkData with resolved title, href, and optional intro */ export async function getLinkData( href: string, diff --git a/src/article-api/lib/graphql-helpers.ts b/src/article-api/lib/graphql-helpers.ts index 6db482d7ff06..dc07ef55ca18 100644 --- a/src/article-api/lib/graphql-helpers.ts +++ b/src/article-api/lib/graphql-helpers.ts @@ -2,10 +2,7 @@ import type { Context, Page } from '@/types' import { renderContent } from '@/content-render/index' import matter from '@gr2m/gray-matter' -/** - * Extract manual content from page markdown - * Used by GraphQL transformers to get content before the auto-generated marker - */ +// Returns the part of the page markdown before the auto-generated marker. export async function extractManualContent(page: Page, context: Context): Promise { if (!page.markdown) return '' diff --git a/src/article-api/lib/load-template.ts b/src/article-api/lib/load-template.ts index 2cf169ba791b..3e17398756ed 100644 --- a/src/article-api/lib/load-template.ts +++ b/src/article-api/lib/load-template.ts @@ -2,30 +2,12 @@ import { readFileSync } from 'fs' import { join, dirname } from 'path' import { fileURLToPath } from 'url' -// Get the directory path for the transformers directory -// This will be used to resolve template paths relative to transformers const __filename = fileURLToPath(import.meta.url) const __dirname = dirname(__filename) -/** - * Load a template file from the templates directory - * - * This helper loads Liquid template files used by transformers. - * Templates are located in src/article-api/templates/ - * - * @param templateName - The name of the template file (e.g., 'landing-page.template.md') - * @returns The template content as a string - * - * @example - * ```typescript - * const template = loadTemplate('landing-page.template.md') - * const rendered = await renderContent(template, context) - * ``` - */ +// Loads a Liquid template file from src/article-api/templates, for use by +// transformers. export function loadTemplate(templateName: string): string { - // Templates are in ../templates relative to the lib directory - // lib is at src/article-api/lib - // templates is at src/article-api/templates const templatePath = join(__dirname, '../templates', templateName) return readFileSync(templatePath, 'utf8') } diff --git a/src/article-api/lib/strip-html-comments.ts b/src/article-api/lib/strip-html-comments.ts index 354bef9b0b63..1f15e67fc084 100644 --- a/src/article-api/lib/strip-html-comments.ts +++ b/src/article-api/lib/strip-html-comments.ts @@ -1,27 +1,10 @@ -/** - * Strips HTML comments from markdown content. - * Removes single-line HTML comments like - * while preserving other content. - * - * @param content - The markdown content to process - * @returns The content with HTML comments removed - */ +// Removes single-line HTML comments such as . export function stripHtmlComments(content: string): string { - // Remove single-line HTML comments () - // This matches comments that are on their own line or inline return content.replace(//g, '').trim() } -/** - * Strips HTML comments and cleans up extra blank lines. - * Useful for cleaning up rendered markdown content where HTML comments - * were on separate lines and their removal creates gaps. - * - * @param content - The markdown content to process - * @returns The content with HTML comments removed and blank lines normalized - */ +// Strips HTML comments, then collapses the blank lines their removal leaves behind. export function stripHtmlCommentsAndNormalizeWhitespace(content: string): string { - // Remove HTML comments let cleaned = stripHtmlComments(content) // Normalize multiple consecutive blank lines to at most 2 blank lines diff --git a/src/article-api/lib/summarize-schema.ts b/src/article-api/lib/summarize-schema.ts index 8ae39784399b..57f0276d6bd2 100644 --- a/src/article-api/lib/summarize-schema.ts +++ b/src/article-api/lib/summarize-schema.ts @@ -194,14 +194,12 @@ export function summarizeSchema(schema: JsonSchema): string { // compact and fast to render (shared types can recur dozens of times). const seen = new Set() - // Handle top-level composition for (const keyword of ['oneOf', 'anyOf', 'allOf'] as const) { if (schema[keyword]) { return renderCompositionVariants(keyword, schema[keyword]!, 0, 0, seen) } } - // Handle top-level array const schemaTypes = Array.isArray(schema.type) ? schema.type : schema.type ? [schema.type] : [] const isNullable = schemaTypes.includes('null') const primaryType = schemaTypes.find((t) => t !== 'null') @@ -215,7 +213,6 @@ export function summarizeSchema(schema: JsonSchema): string { const constraintStr = constraints.length ? ` (${constraints.join(', ')})` : '' const itemTitle = items.title - // Composition inside items const compositionKey = (['oneOf', 'anyOf', 'allOf'] as const).find((k) => items[k]) if (compositionKey) { const label = compositionKey.replace('Of', ' of') @@ -248,7 +245,6 @@ export function summarizeSchema(schema: JsonSchema): string { return `Array${constraintStr} of ${renderTypeConstraints(items)}${isNullable ? ' or null' : ''}` } - // Handle top-level object if (schema.properties) { // Note: we deliberately do NOT pre-mark schema.title here. Unlike the // array-items case above, a top-level object emits no visible titled diff --git a/src/article-api/liquid-renderers/index.ts b/src/article-api/liquid-renderers/index.ts index b25734dbad69..7b2739dec3ea 100644 --- a/src/article-api/liquid-renderers/index.ts +++ b/src/article-api/liquid-renderers/index.ts @@ -1,16 +1,9 @@ -/** - * API Transformer Liquid Tags - * - * This module contains custom Liquid tags used by article-api transformers - * to render API documentation in a consistent format. - */ +// Custom Liquid tags used by article-api transformers. import { restTags } from './rest-tags' -// Export all API transformer tags for registration export const apiTransformerTags = { ...restTags, } -// Re-export individual tag modules for direct access if needed export { restTags } from './rest-tags' diff --git a/src/article-api/liquid-renderers/rest-tags.ts b/src/article-api/liquid-renderers/rest-tags.ts index 63b3ee52cd6f..e8121e40c9c2 100644 --- a/src/article-api/liquid-renderers/rest-tags.ts +++ b/src/article-api/liquid-renderers/rest-tags.ts @@ -7,10 +7,7 @@ import { createLogger } from '@/observability/logger' const logger = createLogger('article-api/liquid-renderers/rest-tags') -/** - * Custom Liquid tag for rendering REST API parameters - * Usage: {% rest_parameter param %} - */ +// Usage: {% rest_parameter param %} export class RestParameter { private paramName: string @@ -56,10 +53,7 @@ export class RestParameter { } } -/** - * Custom Liquid tag for rendering REST API body parameters - * Usage: {% rest_body_parameter param indent %} - */ +// Usage: {% rest_body_parameter param indent %} export class RestBodyParameter { constructor( token: TagToken, @@ -67,7 +61,6 @@ export class RestBodyParameter { liquid: Liquid, private liquidContext?: LiquidContext, ) { - // Parse arguments - param name and optional indent level const args = token.args.trim().split(/\s+/) this.param = args[0] this.indent = args[1] ? parseInt(args[1]) : 0 @@ -106,7 +99,6 @@ export class RestBodyParameter { lines.push(`${prefix} Can be one of: ${param.enum.map((v) => `\`${v}\``).join(', ')}`) } - // Handle nested parameters if (param.childParamsGroups && param.childParamsGroups.length > 0) { for (const childGroup of param.childParamsGroups) { lines.push(await renderChildParameter(childGroup, context, indent + 1)) @@ -117,10 +109,7 @@ export class RestBodyParameter { } } -/** - * Custom Liquid tag for rendering REST API status codes - * Usage: {% rest_status_code statusCode %} - */ +// Usage: {% rest_status_code statusCode %} export class RestStatusCode { private statusCodeName: string @@ -158,9 +147,6 @@ export class RestStatusCode { } } -/** - * Helper function to render child parameters recursively - */ async function renderChildParameter( param: ChildParameter, context: Context, @@ -186,7 +172,6 @@ async function renderChildParameter( lines.push(`${prefix} Can be one of: ${param.enum.map((v: string) => `\`${v}\``).join(', ')}`) } - // Recursively handle nested parameters if (param.childParamsGroups && param.childParamsGroups.length > 0) { for (const child of param.childParamsGroups) { lines.push(await renderChildParameter(child, context, indent + 1)) @@ -196,9 +181,6 @@ async function renderChildParameter( return lines.join('\n') } -/** - * Helper function to convert HTML to markdown - */ async function htmlToMarkdown(html: string, context: Context): Promise { if (!html) return '' @@ -215,12 +197,10 @@ async function htmlToMarkdown(html: string, context: Context): Promise { if (process.env.NODE_ENV !== 'production') { throw error } - // Fallback to simple text extraction return fastTextOnly(html) } } -// Export tag names for registration export const restTags = { rest_parameter: RestParameter, rest_body_parameter: RestBodyParameter, diff --git a/src/article-api/middleware/article-body.ts b/src/article-api/middleware/article-body.ts index cae5e7f003b8..ce0c6717c341 100644 --- a/src/article-api/middleware/article-body.ts +++ b/src/article-api/middleware/article-body.ts @@ -11,10 +11,7 @@ import { normalizeRenderedMarkdown } from '@/article-api/lib/normalize-markdown' import { allVersions } from '@/versions/lib/all-versions' import type { Page } from '@/types' -/** - * Creates a mocked rendering request and contextualizes it. - * This is used to prepare a request for rendering pages in markdown format. - */ +// Creates a mocked rendering request, contextualized for rendering a page as markdown. async function createContextualizedRenderingRequest(pathname: string, page: Page) { const mockedContext: Context = {} const renderingReq = { @@ -52,7 +49,6 @@ export async function getArticleBody(req: ExtendedRequestWithPageInfo) { if (archived?.isArchived) throw new Error(`Page ${pathname} is archived and can't be rendered in markdown.`) - // Extract apiVersion from query params if provided const apiVersion = req.query.apiVersion as string | undefined // With the catch-all ArticleTransformer registered last, @@ -60,7 +56,6 @@ export async function getArticleBody(req: ExtendedRequestWithPageInfo) { const transformer = transformerRegistry.findTransformer(page) if (!transformer) throw new Error(`No transformer found for page: ${pathname}`) - // Use the transformer const renderingReq = await createContextualizedRenderingRequest(pathname, page) // Determine the API version to use (provided or latest) @@ -68,7 +63,6 @@ export async function getArticleBody(req: ExtendedRequestWithPageInfo) { const currentVersion = renderingReq.context.currentVersion let effectiveApiVersion = apiVersion - // Use latest version if not provided if (!effectiveApiVersion && currentVersion && allVersions[currentVersion]) { effectiveApiVersion = allVersions[currentVersion].latestApiVersion || undefined } diff --git a/src/article-api/middleware/pagelist.ts b/src/article-api/middleware/pagelist.ts index 295a18c49acf..a80978c481c4 100644 --- a/src/article-api/middleware/pagelist.ts +++ b/src/article-api/middleware/pagelist.ts @@ -35,15 +35,12 @@ router.get( defaultCacheControl(res) const response = { - // Simple list of all version strings versions: allVersionKeys, - // GHES-specific information ghesVersions: enterpriseServerReleases.supported, ghesLatest: enterpriseServerReleases.latest, ghesLatestStable: enterpriseServerReleases.latestStable, ghesReleaseCandidate: enterpriseServerReleases.releaseCandidate, ghesDeprecated: enterpriseServerReleases.deprecated, - // Full version details allVersions, } @@ -77,9 +74,7 @@ router.get( ) const response = { - // Simple list of language codes languages: languageKeys, - // Full language details (without redirectPatterns) allLanguages: sanitizedLanguages, } @@ -142,10 +137,8 @@ router.get( const pages = req.context.pages - // the keys of `context.pages` are permalinks const keys = Object.keys(pages) - // we filter the permalinks to get only our target version and language const filteredPermalinks = keys.filter((key) => versionMatcher(key, req.context!.currentVersion!, req.context!.currentLanguage!), ) diff --git a/src/article-api/middleware/validation.ts b/src/article-api/middleware/validation.ts index 031b60704e49..eff2c9c842da 100644 --- a/src/article-api/middleware/validation.ts +++ b/src/article-api/middleware/validation.ts @@ -16,23 +16,19 @@ export const pagelistValidationMiddleware = ( res: Response, next: NextFunction, ) => { - // get version from path, fallback to default version if it can't be resolved const versionFromPath = getVersionStringFromPath(req.path) || nonEnterpriseDefaultVersion - // in the rare case that this failed, probably won't be reached + // Defensive: getVersionStringFromPath falls back above, so this is unreachable. if (!versionFromPath) return res.status(400).json({ error: `Couldn't get version from the given path.` }) - // get the language from path, fallback to english if it can't be resolved const langFromPath = getLangFromPath(req.path) || 'en' - // in the rare case that the language fallback failed if (!langFromPath) return res.status(400).json({ error: `Couldn't get language from the from the given path.`, }) - // set the version and language in the context, we'll use it later req.context!.currentVersion = versionFromPath req.context!.currentLanguage = langFromPath return next() @@ -96,7 +92,7 @@ export const pageValidationMiddleware = ( pathname = `/${req.context.currentLanguage}` } - // Initialize archived property to avoid it being undefined + // Initialized so downstream readers never see undefined. req.pageinfo.archived = { isArchived: false } if (!(pathname in req.context.pages)) { @@ -133,24 +129,20 @@ export const apiVersionValidationMiddleware = ( ) => { const apiVersion = req.query.apiVersion as string | string[] | undefined - // If no apiVersion is provided, continue (it will default to latest) if (!apiVersion) { return next() } - // Validate apiVersion is a single string, not an array if (Array.isArray(apiVersion)) { return res.status(400).json({ error: "Multiple 'apiVersion' keys" }) } - // Get the version from the pathname query parameter const pathname = req.pageinfo?.pathname || (req.query.pathname as string) if (!pathname) { // This should not happen as pathValidationMiddleware runs first throw new Error('pathname not available for apiVersion validation') } - // Extract version from the pathname const currentVersion = getVersionStringFromPath(pathname) || nonEnterpriseDefaultVersion const versionInfo = allVersions[currentVersion] @@ -160,7 +152,6 @@ export const apiVersionValidationMiddleware = ( const validApiVersions = versionInfo.apiVersions || [] - // If this version has API versioning, validate the provided version if (validApiVersions.length > 0 && !validApiVersions.includes(apiVersion)) { return res.status(400).json({ error: `Invalid apiVersion '${apiVersion}' for ${currentVersion}. Valid API versions are: ${validApiVersions.join(', ')}`, diff --git a/src/article-api/scripts/generate-api-docs.ts b/src/article-api/scripts/generate-api-docs.ts index ba1b64c5f83d..081a72799d09 100644 --- a/src/article-api/scripts/generate-api-docs.ts +++ b/src/article-api/scripts/generate-api-docs.ts @@ -11,23 +11,18 @@ type ApiDoc = { } function main({ sources, outputPath }: { sources: string[]; outputPath: string }): void { - // Extract API documentation comments from all source files const allDocs = sources.flatMap((sourcePath) => extractApiDocs(sourcePath)) - // Generate markdown const markdown = generateMarkdown(allDocs) - // Update README updateReadme(outputPath, markdown) console.log('API documentation generated successfully!') } -// Extract API docs from comments in the file function extractApiDocs(file: string): ApiDoc[] { const apiDocs: ApiDoc[] = [] - // get the content from the api routes const content = readFileSync(file, 'utf8') // Get the router method definitions with JSDOC-style comments @@ -40,15 +35,13 @@ function extractApiDocs(file: string): ApiDoc[] { const method = match[2] const path = match[3] - // The description is first line of the comment const description = commentBlock .trim() .split('\n')[0] .trim() .replace(/^\*\s*/, '') - // Grab the other elements from the comment - // we currently support: params, returns, examples, throws + // We currently support params, returns, examples, and throws. const params = extractParams(commentBlock) const returns = extractReturns(commentBlock) const examples = extractExample(commentBlock) @@ -82,7 +75,6 @@ function extractThrows(commentBlock: string): string[] { return throws } -// Extract parameters from comment block function extractParams(commentBlock: string): string[] { const paramRegex = /@param\s+{([^}]+)}\s+([^\s]+)\s+([^\n]+)/g const params: string[] = [] @@ -98,7 +90,6 @@ function extractParams(commentBlock: string): string[] { return params } -// Extract return info from comment block function extractReturns(commentBlock: string): string { const returnMatch = commentBlock.match(/@returns\s+{([^}]+)}\s+([^\n]+)/) if (returnMatch) { @@ -109,7 +100,6 @@ function extractReturns(commentBlock: string): string { return '' } -// Extract example from comment block function extractExample(commentBlock: string): string { const exampleMatch = commentBlock.match(/@example\b([\s\S]*?)(?=\s*\*\s*@|\s*\*\/|$)/) if (exampleMatch) { @@ -123,7 +113,6 @@ function extractExample(commentBlock: string): string { return '' } -// Generate markdown from parsed documentation function generateMarkdown(apiDocs: ApiDoc[]): string { let markdown = '## Reference: API endpoints\n\n' @@ -157,14 +146,12 @@ function generateMarkdown(apiDocs: ApiDoc[]): string { return markdown } -// Update README with generated documentation function updateReadme(readmePath: string, markdown: string): void { if (existsSync(readmePath)) { let readme = readFileSync(readmePath, 'utf8') const placeholderComment = `` - // Replace API documentation section, or append to end if (readme.includes(placeholderComment)) { const pattern = new RegExp(`${placeholderComment}[\\s\\S]*`, 'g') readme = readme.replace(pattern, `${placeholderComment}\n${markdown}`) diff --git a/src/article-api/tests/article-body.ts b/src/article-api/tests/article-body.ts index 932d3e230ebb..ea47acacb8b2 100644 --- a/src/article-api/tests/article-body.ts +++ b/src/article-api/tests/article-body.ts @@ -38,7 +38,6 @@ describe('article body api', () => { const res = await get(makeURL('/en/get-started/start-your-journey/hello-world')) expect(res.statusCode).toBe(200) - // Check that octicons without aria-label get auto-generated ones expect(res.body).toContain('aria-label="check icon"') expect(res.body).toContain('aria-label="git branch icon"') }) @@ -47,7 +46,6 @@ describe('article body api', () => { const res = await get(makeURL('/en/get-started/start-your-journey/hello-world')) expect(res.statusCode).toBe(200) - // Check that custom aria-labels are preserved expect(res.body).toContain('aria-label="Supported"') expect(res.body).toContain('aria-label="Not supported"') }) @@ -56,7 +54,6 @@ describe('article body api', () => { const res = await get(makeURL('/en/get-started/start-your-journey/hello-world')) expect(res.statusCode).toBe(200) - // Check that octicons with width attribute still get aria-labels expect(res.body).toContain('aria-label="rocket icon"') expect(res.body).toContain('width="32"') }) diff --git a/src/article-api/tests/audit-logs-transformer.ts b/src/article-api/tests/audit-logs-transformer.ts index d7869a6538ac..9e20c1cd2874 100644 --- a/src/article-api/tests/audit-logs-transformer.ts +++ b/src/article-api/tests/audit-logs-transformer.ts @@ -23,21 +23,16 @@ describe('Audit Logs transformer', () => { expect(res.statusCode).toBe(200) expect(res.headers['content-type']).toContain('text/markdown') - // Check for the main heading expect(res.body).toContain('# Security log events') - // Check for intro expect(res.body).toContain( 'Learn about security log events recorded for your personal account.', ) - // Check for manual content section heading expect(res.body).toContain('## About security log events') - // Check for new main heading expect(res.body).toContain('## Audit log events') - // Check for category heading // The template renders "### Category" expect(res.body).toMatch(/### \w+/) }) @@ -72,17 +67,14 @@ describe('Audit Logs transformer', () => { ) expect(res.statusCode).toBe(200) - // Check for event action header // #### `action.name` expect(res.body).toMatch(/#### `[\w.]+`/) - // Check for fields section - either common fields summary or additional fields per event const body = res.body const hasCommonFields = body.includes('### Common fields') const hasAdditionalFields = body.includes('**Additional fields:**') expect(hasCommonFields || hasAdditionalFields).toBe(true) - // Validate that a known common field is in the common section and not duplicated if (hasCommonFields) { const commonFieldsIndex = body.indexOf('### Common fields') const commonFieldsSection = body.slice( @@ -92,7 +84,7 @@ describe('Audit Logs transformer', () => { expect(commonFieldsSection).toContain('`action`') } - // Ensure common fields do not appear in any "Additional fields" section + // Common fields are listed once in their own section, never repeated per event. if (hasAdditionalFields) { const additionalSections = body.split('**Additional fields:**').slice(1) for (const section of additionalSections) { @@ -101,7 +93,6 @@ describe('Audit Logs transformer', () => { } } - // Check for reference section expect(res.body).toContain('**Reference:**') }) diff --git a/src/article-api/tests/bespoke-landing-transformer.ts b/src/article-api/tests/bespoke-landing-transformer.ts index 0e6828ea5c0a..3e300116fedc 100644 --- a/src/article-api/tests/bespoke-landing-transformer.ts +++ b/src/article-api/tests/bespoke-landing-transformer.ts @@ -12,10 +12,8 @@ describe('bespoke landing transformer', () => { expect(res.statusCode).toBe(200) expect(res.headers['content-type']).toContain('text/markdown') - // Check for title expect(res.body).toContain('# Article Grid Bespoke Landing') - // Should have intro expect(res.body).toContain('A test page for testing') }) @@ -23,7 +21,6 @@ describe('bespoke landing transformer', () => { const res = await get(makeURL('/en/get-started/article-grid-bespoke')) expect(res.statusCode).toBe(200) - // Should have Articles section with all descendant articles (recursive) expect(res.body).toContain('## Articles') expect(res.body).toContain('[Grid Article One]') expect(res.body).toContain('[Grid Article Two]') diff --git a/src/article-api/tests/codeql-cli-transformer.ts b/src/article-api/tests/codeql-cli-transformer.ts index 2f82ae5279c1..dca2b457f07f 100644 --- a/src/article-api/tests/codeql-cli-transformer.ts +++ b/src/article-api/tests/codeql-cli-transformer.ts @@ -21,20 +21,16 @@ describe('codeql cli article body api', () => { expect(res.statusCode).toBe(200) expect(res.headers['content-type']).toContain('text/markdown') - // Check for title injection expect(res.body).toContain('# database analyze') - // Check for intro injection expect(res.body).toContain( 'Analyze a database, producing meaningful results in the context of the source code.', ) - // Check for content expect(res.body).toContain('## Synopsis') expect(res.body).toContain('## Description') expect(res.body).toContain('## Options') - // Verify HTML comments are stripped expect(res.body).not.toContain('') expect(res.body).not.toContain('') expect(res.body).not.toContain('') diff --git a/src/article-api/tests/discovery-landing-transformer.ts b/src/article-api/tests/discovery-landing-transformer.ts index fe013bf7f5e6..8c300d0fd713 100644 --- a/src/article-api/tests/discovery-landing-transformer.ts +++ b/src/article-api/tests/discovery-landing-transformer.ts @@ -12,11 +12,9 @@ describe('discovery landing transformer', () => { expect(res.statusCode).toBe(200) expect(res.headers['content-type']).toContain('text/markdown') - // Check for title and intro expect(res.body).toContain('# Landing Page Carousel') expect(res.body).toContain('A test category page for testing the LandingCarousel component') - // Should have Articles section with all descendant articles expect(res.body).toContain('## Articles') expect(res.body).toContain('[Carousel Article One]') expect(res.body).toContain('[Carousel Article Two]') @@ -29,10 +27,8 @@ describe('discovery landing transformer', () => { expect(res.statusCode).toBe(200) expect(res.headers['content-type']).toContain('text/markdown') - // Check for title expect(res.body).toContain('# Article Grid Discovery') - // Should have Articles section with all descendant articles (recursive) expect(res.body).toContain('## Articles') expect(res.body).toContain('[Grid Article One]') expect(res.body).toContain('[Grid Article Two]') @@ -41,14 +37,11 @@ describe('discovery landing transformer', () => { }) test('handles discovery landing structure consistently', async () => { - // Discovery pages should have a consistent structure const res = await get(makeURL('/en/get-started/carousel')) expect(res.statusCode).toBe(200) - // Should have intro expect(res.body).toMatch(/^# .+\n\n.+\n\n/) - // Should have at least one section expect(res.body).toContain('##') }) diff --git a/src/article-api/tests/get-link-data.ts b/src/article-api/tests/get-link-data.ts index fc11254faba3..41f9a16b4e39 100644 --- a/src/article-api/tests/get-link-data.ts +++ b/src/article-api/tests/get-link-data.ts @@ -2,7 +2,6 @@ import { describe, expect, test, vi } from 'vitest' import { getLinkData } from '@/article-api/lib/get-link-data' import type { Context, Page, Permalink } from '@/types' -// Helper to create a minimal mock page function createMockPage(options: { title?: string intro?: string @@ -18,7 +17,6 @@ function createMockPage(options: { return page as unknown as Page } -// Helper to create a minimal context function createContext(currentVersion = 'free-pro-team@latest'): Context { return { currentVersion } as unknown as Context } diff --git a/src/article-api/tests/github-apps-transformer.ts b/src/article-api/tests/github-apps-transformer.ts index 80fc1acfc4bc..d2ff01d2dfcb 100644 --- a/src/article-api/tests/github-apps-transformer.ts +++ b/src/article-api/tests/github-apps-transformer.ts @@ -29,13 +29,10 @@ describe('GitHub Apps transformer', () => { expect(res.statusCode).toBe(200) expect(res.headers['content-type']).toContain('text/markdown') - // Check for the main heading expect(res.body).toContain('# Endpoints available for GitHub App installation access tokens') - // Should have category headings as h2 expect(res.body).toMatch(/^## /m) - // Should not contain HTML comments expect(res.body).not.toMatch(//) }) @@ -46,10 +43,8 @@ describe('GitHub Apps transformer', () => { expect(res.statusCode).toBe(200) expect(res.headers['content-type']).toContain('text/markdown') - // Check for the main heading expect(res.body).toContain('# Endpoints available for GitHub App user access tokens') - // Should have category headings as h2 expect(res.body).toMatch(/^## /m) }) @@ -62,10 +57,8 @@ describe('GitHub Apps transformer', () => { expect(res.statusCode).toBe(200) expect(res.headers['content-type']).toContain('text/markdown') - // Check for the main heading expect(res.body).toContain('# Endpoints available for fine-grained personal access tokens') - // Should have category headings as h2 expect(res.body).toMatch(/^## /m) }) @@ -81,7 +74,7 @@ describe('GitHub Apps transformer', () => { console.log(`[DEBUG] Test response: ${res.statusCode} in ${Date.now() - startTime}ms`) expect(res.statusCode).toBe(200) - // Check for bullet list items with asterisks (per content guidelines) + // Content guidelines require asterisk bullets, not hyphens. expect(res.body).toContain('*') expect(res.body).toMatch(/\* \[`[A-Z]+ \//) }) @@ -94,7 +87,6 @@ describe('GitHub Apps transformer', () => { ) expect(res.statusCode).toBe(200) - // Check for common HTTP verbs expect(res.body).toMatch(/`GET \//) // May also have POST, PUT, PATCH, DELETE depending on data }) @@ -118,10 +110,8 @@ describe('GitHub Apps transformer', () => { expect(res.statusCode).toBe(200) expect(res.headers['content-type']).toContain('text/markdown') - // Check for the main heading expect(res.body).toContain('# Permissions required for GitHub Apps') - // Should have permission group headings as h2 expect(res.body).toMatch(/^## /m) }) @@ -134,10 +124,8 @@ describe('GitHub Apps transformer', () => { expect(res.statusCode).toBe(200) expect(res.headers['content-type']).toContain('text/markdown') - // Check for the main heading expect(res.body).toContain('# Permissions required for fine-grained personal access tokens') - // Should have permission group headings as h2 expect(res.body).toMatch(/^## /m) }) @@ -145,7 +133,6 @@ describe('GitHub Apps transformer', () => { const res = await get(makeURL('/en/rest/authentication/permissions-required-for-github-apps')) expect(res.statusCode).toBe(200) - // Check for table structure expect(res.body).toContain('| Endpoint | Access | Tokens | Additional Permissions |') expect(res.body).toContain('|----------|--------|--------|------------------------|') }) @@ -154,7 +141,6 @@ describe('GitHub Apps transformer', () => { const res = await get(makeURL('/en/rest/authentication/permissions-required-for-github-apps')) expect(res.statusCode).toBe(200) - // Check for access levels in table cells expect(res.body).toMatch(/\| read \|/) expect(res.body).toMatch(/\| write \|/) // May also have admin depending on data @@ -164,11 +150,9 @@ describe('GitHub Apps transformer', () => { const res = await get(makeURL('/en/rest/authentication/permissions-required-for-github-apps')) expect(res.statusCode).toBe(200) - // Check for legend expect(res.body).toContain('UAT = user access token') expect(res.body).toContain('IAT = installation access token') - // Check that token types appear in actual table rows (not just the legend) expect(res.body).toMatch(/\|\s*(?:UAT|IAT|UAT, IAT|None)\s*\|/) }) @@ -176,7 +160,6 @@ describe('GitHub Apps transformer', () => { const res = await get(makeURL('/en/rest/authentication/permissions-required-for-github-apps')) expect(res.statusCode).toBe(200) - // Check for checkmark (āœ“) or cross (āœ—) symbols in Additional Permissions column expect(res.body).toMatch(/\| [āœ“āœ—] \|/) }) @@ -184,7 +167,6 @@ describe('GitHub Apps transformer', () => { const res = await get(makeURL('/en/rest/authentication/permissions-required-for-github-apps')) expect(res.statusCode).toBe(200) - // Should have multiple permission group headings const headings = res.body.match(/^## .* permissions for .*/gm) expect(headings).toBeTruthy() if (headings) { @@ -198,7 +180,6 @@ describe('GitHub Apps transformer', () => { const res = await get(makeURL('/en/rest/authentication/permissions-required-for-github-apps')) expect(res.statusCode).toBe(200) - // Check for manual content that should be in the markdown expect(res.body).toContain('GitHub Apps are created with a set of permissions') }) @@ -222,7 +203,6 @@ describe('GitHub Apps transformer', () => { const res = await get(makeURL('/en/rest/authentication/permissions-required-for-github-apps')) expect(res.statusCode).toBe(200) - // Check that AUTOTITLE has been resolved expect(res.body).not.toContain('[AUTOTITLE]') }) @@ -247,7 +227,6 @@ describe('GitHub Apps transformer', () => { test('Missing apiVersion defaults to latest', async () => { const res = await get(makeURL('/en/rest/authentication/permissions-required-for-github-apps')) expect(res.statusCode).toBe(200) - // Should work without explicit apiVersion }) test('Non-GitHub Apps pages are not transformed', async () => { diff --git a/src/article-api/tests/graphql-transformer.ts b/src/article-api/tests/graphql-transformer.ts index f0668c40b88c..127584b86d46 100644 --- a/src/article-api/tests/graphql-transformer.ts +++ b/src/article-api/tests/graphql-transformer.ts @@ -8,7 +8,7 @@ const makeURL = (pathname: string): string => { } describe('GraphQL transformer', { timeout: 10000 }, () => { - // Cache expensive responses to avoid duplicate requests + // Several tests hit the same URL, so cache slow responses to avoid repeated requests. const responseCache = new Map>>() const getCached = async (url: string) => { @@ -32,7 +32,6 @@ describe('GraphQL transformer', { timeout: 10000 }, () => { expect(res.statusCode).toBe(200) expect(res.headers['content-type']).toContain('text/markdown') - // Check for the main heading expect(res.body).toContain('# Repositories') // Items render as flat alphabetical level 2 headings with a kind suffix @@ -47,10 +46,8 @@ describe('GraphQL transformer', { timeout: 10000 }, () => { // Item headings are now at level 3 expect(res.body).toContain('## repository - query') - // Check for query description expect(res.body).toContain('Lookup a given repository by the owner and repository name.') - // Check for type (without link) expect(res.body).toContain('**Type:** Repository') }) @@ -61,7 +58,6 @@ describe('GraphQL transformer', { timeout: 10000 }, () => { // codeOfConduct query is in the meta category expect(res.body).toContain('### Arguments for `codeOfConduct`') - // Check for specific arguments in bullet format expect(res.body).toContain('`key` (String!)') expect(res.body).toContain("The code of conduct's key.") }) @@ -101,7 +97,6 @@ describe('GraphQL transformer', { timeout: 10000 }, () => { expect(res.body).toContain('## User - object') expect(res.body).toContain('`repositories`') - // Check for nested argument bullets expect(res.body).toContain('`first`') expect(res.body).toContain('Returns the first n elements from the list.') expect(res.body).toContain('`orderBy`') @@ -172,10 +167,8 @@ describe('GraphQL transformer', { timeout: 10000 }, () => { const res = await getCached('/en/graphql/reference') expect(res.statusCode).toBe(200) - // Check for main heading expect(res.body).toContain('# Reference') - // Check for intro with liquid variable rendered expect(res.body).toMatch(/(GitHub|HubGit) GraphQL API schema/) }) }) @@ -185,15 +178,12 @@ describe('GraphQL transformer', { timeout: 10000 }, () => { const res = await getCached('/en/graphql/overview/changelog') expect(res.statusCode).toBe(200) - // Check for main heading expect(res.body).toContain('# Changelog') - // Check for intro expect(res.body).toContain( 'The GraphQL schema changelog is a list of recent and upcoming changes', ) - // Check for manual content expect(res.body).toContain( 'Breaking changes include changes that will break existing queries', ) @@ -201,13 +191,10 @@ describe('GraphQL transformer', { timeout: 10000 }, () => { // Index page shows latest year (2026) entries only expect(res.body).toContain('## Schema changes for 2026-') - // Check for change items expect(res.body).toContain('### The GraphQL schema includes these changes:') - // Should NOT contain entries from other years expect(res.body).not.toContain('## Schema changes for 2025-') - // Check for year navigation expect(res.body).toContain('2026') expect(res.body).toContain('2025') }) @@ -216,23 +203,19 @@ describe('GraphQL transformer', { timeout: 10000 }, () => { const res = await getCached('/en/graphql/overview/changelog/2025') expect(res.statusCode).toBe(200) - // Check for year-specific heading expect(res.body).toContain('# GraphQL changelog for 2025') - // Check for date-based changelog sections from 2025 expect(res.body).toContain('## Schema changes for 2025-') - // Should NOT contain entries from other years expect(res.body).not.toContain('## Schema changes for 2026-') expect(res.body).not.toContain('## Schema changes for 2024-') }) test('changelog removes HTML tags from changes', async () => { - // Use a year page that has the specific test data + // The 2025 fixture is the one whose change descriptions contain HTML. const res = await getCached('/en/graphql/overview/changelog/2025') expect(res.statusCode).toBe(200) - // Check that HTML tags are removed expect(res.body).toContain('Field suggestedReviewerActors was added') expect(res.body).not.toContain('') expect(res.body).not.toContain('') @@ -244,17 +227,13 @@ describe('GraphQL transformer', { timeout: 10000 }, () => { const res = await getCached('/en/graphql/overview/breaking-changes') expect(res.statusCode).toBe(200) - // Check for main heading expect(res.body).toContain('# Breaking changes') - // Check for intro expect(res.body).toContain('Learn about recent and upcoming breaking changes') - // Check for manual content expect(res.body).toContain('## About breaking changes') expect(res.body).toContain('Breaking:** Changes that will break existing queries') - // Check for date-based sections expect(res.body).toContain('## Changes scheduled for 2025-04-01') expect(res.body).toContain('## Changes scheduled for 2026-04-01') }) @@ -263,7 +242,6 @@ describe('GraphQL transformer', { timeout: 10000 }, () => { const res = await getCached('/en/graphql/overview/breaking-changes') expect(res.statusCode).toBe(200) - // Check for breaking criticality expect(res.body).toMatch(/\*\*Breaking\*\*\s+A change will be made to `\w+\.\w+`\./) expect(res.body).toMatch(/\*\*Description:\*\*.*will be removed/) expect(res.body).toMatch(/\*\*Reason:\*\*/) @@ -274,7 +252,6 @@ describe('GraphQL transformer', { timeout: 10000 }, () => { expect(res.statusCode).toBe(200) expect(res.body).toContain('scheduled for') - // Check that HTML tags are removed from descriptions expect(res.body).not.toContain('

') expect(res.body).not.toContain('

') expect(res.body).not.toContain('') @@ -291,7 +268,6 @@ describe('GraphQL transformer', { timeout: 10000 }, () => { const res = await getCached('/en/graphql/reference/repos') expect(res.statusCode).toBe(200) - // Make sure the raw AUTOTITLE tag is not present expect(res.body).not.toContain('[AUTOTITLE]') }) @@ -308,7 +284,6 @@ describe('GraphQL transformer', { timeout: 10000 }, () => { const res = await getCached('/en/graphql/overview/breaking-changes') expect(res.statusCode).toBe(200) - // Check that liquid variables in intro are rendered expect(res.body).toMatch(/(GitHub|HubGit) GraphQL API/) expect(res.body).not.toContain('{% data variables.product.prodname_dotcom %}') }) diff --git a/src/article-api/tests/journey-landing-transformer.ts b/src/article-api/tests/journey-landing-transformer.ts index 09e56d9af8fc..63a4110695ea 100644 --- a/src/article-api/tests/journey-landing-transformer.ts +++ b/src/article-api/tests/journey-landing-transformer.ts @@ -12,7 +12,6 @@ describe('journey landing transformer', () => { expect(res.statusCode).toBe(200) expect(res.headers['content-type']).toContain('text/markdown') - // Check for journey tracks (now under Links section with track title as h3) expect(res.body).toContain('## Links') expect(res.body).toContain('### First Track') expect(res.body).toContain('* [Hello World](/en/get-started/start-your-journey/hello-world)') diff --git a/src/article-api/tests/pageinfo.ts b/src/article-api/tests/pageinfo.ts index f17804619633..5739bc97c2f6 100644 --- a/src/article-api/tests/pageinfo.ts +++ b/src/article-api/tests/pageinfo.ts @@ -47,9 +47,7 @@ describe('pageinfo api', () => { 'Get started using HubGit to manage Git repositories and collaborate with others.', ) expect(meta.documentType).toBe('category') - // Canonical URLs should not have redirectedFrom expect(meta.redirectedFrom).toBeUndefined() - // Check that it can be cached at the CDN expect(res.headers['set-cookie']).toBeUndefined() expect(res.headers['cache-control']).toContain('public') expect(res.headers['cache-control']).toMatch(/max-age=[1-9]/) diff --git a/src/article-api/tests/pagelist.ts b/src/article-api/tests/pagelist.ts index 0cdb55cfcd04..684d64cde4c8 100644 --- a/src/article-api/tests/pagelist.ts +++ b/src/article-api/tests/pagelist.ts @@ -23,7 +23,6 @@ describe.each(allVersionKeys)('pagelist api for %s', async (versionKey) => { } }) - // queries the pagelist API for each version const res = await get(`/api/pagelist/en/${versionKey}`) test('is reachable, returns 200 OK', async () => { @@ -82,7 +81,6 @@ describe('Versions API', () => { const data = JSON.parse(res.body) - // Check top-level keys exist expect(data).toHaveProperty('versions') expect(data).toHaveProperty('ghesVersions') expect(data).toHaveProperty('ghesLatest') @@ -91,20 +89,16 @@ describe('Versions API', () => { expect(data).toHaveProperty('ghesDeprecated') expect(data).toHaveProperty('allVersions') - // Versions array should contain expected values expect(Array.isArray(data.versions)).toBe(true) expect(data.versions).toContain('free-pro-team@latest') expect(data.versions).toContain('enterprise-cloud@latest') - // GHES versions should be an array of version strings expect(Array.isArray(data.ghesVersions)).toBe(true) expect(data.ghesVersions.length).toBeGreaterThan(0) expect(data.ghesVersions[0]).toMatch(/^\d+\.\d+$/) - // ghesLatest should be a valid version string expect(data.ghesLatest).toMatch(/^\d+\.\d+$/) - // allVersions should be an object with version details expect(typeof data.allVersions).toBe('object') expect(data.allVersions['free-pro-team@latest']).toHaveProperty('version') expect(data.allVersions['free-pro-team@latest']).toHaveProperty('versionTitle') @@ -119,17 +113,14 @@ describe('Languages API', () => { const data = JSON.parse(res.body) - // Check top-level keys exist expect(data).toHaveProperty('languages') expect(data).toHaveProperty('allLanguages') - // Languages array should contain expected values expect(Array.isArray(data.languages)).toBe(true) expect(data.languages).toContain('en') expect(data.languages).toContain('ja') expect(data.languages).toContain('es') - // allLanguages should be an object with language details expect(typeof data.allLanguages).toBe('object') expect(data.allLanguages.en).toHaveProperty('name') expect(data.allLanguages.en).toHaveProperty('code') @@ -137,7 +128,7 @@ describe('Languages API', () => { expect(data.allLanguages.en).toHaveProperty('locale') expect(data.allLanguages.en.name).toBe('English') - // Should not contain redirectPatterns (not JSON serializable) + // redirectPatterns is not JSON serializable, so it must not be in the response. expect(data.allLanguages.ja).not.toHaveProperty('redirectPatterns') }) }) diff --git a/src/article-api/tests/resolve-path.ts b/src/article-api/tests/resolve-path.ts index 6156b8c1e689..3fc654f84fd8 100644 --- a/src/article-api/tests/resolve-path.ts +++ b/src/article-api/tests/resolve-path.ts @@ -2,7 +2,6 @@ import { describe, expect, test } from 'vitest' import { resolvePath } from '@/article-api/lib/resolve-path' import type { Context, Page } from '@/types' -// Helper to create a minimal mock page function createMockPage(relativePath: string): Page { return { relativePath, @@ -10,7 +9,6 @@ function createMockPage(relativePath: string): Page { } as unknown as Page } -// Helper to create a minimal context with pages function createContext(pages: Record): Context { return { pages } as unknown as Context } diff --git a/src/article-api/tests/rest-transformer.ts b/src/article-api/tests/rest-transformer.ts index cee19cf3a62c..4673e9a89e2a 100644 --- a/src/article-api/tests/rest-transformer.ts +++ b/src/article-api/tests/rest-transformer.ts @@ -24,13 +24,10 @@ describe('REST transformer', () => { expect(res.statusCode).toBe(200) expect(res.headers['content-type']).toContain('text/markdown') - // Check for the main heading expect(res.body).toContain('# GitHub Actions Artifacts') - // Check for intro (using fixture's prodname_actions which is 'HubGit Actions') expect(res.body).toContain('Use the REST API to interact with artifacts in HubGit Actions.') - // Check for manual content section heading expect(res.body).toContain('## About artifacts in HubGit Actions') }) @@ -38,13 +35,10 @@ describe('REST transformer', () => { const res = await get(makeURL('/en/rest/actions/artifacts')) expect(res.statusCode).toBe(200) - // Check for operation heading expect(res.body).toContain('## List artifacts for a repository') - // Check for HTTP method and endpoint expect(res.body).toContain('GET /repos/{owner}/{repo}/actions/artifacts') - // Check for operation description expect(res.body).toContain('Lists all artifacts for a repository.') }) @@ -52,13 +46,10 @@ describe('REST transformer', () => { const res = await get(makeURL('/en/rest/actions/artifacts')) expect(res.statusCode).toBe(200) - // Check for parameters heading expect(res.body).toContain('### Parameters') - // Check for headers section expect(res.body).toContain('#### Headers') - // Check for accept header expect(res.body).toContain('**`accept`** (string)') expect(res.body).toContain('Setting to `application/vnd.github+json` is recommended.') }) @@ -67,10 +58,8 @@ describe('REST transformer', () => { const res = await get(makeURL('/en/rest/actions/artifacts')) expect(res.statusCode).toBe(200) - // Check for path and query parameters section expect(res.body).toContain('#### Path and query parameters') - // Check for specific parameters expect(res.body).toContain('**`owner`** (string) (required)') expect(res.body).toContain('The account owner of the repository.') @@ -90,10 +79,8 @@ describe('REST transformer', () => { console.log(`[DEBUG] Test response: ${res.statusCode} in ${Date.now() - startTime}ms`) expect(res.statusCode).toBe(200) - // Check for status codes section expect(res.body).toContain('### HTTP response status codes') - // Check for specific status code expect(res.body).toContain('**200**') expect(res.body).toContain('OK') }) @@ -102,14 +89,11 @@ describe('REST transformer', () => { const res = await get(makeURL('/en/rest/actions/artifacts')) expect(res.statusCode).toBe(200) - // Check for code examples section expect(res.body).toContain('### Code examples') - // Check for request/response labels expect(res.body).toContain('**Request:**') expect(res.body).toContain('**Response schema (Status: 200):**') - // Check for curl code block expect(res.body).toContain('```curl') expect(res.body).toContain('curl -L \\') expect(res.body).toContain('-X GET \\') @@ -120,7 +104,6 @@ describe('REST transformer', () => { const res = await get(makeURL('/en/rest/actions/artifacts')) expect(res.statusCode).toBe(200) - // Check that auth note is at the top using [!NOTE] syntax expect(res.body).toContain('[!NOTE]') expect(res.body).toContain('Authorization: Bearer ') expect(res.body).toContain('application/vnd.github+json') @@ -130,7 +113,6 @@ describe('REST transformer', () => { const res = await get(makeURL('/en/rest/actions/artifacts')) expect(res.statusCode).toBe(200) - // Check for API version in the auth note (any valid date format) expect(res.body).toMatch(/X-GitHub-Api-Version: \d{4}-\d{2}-\d{2}/) }) @@ -138,7 +120,6 @@ describe('REST transformer', () => { const res = await get(makeURL('/en/rest/actions/artifacts', '2022-11-28')) expect(res.statusCode).toBe(200) - // Check for the specified API version in auth note expect(res.body).toContain('X-GitHub-Api-Version: 2022-11-28') }) @@ -150,7 +131,6 @@ describe('REST transformer', () => { expect(res.body).toContain('HubGit Actions') expect(res.body).not.toContain('{% data variables.product.prodname_actions %}') - // Check in both the intro and the manual content section expect(res.body).toMatch(/Use the REST API to interact with artifacts in HubGit Actions/) expect(res.body).toMatch(/About artifacts in HubGit Actions/) }) @@ -159,15 +139,12 @@ describe('REST transformer', () => { const res = await get(makeURL('/en/rest/actions/artifacts')) expect(res.statusCode).toBe(200) - // Check that AUTOTITLE has been resolved to actual link text // The link should have the actual page title, not "AUTOTITLE" expect(res.body).toContain('[Storing workflow data as artifacts]') expect(res.body).toContain('(/en/actions/using-workflows/storing-workflow-data-as-artifacts)') - // Make sure the raw AUTOTITLE tag is not present expect(res.body).not.toContain('[AUTOTITLE]') - // Verify the link appears in the manual content section expect(res.body).toMatch( /About artifacts in HubGit Actions[\s\S]*Storing workflow data as artifacts/, ) @@ -177,7 +154,6 @@ describe('REST transformer', () => { const res = await get(makeURL('/en/rest/actions/artifacts')) expect(res.statusCode).toBe(200) - // Check that markdown links are preserved expect(res.body).toMatch(/\[.*?\]\(\/en\/.*?\)/) }) @@ -185,14 +161,12 @@ describe('REST transformer', () => { const res = await get(makeURL('/en/rest/actions/artifacts')) expect(res.statusCode).toBe(200) - // Check for markdown-formatted schema expect(res.body).toContain('**Response schema (Status: 200):**') // Schema should be rendered as a markdown bullet list, not JSON expect(res.body).toContain('* `total_count`:') expect(res.body).toContain('* `artifacts`:') - // Should not contain raw JSON Schema keywords expect(res.body).not.toContain('"properties":') }) @@ -240,7 +214,6 @@ describe('REST transformer', () => { const res = await get(makeURL('/en/rest/actions/artifacts')) expect(res.statusCode).toBe(200) - // Should include the default API version in auth note (any valid date format) expect(res.body).toMatch(/X-GitHub-Api-Version: \d{4}-\d{2}-\d{2}/) }) @@ -248,7 +221,6 @@ describe('REST transformer', () => { const res = await get(makeURL('/en/rest/actions/artifacts')) expect(res.statusCode).toBe(200) - // Check for multiple operation headings expect(res.body).toContain('## List artifacts for a repository') expect(res.body).toContain('## Get an artifact') expect(res.body).toContain('## Delete an artifact') @@ -281,7 +253,7 @@ describe('REST transformer', () => { // because it looks for 'rest' in the path and gets the category/subcategory after it // e.g. /ja/rest/actions/artifacts should work the same as /en/rest/actions/artifacts - // Verify the operation content is present (in English, since REST data is not translated) + // REST data is not translated, so the operation content is English either way. expect(res.body).toContain('## List artifacts for a repository') expect(res.body).toContain('GET /repos/{owner}/{repo}/actions/artifacts') @@ -294,7 +266,6 @@ describe('REST transformer', () => { // One of them must be present expect(hasJapaneseTitle || hasEnglishTitle).toBe(true) - // Verify the appropriate content based on which language was served if (hasJapaneseTitle) { // If Japanese is loaded, expect Japanese intro text expect(res.body).toContain('ć‚¢ćƒ¼ćƒ†ć‚£ćƒ•ć‚”ć‚Æćƒˆ') diff --git a/src/article-api/tests/secret-scanning-transformer.ts b/src/article-api/tests/secret-scanning-transformer.ts index be236faf377a..c5daac87568e 100644 --- a/src/article-api/tests/secret-scanning-transformer.ts +++ b/src/article-api/tests/secret-scanning-transformer.ts @@ -13,20 +13,16 @@ describe('secret scanning article body api', () => { expect(res.statusCode).toBe(200) expect(res.headers['content-type']).toContain('text/markdown') - // Check for expected content expect(res.body).toContain('# Supported secret scanning patterns') expect(res.body).toContain('## Supported secrets') - // Verify HTML comments are stripped expect(res.body).not.toMatch(//) - // Verify HTML icon spans are not present (would be replaced with āœ“/āœ—) + // The icon spans are replaced with plain āœ“/āœ— characters. expect(res.body).not.toMatch(/]*aria-label="Supported"/) expect(res.body).not.toMatch(/]*aria-label="Unsupported"/) - // Verify no raw HTML span tags remain expect(res.body).not.toMatch(/]*>/) - // Verify table content is present with providers expect(res.body).toMatch(/|\s*Provider\s*|/) expect(res.body).toMatch(/\| (Adafruit|AWS|Alibaba|Amazon)/) diff --git a/src/article-api/tests/toc-transformer.ts b/src/article-api/tests/toc-transformer.ts index 4bef199a0879..6edd660fadd0 100644 --- a/src/article-api/tests/toc-transformer.ts +++ b/src/article-api/tests/toc-transformer.ts @@ -12,12 +12,11 @@ describe('toc transformer', () => { expect(res.statusCode).toBe(200) expect(res.headers['content-type']).toContain('text/markdown') - // Check for title and intro expect(res.body).toContain('# Category page of GitHub Actions') expect(res.body).toContain('Learn how to migrate your existing CI/CD workflows') - // Should have Links section with children (uses full title, not shortTitle) expect(res.body).toContain('## Links') + // Child links use the full title, not shortTitle. expect(res.body).toContain('[Subcategory page about Actions](/en/actions/category/subcategory)') }) @@ -27,10 +26,8 @@ describe('toc transformer', () => { expect(res.statusCode).toBe(200) expect(res.headers['content-type']).toContain('text/markdown') - // Check for title expect(res.body).toContain('# Subcategory page about Actions') - // Should have Links section with children expect(res.body).toContain('## Links') }) @@ -46,10 +43,8 @@ describe('toc transformer', () => { const res = await get(makeURL('/en/actions/category/subcategory')) expect(res.statusCode).toBe(200) - // Should NOT have href paths as titles expect(res.body).not.toContain('* [/en/actions/category/subcategory/') - // Should have proper article titles (or shortTitle) expect(res.body).toMatch(/\[.*\]\(\/en\/actions\/category\/subcategory\/.*\)/) }) }) diff --git a/src/article-api/tests/webhooks-transformer.ts b/src/article-api/tests/webhooks-transformer.ts index 8c611b285177..32f6db087ac0 100644 --- a/src/article-api/tests/webhooks-transformer.ts +++ b/src/article-api/tests/webhooks-transformer.ts @@ -22,10 +22,8 @@ describe('Webhooks transformer', () => { expect(res.statusCode).toBe(200) expect(res.headers['content-type']).toContain('text/markdown') - // Should have title expect(res.body).toContain('# Webhook events and payloads') - // Should have intro expect(res.body).toContain('Learn about when each webhook event occurs') }) @@ -33,7 +31,6 @@ describe('Webhooks transformer', () => { const res = await get(makeURL('/en/webhooks/webhook-events-and-payloads')) expect(res.statusCode).toBe(200) - // Check for webhook event headers (## webhook_name) expect(res.body).toMatch(/^## \w+/m) }) @@ -41,7 +38,6 @@ describe('Webhooks transformer', () => { const res = await get(makeURL('/en/webhooks/webhook-events-and-payloads')) expect(res.statusCode).toBe(200) - // Should list action types for webhooks with multiple actions expect(res.body).toContain('**Action type:**') }) @@ -49,7 +45,6 @@ describe('Webhooks transformer', () => { const res = await get(makeURL('/en/webhooks/webhook-events-and-payloads')) expect(res.statusCode).toBe(200) - // Should show availability as a heading expect(res.body).toContain('### Availability') }) @@ -57,7 +52,6 @@ describe('Webhooks transformer', () => { const res = await get(makeURL('/en/webhooks/webhook-events-and-payloads')) expect(res.statusCode).toBe(200) - // Check for some known manual content from the markdown file expect(res.body).toContain('About webhook events and payloads') }) @@ -73,7 +67,6 @@ describe('Webhooks transformer', () => { const res = await get(makeURL('/en/webhooks/webhook-events-and-payloads')) expect(res.statusCode).toBe(200) - // Check that data variables are rendered (not left as Liquid syntax) expect(res.body).not.toContain('{% data') expect(res.body).not.toContain('{{') }) @@ -90,10 +83,8 @@ describe('Webhooks transformer', () => { const res = await get(makeURL('/en/webhooks/webhook-events-and-payloads')) expect(res.statusCode).toBe(200) - // Should show payload object parameters section expect(res.body).toContain('### Webhook payload object') expect(res.body).toContain('#### Webhook payload object parameters') - // Should have a markdown table with parameter columns (may have extra spacing from formatting) expect(res.body).toMatch(/\|\s*Name\s*\|\s*Type\s*\|\s*Description\s*\|/) }) @@ -101,7 +92,6 @@ describe('Webhooks transformer', () => { const res = await get(makeURL('/en/webhooks/webhook-events-and-payloads')) expect(res.statusCode).toBe(200) - // Should include webhook descriptions (converted from HTML to plain text) // Using actual descriptions from real webhook data expect(res.body).toContain('A check run was completed') }) @@ -110,11 +100,9 @@ describe('Webhooks transformer', () => { const res = await get(makeURL('/en/webhooks/webhook-events-and-payloads')) expect(res.statusCode).toBe(200) - // Should show parameter names and types in tables expect(res.body).toContain('`action`') expect(res.body).toContain('`string`') expect(res.body).toContain('`object`') - // Should mark required parameters expect(res.body).toContain('**Required.**') }) @@ -132,7 +120,6 @@ describe('Webhooks transformer', () => { const res = await get(makeURL('/en/webhooks/webhook-events-and-payloads')) expect(res.statusCode).toBe(200) - // Should have a common parameters section at the top expect(res.body).toContain('## Common payload parameters') expect(res.body).toContain('Most webhook events include these standard parameters') diff --git a/src/audit-logs/lib/deduplicate.ts b/src/audit-logs/lib/deduplicate.ts index 39411f47df76..f637fddb4a28 100644 --- a/src/audit-logs/lib/deduplicate.ts +++ b/src/audit-logs/lib/deduplicate.ts @@ -77,7 +77,6 @@ export async function writeDeduplicatedAuditLogData( } } - // Write shared files const sharedDir = path.join(AUDIT_LOG_DATA_DIR, 'shared') if (!existsSync(sharedDir)) { await mkdirp(sharedDir) diff --git a/src/audit-logs/lib/index.ts b/src/audit-logs/lib/index.ts index 4a5bcfcde5d4..4da4d9e11cd3 100644 --- a/src/audit-logs/lib/index.ts +++ b/src/audit-logs/lib/index.ts @@ -19,11 +19,10 @@ import config from './config.json' export const AUDIT_LOG_DATA_DIR = 'src/audit-logs/data' -// cache of audit log data const auditLogEventsCache = new Map>() const categorizedAuditLogEventsCache = new Map>() -// Shared dedup data — loaded once, shared across all versions +// Shared dedup data, loaded once and shared across all versions. let sharedEntries: DeduplicatedAuditLogEntry[] | null = null let sharedFieldsPool: string[][] | null = null let sharedVersionIndex: AuditLogVersionIndex | null = null @@ -58,10 +57,10 @@ function loadSharedFormat(): boolean { sharedFormatAvailable = true } catch (err) { if (isFileNotFoundError(err)) { - // Shared files don't exist — fall back to per-version files silently. + // Shared files don't exist, so fall back to per-version files silently. sharedFormatAvailable = false } else { - // Corrupt JSON, schema mismatch, etc. — surface this instead of hiding it. + // Corrupt JSON, schema mismatch, and so on. Surface it instead of hiding it. console.error('Failed to load shared audit log dedup format (corrupt data?):', err) throw err } @@ -108,7 +107,6 @@ type PipelineConfig = { appendedDescriptions: Record } -// get category notes from config export function getCategoryNotes(): CategoryNotes { const auditLogConfig = config as AuditLogConfig return auditLogConfig.categoryNotes || {} @@ -126,7 +124,6 @@ export type TitleResolutionContext = Context & { // request (~90–150ms of repeated work). See docs-engineering#6650. const referenceLinksMarkdownCache = new Map>() -// Resolves docs_reference_links URLs to markdown links export function resolveReferenceLinksToMarkdown( docsReferenceLinks: string, context: TitleResolutionContext, @@ -147,7 +144,6 @@ async function computeReferenceLinksToMarkdown( docsReferenceLinks: string, context: TitleResolutionContext, ): Promise { - // Handle multiple comma-separated or space-separated links const links = docsReferenceLinks .split(/[,\s]+/) .map((link) => link.trim()) @@ -188,7 +184,6 @@ async function computeReferenceLinksToMarkdown( return markdownLinks.join(', ') } -// Resolves docs_reference_links URLs to page titles async function resolveReferenceLinksToTitles( docsReferenceLinks: string, context: TitleResolutionContext, @@ -197,7 +192,6 @@ async function resolveReferenceLinksToTitles( return '' } - // Handle multiple comma-separated or space-separated links const links = docsReferenceLinks .split(/[,\s]+/) .map((link) => link.trim()) @@ -258,7 +252,6 @@ export function getAuditLogEvents(page: string, version: string): AuditLogEventT auditLogEventsCache.set(openApiVersion, new Map()) } if (!auditLogEventsCache.get(openApiVersion)?.has(page)) { - // Try shared deduplicated format first const events = reconstructEventsFromSharedFormat(openApiVersion, page) if (events) { auditLogEventsCache.get(openApiVersion)?.set(page, events) @@ -307,7 +300,6 @@ export function getCategorizedAuditLogEvents(page: string, version: string): Cat return categorizedAuditLogEventsCache.get(openApiVersion)?.get(page) || {} } -// Filters audit log events based on allowlist values. export async function filterByAllowlistValues({ eventsToCheck, allowListValues, @@ -337,7 +329,6 @@ export async function filterByAllowlistValues({ if (seen.has(event.action)) continue seen.add(event.action) - // Merge global fields with event-specific fields const mergedFields = event.fields ? [...new Set([...globalFields, ...event.fields])] : globalFields.length > 0 @@ -351,7 +342,6 @@ export async function filterByAllowlistValues({ fields: mergedFields, } - // Resolve reference link titles if context is provided if (titleContext && event.docs_reference_links && event.docs_reference_links !== 'N/A') { try { minimal.docs_reference_titles = await resolveReferenceLinksToTitles( @@ -375,13 +365,6 @@ export async function filterByAllowlistValues({ // Filters audit log events based on allowlist values and processes an // event's supported GHES versions. // -// * eventsToCheck: events to consider -// * allowListvalue: allowlist value to filter by -// * currentEvents: events already collected -// * pipelineConfig: audit log pipeline config data -// * auditLogPage: the audit log page the event belongs to -// * titleContext: optional context for resolving reference link titles -// // Mutates `currentGhesEvents` and updates it with any new filtered for audit // log events, the object maps GHES versions to page events for that version e.g.: // @@ -442,10 +425,8 @@ export async function filterAndUpdateGhesDataByAllowlistValues({ if (seenByGhesVersion.get(fullGhesVersion)?.has(event.action)) continue if (ghesVersionAllowlists.includes(allowListValue)) { - // Get event-specific fields (prefer GHES version fields, fall back to base fields) const eventFields = event.ghes[ghesVersion].fields || event.fields - // Merge global fields with event-specific fields const mergedFields = eventFields ? [...new Set([...globalFields, ...eventFields])] : globalFields.length > 0 @@ -459,7 +440,6 @@ export async function filterAndUpdateGhesDataByAllowlistValues({ fields: mergedFields, } - // Resolve reference link titles if context is provided if (titleContext && event.docs_reference_links && event.docs_reference_links !== 'N/A') { try { minimal.docs_reference_titles = await resolveReferenceLinksToTitles( @@ -501,7 +481,6 @@ export async function filterAndUpdateGhesDataByAllowlistValues({ } } -// Categorizes the given array of audit log events by event category function categorizeEvents(events: AuditLogEventT[]) { const categorizedEvents: CategorizedEvents = {} for (const event of events) { diff --git a/src/audit-logs/scripts/rebuild-dedup.ts b/src/audit-logs/scripts/rebuild-dedup.ts index e369478ac057..32daff1c6ef4 100644 --- a/src/audit-logs/scripts/rebuild-dedup.ts +++ b/src/audit-logs/scripts/rebuild-dedup.ts @@ -1,16 +1,14 @@ -/** - * Rebuilds the deduplicated "shared" audit log format - * (src/audit-logs/data/shared/entries.json, fields-pool.json, and - * version-index.json) from the per-version page files already on disk. - * - * Unlike `sync-audit-log`, this does NOT fetch from github/audit-log-allowlists - * and needs no GITHUB_TOKEN. Use it to regenerate the shared files when the - * per-version JSON files have changed but the shared files are stale (for - * example, an older audit-log-pipeline PR that predates the dedup format), which - * makes the `deduplication` test fail. - * - * npm run rebuild-audit-log-dedup - */ +// Rebuilds the deduplicated "shared" audit log format from the per-version page +// files already on disk: src/audit-logs/data/shared/entries.json and +// fields-pool.json, plus src/audit-logs/data/version-index.json. +// +// Unlike `sync-audit-log`, this does NOT fetch from github/audit-log-allowlists +// and needs no GITHUB_TOKEN. Use it to regenerate the shared files when the +// per-version JSON files have changed but the shared files are stale, for +// example on an older audit-log-pipeline PR that predates the dedup format. +// That is what makes the `deduplication` test fail. +// +// npm run rebuild-audit-log-dedup import { existsSync, readdirSync, readFileSync, statSync } from 'fs' import path from 'path' diff --git a/src/audit-logs/scripts/sync.ts b/src/audit-logs/scripts/sync.ts index e46b83565ee6..1718d90416a4 100755 --- a/src/audit-logs/scripts/sync.ts +++ b/src/audit-logs/scripts/sync.ts @@ -1,12 +1,7 @@ -/** - * Required env variables: - * - * GITHUB_TOKEN - * - * Gets latest audit log event data, extracts the data we need for rendering on - * the 3 different audit log pages, and writes out the data to files versioned - * per page. - */ +// Gets the latest audit log event data, extracts what we need for the 3 audit +// log pages, and writes it out to files versioned per page. +// +// Requires GITHUB_TOKEN. import { existsSync } from 'fs' import { readFile, writeFile } from 'fs/promises' import { mkdirp } from 'mkdirp' @@ -72,7 +67,6 @@ async function main() { pipelineConfig.sha = mainSha await writeFile(configFilepath, JSON.stringify(pipelineConfig, null, 2)) - // Load pages and redirects for title resolution console.log('Loading pages and redirects for title resolution...') const pageList = await loadPages(undefined, ['en']) const pages = await loadPageMap(pageList) diff --git a/src/automated-pipelines/lib/update-markdown.ts b/src/automated-pipelines/lib/update-markdown.ts index 49addfc6ee14..964e0c78bce2 100644 --- a/src/automated-pipelines/lib/update-markdown.ts +++ b/src/automated-pipelines/lib/update-markdown.ts @@ -14,7 +14,6 @@ import type { MarkdownFrontmatter } from '@/types' const logger = createLogger(import.meta.url) -// Type definitions - extending existing type to add missing fields and make most fields optional type FrontmatterData = Partial & { autogenerated?: string [key: string]: unknown @@ -67,8 +66,9 @@ type ChildrenComparison = { const ROOT_INDEX_FILE = 'content/index.md' export const MARKDOWN_COMMENT = '\n\n' -// Main entrypoint into this module. This function adds, removes, and updates -// versions frontmatter in all directories under the targetDirectory. +// Main entrypoint into this module. +// Walks every directory under targetDirectory, adding and removing Markdown +// files and keeping the index.md children and versions frontmatter in sync. export async function updateContentDirectory({ targetDirectory, sourceContent, @@ -87,7 +87,6 @@ async function removeMarkdownFiles( sourceFiles: string[], autogeneratedType: string | undefined, ): Promise { - // Copy the autogenerated Markdown files to the target directory const autogeneratedFiles = await getAutogeneratedFiles(targetDirectory, autogeneratedType) // If the first array contains items that the second array does not, // it means that a Markdown page was deleted from the OpenAPI schema @@ -98,7 +97,6 @@ async function removeMarkdownFiles( count: filesToRemove.length, }) } - // Markdown files that need to be deleted for (const file of filesToRemove) { unlinkSync(file) } @@ -140,19 +138,15 @@ async function updateMarkdownFiles( for (const [file, newContent] of Object.entries(sourceContent)) { await updateMarkdownFile(file, newContent.data, newContent.content) } - // This function recursively updates the index.md files in each - // of the directories under targetDirectory await updateDirectory(targetDirectory, frontmatter, { indexOrder }) - // We don't want to update directories that the pipelines don't affect - // so we make one call to update only the root index.md file - // in targetDirectory to prevent any unintended changes + // The pipelines should not touch directories they do not own, so this + // call updates only the index.md file in the parent directory. await updateDirectory(path.dirname(targetDirectory), frontmatter, { rootDirectoryOnly: true }) } -// If the Markdown file already exists on disk, we only update the -// content and version frontmatter to allow writers to manually -// edit the modifiable content of the file. If the Markdown file doesn't -// exists, we create a new Markdown file. +// If the Markdown file already exists on disk, we update only the content +// and the versions frontmatter, so writers can hand-edit the other fields. +// If it does not exist, we create it. async function updateMarkdownFile( file: string, sourceData: FrontmatterData, @@ -160,8 +154,6 @@ async function updateMarkdownFile( commentDelimiter: string = MARKDOWN_COMMENT, ): Promise { if (existsSync(file)) { - // update only the versions property of the file, assuming - // the other properties have already been added and edited const { data, content } = matter(await readFile(file, 'utf-8')) // Double check that the comment delimiter is only used once @@ -179,7 +171,6 @@ async function updateMarkdownFile( const isDelimiterMissing = !matches const isContentSame = automatedContent === sourceContent const isVersionsSame = isEqual(sourceData.versions, data.versions) - // Only proceed if the content or versions have changed if (isContentSame && isVersionsSame && !isDelimiterMissing) { logger.debug('Markdown file unchanged, skipping', { file }) return @@ -219,14 +210,12 @@ async function updateDirectory( { rootDirectoryOnly = false, shortTitle = false, indexOrder = {} }: UpdateDirectoryOptions = {}, ): Promise { const initialDirectoryListing = await getDirectoryInfo(directory) - // If there are no children on disk, remove the directory if (initialDirectoryListing.directoryContents.length === 0 && !rootDirectoryOnly) { logger.info('Removing empty directory', { directory }) await rimraf(directory) return } - // Recursively update child directories if (!rootDirectoryOnly) { await Promise.all( initialDirectoryListing.childDirectories.map(async (subDirectory) => { @@ -256,7 +245,6 @@ async function updateDirectory( } if (!rootDirectoryOnly) { - // Update the versions in the index.md file to match the directory contents directoryFiles.push(...childDirectories.map((dir) => path.join(dir, 'index.md'))) const newVersions = await getIndexFileVersions(directory, directoryFiles) const isVersionEqual = isEqual(newVersions, data.versions) @@ -265,7 +253,6 @@ async function updateDirectory( } } - // Update the index.md file contents and write the file to disk const dataUpdatedChildren = updateIndexChildren( data, { itemsToAdd, itemsToRemove }, @@ -295,7 +282,6 @@ function getChildrenToCompare( const isEarlyAccess = (item: string) => isRootIndexFile(indexFile) && item === 'early-access' - // Get the list of children from the directory contents const childrenOnDisk = directoryContents .map((file) => `${path.basename(file, '.md')}`) .filter((item) => !isEarlyAccess(item)) @@ -317,7 +303,7 @@ function getChildrenToCompare( // the order of the first items in the index files children // property. All other items are sorted and appended to list. // -// 2. If not config is defined and the index file is an +// 2. If no config is defined and the index file is an // autogenerated file, we sort all the children alphabetically. // // 3. If the index file is not autogenerated, we leave the ordering @@ -332,7 +318,6 @@ function updateIndexChildren( const { itemsToAdd, itemsToRemove } = childUpdates const childPrefix = rootIndex ? '' : '/' - // Get a new list of children with added and removed items const children = [...(data.children || [])] // remove the '/' prefix used in index.md children .map((item) => item.replace(childPrefix, '')) @@ -373,8 +358,8 @@ function updateIndexChildren( return updatedData } -// Gets the contents of the index.md file from disk if it exits or -// creates a new index.md file with the default frontmatter. +// Gets the contents of the index.md file from disk if it exists, +// or returns default frontmatter for a new one. async function getIndexFileContents( indexFile: string, frontmatter: FrontmatterData, @@ -413,7 +398,6 @@ async function getIndexFileVersions( `File ${filepath} does not exist while assembling directory index.md files to create parent version.`, ) } - // If not a markdown(x) file, skip it if (!file.endsWith('.md') && !file.endsWith('.mdx')) { return } @@ -536,12 +520,10 @@ function checkVersionContinuity(versions: (string | undefined)[]): boolean { return availableVersions.every(Boolean) } -// Returns true if the indexFile is the root index.md file function isRootIndexFile(indexFile: string): boolean { return indexFile === ROOT_INDEX_FILE } -// Creates a new directory if it doesn't exist async function createDirectory(targetDirectory: string): Promise { if (!existsSync(targetDirectory)) { await mkdirp(targetDirectory) diff --git a/src/automated-pipelines/tests/rendering.ts b/src/automated-pipelines/tests/rendering.ts index e4a1d2cb6e10..f8dfc17ca55f 100644 --- a/src/automated-pipelines/tests/rendering.ts +++ b/src/automated-pipelines/tests/rendering.ts @@ -6,7 +6,6 @@ import { describe, expect, test, vi } from 'vitest' import { loadPages } from '@/frame/lib/page-data' import { get } from '@/tests/helpers/e2etest' -// Type definitions for page objects type Page = { autogenerated?: string fullPath: string @@ -17,7 +16,6 @@ type Page = { } } -// Get a list of the autogenerated pages const pageList: Page[] = await loadPages(undefined, ['en']) describe('autogenerated docs render', () => { diff --git a/src/automated-pipelines/tests/update-markdown.ts b/src/automated-pipelines/tests/update-markdown.ts index 210ffb9dc447..3b2290b86864 100644 --- a/src/automated-pipelines/tests/update-markdown.ts +++ b/src/automated-pipelines/tests/update-markdown.ts @@ -10,7 +10,6 @@ import type { FrontmatterVersions } from '@/types' import { updateContentDirectory } from '../lib/update-markdown' -// Type definitions type ContentItem = { data: { title: string @@ -92,8 +91,6 @@ describe('automated content directory updates', () => { contentDataFullPath[path.join(targetDirectory, key)] = newContentData[key] } - // Rewrites the content directory in the operating system's - // temp directory. await updateContentDirectory({ targetDirectory, sourceContent: contentDataFullPath, diff --git a/src/content-linter/scripts/disable-rules.ts b/src/content-linter/scripts/disable-rules.ts index a6210829e740..9d07356915ce 100755 --- a/src/content-linter/scripts/disable-rules.ts +++ b/src/content-linter/scripts/disable-rules.ts @@ -1,6 +1,6 @@ // Disables markdownlint rules in markdown files with same-line comments. This is // useful when introducing a new rule that causes many failures. The comments -// can be fixed and removed at while updating the file later. +// can be fixed and removed while updating the file later. // // Usage: // diff --git a/src/content-linter/scripts/generate-docs.ts b/src/content-linter/scripts/generate-docs.ts index be11c3378252..c70ced85f2ba 100644 --- a/src/content-linter/scripts/generate-docs.ts +++ b/src/content-linter/scripts/generate-docs.ts @@ -14,7 +14,6 @@ function main() { markdown.push('| Rule ID | Rule Name(s) | Description | Severity | Tags |') markdown.push('| ------- | ------------ | ----------- | -------- | ---- |') - // Collect all rules and their generated rows const mdRules: Array<{ ruleId: string; row: string }> = [] const ghRules: Array<{ ruleId: string; row: string }> = [] const ghdRules: Array<{ ruleId: string; row: string }> = [] @@ -35,7 +34,6 @@ function main() { row.push(allConfig[ruleName].severity) row.push(rule.tags.join(', ')) - // Categorize rules by their ID prefix const ruleData = { ruleId: rule.names[0], row: `| ${row.join(' | ')} |` } if (rule.names[0].startsWith('GHD')) { ghdRules.push(ruleData) @@ -46,7 +44,6 @@ function main() { } } - // Sort each category alphabetically by rule ID mdRules.sort((a, b) => a.ruleId.localeCompare(b.ruleId)) ghRules.sort((a, b) => a.ruleId.localeCompare(b.ruleId)) ghdRules.sort((a, b) => a.ruleId.localeCompare(b.ruleId)) @@ -66,13 +63,12 @@ function main() { writeFileSync('data/reusables/contributing/content-linter-rules.md', markdown.join('\n')) } -// The search-replace rule configures multiple psuedo-rules +// The search-replace rule configures multiple pseudo-rules // under the rules key. function getSearchReplaceRules(srRule: Rule, ruleConfig: Config) { const name = srRule.information ? `[search-replace](${srRule.information})` : 'search-replace' const markdown = [] - // Sort rules alphabetically by name const sortedRules = [...(ruleConfig.rules || [])].sort((a, b) => a.name.localeCompare(b.name)) for (const rule of sortedRules) { diff --git a/src/content-linter/scripts/lint-content.ts b/src/content-linter/scripts/lint-content.ts index 416c33b37b78..b58caf6b09bb 100755 --- a/src/content-linter/scripts/lint-content.ts +++ b/src/content-linter/scripts/lint-content.ts @@ -26,7 +26,6 @@ import type { } from 'markdownlint' import type { Rule, Config } from '@/content-linter/types' -// Type definitions for Markdownlint results interface LintError { lineNumber: number ruleNames: string[] @@ -88,11 +87,8 @@ interface FormattedResult { type FormattedResults = Record -/** - * Config that applies to all rules in all environments (CI, reports, precommit). - */ +// Config that applies to all rules in all environments (CI, reports, precommit). export const globalConfig = { - // Do not ever lint these filepaths excludePaths: ['content/contributing/', 'data/llms-txt/'], } @@ -150,7 +146,8 @@ async function main() { // Get the updated paths after validation (invalid paths will have been filtered out) const validatedPaths = program.opts().paths - // If paths has not been specified, lint all files + // With no paths and no --summary-by-rule, fall back to the files changed + // in the local git checkout. const files = getFilesToLint( (summaryByRule && ALL_CONTENT_DIR) || validatedPaths || getChangedFiles(), ) @@ -169,7 +166,6 @@ async function main() { spinner.start() const start = Date.now() - // Initializes the config to pass to markdownlint based on the input options const { config, configuredRules } = getMarkdownLintConfig(errorsOnly, rules) // Run Markdownlint for content directory @@ -395,9 +391,8 @@ function getFilesToLint(inputPaths: string[]): FileList { const root = path.resolve(languages.en.dir) const contentDir = path.join(root, 'content') const dataDir = path.join(root, 'data') - // The path passed to Markdownlint is what is displayed - // in the error report, so we want to normalize it and - // and make it relative if it's absolute. + // The path passed to Markdownlint is what is displayed in the error report, + // so normalize it and make it relative if it's absolute. for (const rawPath of inputPaths) { const absPath = path.resolve(rawPath) if (fs.statSync(rawPath).isDirectory()) { @@ -418,7 +413,7 @@ function getFilesToLint(inputPaths: string[]): FileList { } } // If it's a file but it's not part of the content or the data - // directory, it's probably file passed in by computing changed files + // directory, it's probably a file passed in by computing changed files // from the git diff. } } @@ -436,7 +431,6 @@ function getFilesToLint(inputPaths: string[]): FileList { const relPath = path.relative(root, filePath) - // Skip files that match any of the excluded paths if (globalConfig.excludePaths.some((excludePath) => relPath.startsWith(excludePath))) { continue } @@ -452,7 +446,6 @@ function getFilesToLint(inputPaths: string[]): FileList { fileList.data = cleanPaths(fileList.data) fileList.yml = cleanPaths(fileList.yml) - // Add a total fileList length property fileList.length = fileList.content.length + fileList.data.length + fileList.yml.length return fileList @@ -504,11 +497,9 @@ function reportSummaryByRule(results: LintResults, config: LintConfig): void { } } -/* - Filter out the files with one or more results and format each - result. Results are sorted by severity per file, with errors - listed first then warnings. -*/ +// Filter out the files with one or more results and format each result. +// Results are sorted by severity per file, with errors listed first then +// warnings. function getFormattedResults( allResults: LintResults, isInPrecommitMode: boolean, @@ -527,7 +518,6 @@ function getFormattedResults( .map((flaw: LintError) => formatResult(flaw, isInPrecommitMode)) .filter((result): result is FormattedResult => result !== null) - // Only add the file to output if there are results after filtering if (formattedResults.length > 0) { const errors = formattedResults.filter((result) => result.severity === 'error') const warnings = formattedResults.filter((result) => result.severity === 'warning') @@ -539,17 +529,10 @@ function getFormattedResults( return output } -// Results are formatted with the key being the filepath -// and the value being an array of errors for that filepath. -// Each result has a rule name, which when looked up in `allConfig` -// will give us its severity and we filter those that are 'warning'. function getWarningCountByFile(results: FormattedResults, fixed = false): number { return getCountBySeverity(results, 'warning', fixed) } -// Results are formatted with the key being the filepath -// and the value being an array of results for that filepath. -// Each result in the array has a severity of error or warning. function getErrorCountByFile(results: FormattedResults, fixed = false): number { return getCountBySeverity(results, 'error', fixed) } @@ -574,7 +557,6 @@ function getCountBySeverity( function formatResult(object: LintError, isInPrecommitMode: boolean): FormattedResult | null { const formattedResult: FormattedResult = {} as FormattedResult - // Add severity to each result object const ruleName = object.ruleNames[1] || object.ruleNames[0] const ruleConfig = allConfig[ruleName] as Config | undefined // Skip rules that aren't in our config. This can happen when using @@ -632,13 +614,9 @@ function listRules() { return ruleList } -/* - Based on input options, configure the Markdownlint rules to run - There are a subset of rules that can't be run on data files, since - those Markdown files are partials included in full Markdown files. - Rules that can't be run on partials have the property - `partial-markdown-files` set to false. -*/ +// Some rules can't be run on data files, since those Markdown files are +// partials included in full Markdown files. Those rules have the property +// `partial-markdown-files` set to false. function getMarkdownLintConfig( filterErrorsOnly: boolean, runRules: string[] | undefined, @@ -670,7 +648,6 @@ function getMarkdownLintConfig( continue } - // Check if the rule should be included based on user-specified rules if (runRules && !shouldIncludeRule(ruleName, runRules)) continue // There are a subset of rules run on just the frontmatter in files @@ -678,9 +655,6 @@ function getMarkdownLintConfig( config.frontMatter[ruleName] = ruleConfig if (customRule) configuredRules.frontMatter.push(customRule) } - // Handle the special case of the search-replace rule - // which has nested rules each with their own - // severity and metadata. if (ruleName === 'search-replace') { const searchReplaceRules: NonNullable = [] const dataSearchReplaceRules: NonNullable = [] @@ -691,14 +665,12 @@ function getMarkdownLintConfig( for (const searchRule of ruleConfig.rules) { const searchRuleSeverity = getSeverity(searchRule, isPrecommit) if (filterErrorsOnly && searchRuleSeverity !== 'error') continue - // Add search-replace rules to frontmatter configuration for rules that make sense in frontmatter - // This ensures rules like TODOCS detection work in frontmatter - // Rules with applyToFrontmatter should ONLY run in the frontmatter pass (which lints the entire file) - // to avoid duplicate detections + // The frontmatter pass lints the whole file, so a rule with + // applyToFrontmatter must run there and nowhere else, or every match + // gets reported twice. if (searchRule.applyToFrontmatter) { frontmatterSearchReplaceRules.push(searchRule) } else { - // Only add to content rules if not a frontmatter-specific rule searchReplaceRules.push(searchRule) } if (searchRule['partial-markdown-files']) { @@ -765,7 +737,6 @@ function getCustomRule(ruleName: string): Rule | MarkdownlintRule { // Check if a rule should be included based on user-specified rules // Handles both short names (e.g., GHD047, MD001) and long names (e.g., table-column-integrity, heading-increment) export function shouldIncludeRule(ruleName: string, runRules: string[]) { - // First check if the rule name itself is in the list if (runRules.includes(ruleName)) { return true } @@ -789,7 +760,7 @@ export function shouldIncludeRule(ruleName: string, runRules: string[]) { The severity of the search-replace custom rule is embedded in each individual search rule. This function returns the severity of the individual search rule. The name we define for each search - rule shows up the the errorDetail property of the error object. + rule shows up in the errorDetail property of the error object. The error object returned from Markdownlint has the following structure: { @@ -830,11 +801,10 @@ function isOptionsValid() { } else { console.warn(`warning: the value '${filePath}' was not found. Skipping this path.`) } - // Continue processing - don't return false here + // Keep going: one bad path should not abandon the rest. } } - // Update the program options to only include valid paths if (optionPaths.length > 0) { program.setOptionValue('paths', validPaths) } diff --git a/src/content-linter/scripts/lint-report.ts b/src/content-linter/scripts/lint-report.ts index 7e39cc15fd9c..f77bde4d3df0 100644 --- a/src/content-linter/scripts/lint-report.ts +++ b/src/content-linter/scripts/lint-report.ts @@ -13,11 +13,8 @@ const MAX_ISSUE_BODY_SIZE = 60000 // If the number of warnings exceeds this number, print a warning so we can give them attention const MAX_WARNINGS_BEFORE_ALERT = 20 -/** - * Config that only applies to automated weekly reports. - */ +// Config that only applies to automated weekly reports. export const reportingConfig = { - // Include only rules with these severities in reports includeSeverities: ['error', 'warning'], // Include these rules regardless of severity in reports includeRules: ['expired-content'], @@ -29,13 +26,9 @@ interface LintFlaw { errorDetail?: string } -/** - * Determines if a lint result should be included in the automated report - */ function shouldIncludeInReport(flaw: LintFlaw): boolean { const allRuleNames = getAllRuleNames(flaw) - // Check if severity should be included if (reportingConfig.includeSeverities.includes(flaw.severity)) { return true } @@ -95,12 +88,10 @@ async function main() { // Keep track of warnings so we can print an alert when they exceed a manageable number let totalWarnings = 0 - // Filter results based on reporting configuration const filteredResults: Record = {} for (const [file, flaws] of Object.entries(parsedResults)) { const filteredFlaws = (flaws as LintFlaw[]).filter((flaw) => shouldIncludeInReport(flaw)) - // Only include files that have remaining flaws after filtering if (filteredFlaws.length > 0) { totalWarnings += filteredFlaws.filter((flaw) => flaw.severity === 'warning').length filteredResults[file] = filteredFlaws @@ -119,7 +110,6 @@ async function main() { for (const [file, flaws] of Object.entries(filteredResults)) { const fileEntry = `File: \`${file}\`:\n\`\`\`json\n${JSON.stringify(flaws, null, 2)}\n\`\`\`\n` - // Check if adding this file would exceed the size limit if (reportBody.length + fileEntry.length > MAX_ISSUE_BODY_SIZE) { truncated = true break @@ -129,7 +119,6 @@ async function main() { filesIncluded++ } - // Add truncation notice if needed if (truncated) { const remaining = totalFiles - filesIncluded reportBody += `\n---\n\nāš ļø **Output truncated**: Showing ${filesIncluded} of ${totalFiles} files with lint issues. ${remaining} additional files have been omitted to stay within GitHub's issue size limits.\n` diff --git a/src/content-linter/scripts/pretty-print-results.ts b/src/content-linter/scripts/pretty-print-results.ts index bd758d0d949f..1bc20181b91f 100644 --- a/src/content-linter/scripts/pretty-print-results.ts +++ b/src/content-linter/scripts/pretty-print-results.ts @@ -37,9 +37,9 @@ export function prettyPrintResults( for (const [file, flaws] of Object.entries(results)) { console.log(chalk.bold(file)) - console.log('') // blank line + console.log('') - // It's very possible that a the same file has multiple flaws of the + // It's very possible that the same file has multiple flaws of the // same rule but on different line numbers. const sorted = [...flaws] .sort((a, b) => a.lineNumber - b.lineNumber) diff --git a/src/content-linter/tests/category-pages.ts b/src/content-linter/tests/category-pages.ts index 2c31c6ef04ca..e12311db0328 100644 --- a/src/content-linter/tests/category-pages.ts +++ b/src/content-linter/tests/category-pages.ts @@ -66,13 +66,12 @@ describe.skip('category pages', () => { // Only include category directories, not standalone category files like content/actions/quickstart.md .filter((link) => fs.existsSync(getPath(productDir, link, 'index'))) - // Map those to the Markdown file paths that represent that category page index const categoryPaths = categoryLinks.map((link) => getPath(productDir, link, 'index')) // Make them relative for nicer display in test names const categoryRelativePaths = categoryPaths.map((p) => path.relative(contentDir, p)) - // Combine those to fit vitests's `.each` usage + // Combine those to fit vitest's `.each` usage const categoryTuples = zip(categoryRelativePaths, categoryPaths, categoryLinks) as [ string, string, @@ -121,7 +120,6 @@ describe.skip('category pages', () => { await contextualize(req as ExtendedRequest, res as Response, next) await shortVersions(req as ExtendedRequest, res as Response, next) - // Read the product index data for rendering const productIndexContents = await fs.promises.readFile(productIndex, 'utf8') const productIndexData = getFrontmatterData(productIndexContents) @@ -145,7 +143,7 @@ describe.skip('category pages', () => { const articleContents = await fs.promises.readFile(articlePath, 'utf8') const articleData = getFrontmatterData(articleContents) - // Do not include subcategories in list of published articles + // Do not include subcategories nor hidden pages in list of published articles if (articleData.subcategory || articleData.hidden) return null // ".../content/github/{category}/{article}.md" => "/{article}" @@ -154,7 +152,6 @@ describe.skip('category pages', () => { ) ).filter(Boolean) as string[] - // Get all of the child articles that exist in the subdir const childEntries = await fs.promises.readdir(categoryDir, { withFileTypes: true }) const childFileEntries = childEntries.filter( (ent) => ent.isFile() && ent.name !== 'index.md', @@ -212,7 +209,6 @@ describe.skip('category pages', () => { test('slugified title matches parent directory name', () => { if (allowTitleToDifferFromFilename) return - // Get the parent directory name const categoryDirPath = path.dirname(indexAbsPath) const categoryDirName = path.basename(categoryDirPath) @@ -230,7 +226,6 @@ describe.skip('category pages', () => { const expectedSlug = expectedSlugs.at(-1) as string const newCategoryDirPath = path.join(path.dirname(categoryDirPath), expectedSlug) customMessage += `\nTo resolve this consider running:\n ./src/content-render/scripts/move-content.ts ${categoryDirPath} ${newCategoryDirPath}\n` - // Check if the directory name matches the expected slug expect(expectedSlugs.includes(categoryDirName), customMessage).toBeTruthy() }) }, diff --git a/src/content-linter/tests/integration/lint-cli.ts b/src/content-linter/tests/integration/lint-cli.ts index c089f08df799..bbc7a701c9c5 100644 --- a/src/content-linter/tests/integration/lint-cli.ts +++ b/src/content-linter/tests/integration/lint-cli.ts @@ -1,24 +1,9 @@ -/** - * Integration tests for the content linter CLI script. - * - * These tests verify the actual end-to-end behavior of the lint-content script - * by running it via npm commands and checking the output. Unlike unit tests that - * test individual functions in isolation, these tests catch issues in the full - * CLI workflow including: - * - * - Command-line argument parsing - * - File discovery and processing logic - * - Rule configuration and filtering - * - Error reporting and exit codes - * - * Test file structure: - * - Test files are created in content/test-integration/ during tests - * - This ensures the linter actually processes them (it only processes files in content/ or data/) - * - Files are cleaned up after each test - * - * These tests serve as regression protection and verify that the linter - * continues to work as expected when changes are made to the CLI logic. - */ +// End-to-end tests for the lint-content script, run via npm and checked by +// their output. They cover argument parsing, file discovery, rule filtering, +// and exit codes. +// +// Test files are written to content/test-integration/ because the linter only +// processes files under content/ or data/. import { execSync } from 'child_process' import { beforeEach, afterEach, describe, test, expect } from 'vitest' @@ -29,18 +14,14 @@ const rootDir = path.join(__dirname, '../../../..') const testContentDir = path.join(rootDir, 'content/test-integration') describe('Content Linter CLI Integration Tests', { timeout: 30000 }, () => { - // Run all tests in sequence to avoid npm process conflicts beforeEach(async () => { - // Create test directory await fs.mkdir(testContentDir, { recursive: true }) }) afterEach(async () => { - // Clean up test files await fs.rm(testContentDir, { recursive: true, force: true }) }) - // Helper function to run linter commands async function runLinter(args: string): Promise<{ output: string; exitCode: number }> { let output = '' let exitCode = 0 @@ -50,7 +31,7 @@ describe('Content Linter CLI Integration Tests', { timeout: 30000 }, () => { encoding: 'utf8', cwd: rootDir, stdio: 'pipe', - timeout: 10000, // 10 second timeout + timeout: 10000, }) } catch (error: unknown) { const execError = error as { stdout?: string; stderr?: string; status?: number } @@ -63,7 +44,7 @@ describe('Content Linter CLI Integration Tests', { timeout: 30000 }, () => { describe('Linter functionality verification', () => { test('should detect errors when explicitly running search-replace rule', async () => { - // Baseline test - ensures the linter can detect errors + // Baseline: if this fails, the linter is not detecting anything at all. const testFile = path.join(testContentDir, 'baseline-test.md') const testContent = `--- title: Baseline Test @@ -110,10 +91,9 @@ TODOCS This is placeholder content that should now be detected by default. const { output, exitCode } = await runLinter(`--paths "${testFile}"`) - // Verify that the linter properly detects errors when no --rules are specified - expect(exitCode).toBe(1) // Should exit with error due to TODOCS - expect(output).toContain('todocs-placeholder') // TODOCS should be detected by default - expect(output).toContain('ERROR') // Errors should be detected + expect(exitCode).toBe(1) + expect(output).toContain('todocs-placeholder') + expect(output).toContain('ERROR') }) test('should respect rule filtering when specific rules are provided', async () => { @@ -137,7 +117,7 @@ TODOCS This file has multiple error types. `--paths "${testFile}" --rules heading-increment`, ) - expect(exitCode).toBe(0) // heading-increment rule behavior with filtering + expect(exitCode).toBe(0) expect(output).not.toContain('ERROR') expect(output).not.toContain('todocs-placeholder') }) diff --git a/src/content-linter/tests/lint-files.ts b/src/content-linter/tests/lint-files.ts index 1cd3f18a2a0a..dd6a6f916721 100755 --- a/src/content-linter/tests/lint-files.ts +++ b/src/content-linter/tests/lint-files.ts @@ -166,7 +166,6 @@ const yamlWalkOptions = { includeBasePath: true, } -// different lint rules apply to different content types let ymlToLint // compile lists of all the files we want to lint @@ -199,7 +198,6 @@ function formatLinkError(message: string, links: string[]) { // Returns `content` if its a string, or `content.description` if it can. // Used for getting the nested `description` key in glossary files. -// Using any because content can be string | { description: string } | other YAML structures function getContent(content: unknown) { if (typeof content === 'string') return content if ( @@ -214,7 +212,7 @@ function getContent(content: unknown) { const diffFiles = getDiffFiles() -// If present, and not empty, leverage it because in most cases it's empty. +// If it is present and not empty, use it. In most cases it is empty. if (diffFiles.length > 0) { // It's faster to do this once and then re-use over and over in the // .filter() later on. diff --git a/src/content-linter/tests/site-data-references.ts b/src/content-linter/tests/site-data-references.ts index c980541b8fc0..c0828043b187 100644 --- a/src/content-linter/tests/site-data-references.ts +++ b/src/content-linter/tests/site-data-references.ts @@ -33,7 +33,7 @@ describe('data references', () => { vi.setConfig({ testTimeout: 60 * 1000 }) test('every data reference found in English variable files is defined and has a value', async () => { - // value can be any type returned by getDataByLanguage - we check if it's a string + // getDataByLanguage can return any YAML type, so the string check happens below. let errors: Array<{ key: string; value: unknown; variableFile: string }> = [] const allVariables = getDeepDataByLanguage('variables', 'en') const variables = Object.values(allVariables) @@ -54,7 +54,7 @@ describe('data references', () => { }), ) - errors = uniqWith(errors, isEqual) // remove duplicates + errors = uniqWith(errors, isEqual) expect(errors.length, JSON.stringify(errors, null, 2)).toBe(0) }) }) diff --git a/src/content-linter/tests/unit/ctas-schema.ts b/src/content-linter/tests/unit/ctas-schema.ts index 0712e6541857..9fa8139057b7 100644 --- a/src/content-linter/tests/unit/ctas-schema.ts +++ b/src/content-linter/tests/unit/ctas-schema.ts @@ -82,11 +82,11 @@ try_ghec_for_free: '{% ifversion ghec %}https://github.com/account/enterprises/n expect(errors.length).toBe(1) // Should detect and try to convert the old CTA format expect(errors[0].fixInfo).toBeDefined() - // The extracted URL should not include the curly brace - verify by checking the fix + // The extracted URL should not include the curly brace from the Liquid tag. const fixedUrl = errors[0].fixInfo?.insertText expect(fixedUrl).toBeDefined() - expect(fixedUrl).not.toContain('{') // Should not include curly brace from Liquid syntax - expect(fixedUrl).not.toContain('}') // Should not include curly brace from Liquid syntax + expect(fixedUrl).not.toContain('{') + expect(fixedUrl).not.toContain('}') expect(fixedUrl).toContain('ref_product=ghec') // Should have converted old format correctly }) diff --git a/src/content-linter/tests/unit/frontmatter-content-type.ts b/src/content-linter/tests/unit/frontmatter-content-type.ts index 932e88902f6e..84e5a5ed83e3 100644 --- a/src/content-linter/tests/unit/frontmatter-content-type.ts +++ b/src/content-linter/tests/unit/frontmatter-content-type.ts @@ -38,9 +38,7 @@ describe('GHD065 - frontmatter-content-type', () => { resetCache() }) - // ------------------------------------------------------------------- // Passing cases - // ------------------------------------------------------------------- test('file with correct contentType matching directory passes', async () => { const strings = { @@ -101,7 +99,7 @@ describe('GHD065 - frontmatter-content-type', () => { test('file outside qualifying product is not checked', async () => { // actions in fixtures has non-EDI subdirs (category/, using-workflows/), - // so it does NOT qualify — the rule should skip it entirely. + // so it does NOT qualify and the rule should skip it entirely. const strings = { 'content/actions/category/test-file.md': md(['title: Test', 'versions:', " fpt: '*'"]), } @@ -119,9 +117,7 @@ describe('GHD065 - frontmatter-content-type', () => { expect(errors).toEqual([]) }) - // ------------------------------------------------------------------- // Failing cases - // ------------------------------------------------------------------- test('missing contentType in qualifying product triggers error', async () => { const strings = { diff --git a/src/content-linter/tests/unit/frontmatter-landing-carousels.ts b/src/content-linter/tests/unit/frontmatter-landing-carousels.ts index 3238d9555f7e..2aaeeefd3f63 100644 --- a/src/content-linter/tests/unit/frontmatter-landing-carousels.ts +++ b/src/content-linter/tests/unit/frontmatter-landing-carousels.ts @@ -92,14 +92,9 @@ describe(ruleName, () => { }) test('absolute paths are prioritized over relative paths', async () => { - // This test verifies that when both absolute and relative paths exist with the same name, - // the absolute path is chosen over the relative path. - // - // Setup: - // - /article-one should resolve to src/fixtures/fixtures/content/article-one.md (absolute) - // - article-one (relative) would resolve to src/content-linter/tests/fixtures/landing-carousels/article-one.md - // - // The test passes because our logic prioritizes the absolute path resolution first + // /article-one exists both as src/fixtures/fixtures/content/article-one.md + // and as src/content-linter/tests/fixtures/landing-carousels/article-one.md. + // The absolute resolution wins. const result = await runRule(frontmatterLandingCarousels, { files: [ABSOLUTE_PRIORITY], ...fmOptions, @@ -108,16 +103,7 @@ describe(ruleName, () => { }) test('path priority resolution works correctly', async () => { - // This test verifies that absolute paths are prioritized over relative paths - // when both files exist with the same name. - // - // Setup: - // - /article-one could resolve to EITHER: - // 1. src/fixtures/fixtures/content/article-one.md (absolute - should be chosen) - // 2. src/content-linter/tests/fixtures/landing-carousels/article-one.md (relative - should be ignored) - // - // Our prioritization logic should choose #1 (absolute) over #2 (relative) - // This test passes because the absolute path exists and is found first + // Same collision as the test above, reached through a different fixture. const result = await runRule(frontmatterLandingCarousels, { files: [PATH_PRIORITY], ...fmOptions, @@ -126,10 +112,7 @@ describe(ruleName, () => { }) test('absolute-only paths work when no relative path exists', async () => { - // This test verifies that absolute path resolution works when no relative path exists - // /article-two exists in src/fixtures/fixtures/content/article-two.md - // but NOT in src/content-linter/tests/fixtures/landing-carousels/article-two.md - // This test would fail if we didn't prioritize absolute paths properly + // /article-two resolves from src/fixtures/fixtures/content/article-two.md. const result = await runRule(frontmatterLandingCarousels, { files: [ABSOLUTE_ONLY], ...fmOptions, diff --git a/src/content-linter/tests/unit/frontmatter-schema.ts b/src/content-linter/tests/unit/frontmatter-schema.ts index a4ca19a5c20b..8fa299079eb2 100644 --- a/src/content-linter/tests/unit/frontmatter-schema.ts +++ b/src/content-linter/tests/unit/frontmatter-schema.ts @@ -3,7 +3,7 @@ import { describe, expect, test } from 'vitest' import { runRule } from '../../lib/init-test' import { frontmatterSchema } from '../../lib/linting-rules/frontmatter-schema' -// Configure the test figure to not split frontmatter and content +// Configure the test fixture to not split frontmatter and content const fmOptions = { markdownlintOptions: { frontMatter: null } } describe(frontmatterSchema.names.join(' - '), () => { diff --git a/src/content-linter/tests/unit/frontmatter-search-replace.ts b/src/content-linter/tests/unit/frontmatter-search-replace.ts index 152e61e67290..788f41102dbf 100644 --- a/src/content-linter/tests/unit/frontmatter-search-replace.ts +++ b/src/content-linter/tests/unit/frontmatter-search-replace.ts @@ -19,7 +19,6 @@ describe('search-replace rule in frontmatter', () => { const errors = result.markdown || [] - // Should find TODOCS in frontmatter const todosErrors = errors.filter((e) => e.errorDetail && /TODOCS/.test(e.errorDetail)) expect(todosErrors.length).toBe(1) expect(todosErrors[0].lineNumber).toBe(2) // title: TODOCS @@ -47,7 +46,6 @@ describe('search-replace rule in frontmatter', () => { const errors = result.markdown || [] - // Should find all TODOCS instances in frontmatter const todosErrors = errors.filter((e) => e.errorDetail && /TODOCS/.test(e.errorDetail)) expect(todosErrors.length).toBe(3) expect(todosErrors[0].lineNumber).toBe(2) // title: TODOCS @@ -77,7 +75,6 @@ describe('search-replace rule in frontmatter', () => { const errors = result.markdown || [] - // Should find domain errors in frontmatter const domainErrors = errors.filter( (e) => e.errorDetail && /docs-domain|help-domain|developer-domain/.test(e.errorDetail), ) @@ -108,7 +105,6 @@ describe('search-replace rule in frontmatter', () => { const errors = result.markdown || [] - // Should find deprecated syntax errors in frontmatter const deprecatedErrors = errors.filter( (e) => e.errorDetail && /site\.data|octicon/.test(e.errorDetail), ) diff --git a/src/content-linter/tests/unit/frontmatter-versions-whitespace.ts b/src/content-linter/tests/unit/frontmatter-versions-whitespace.ts index 99ca3a013552..d81bf4350f8d 100644 --- a/src/content-linter/tests/unit/frontmatter-versions-whitespace.ts +++ b/src/content-linter/tests/unit/frontmatter-versions-whitespace.ts @@ -18,7 +18,6 @@ interface InvalidTestCase { expectedMessage?: string } -// Valid cases - should pass const validCases: ValidTestCase[] = [ { name: 'valid-simple-versions', @@ -63,7 +62,6 @@ This is a test. }, ] -// Invalid cases - should fail const invalidCases: InvalidTestCase[] = [ { name: 'trailing-whitespace', diff --git a/src/content-linter/tests/unit/image-alt-text-end-punctuation.ts b/src/content-linter/tests/unit/image-alt-text-end-punctuation.ts index c4e05da23279..956692c95240 100644 --- a/src/content-linter/tests/unit/image-alt-text-end-punctuation.ts +++ b/src/content-linter/tests/unit/image-alt-text-end-punctuation.ts @@ -59,9 +59,8 @@ describe(imageAltTextEndPunctuation.names.join(' - '), () => { ].join('\n') const result = await runRule(imageAltTextEndPunctuation, { strings: { markdown } }) const errors = result.markdown - // This rule is not concerned with empty alt text - // That will be caught by the incorrect-alt-text-length rule - // So technically, it's not imageAltTextEndPunctuation's problem. + // This rule is not concerned with empty alt text. The + // incorrect-alt-text-length rule catches that instead. expect(errors.length).toBe(0) }) }) diff --git a/src/content-linter/tests/unit/image-alt-text-exclude-start-words.ts b/src/content-linter/tests/unit/image-alt-text-exclude-start-words.ts index 7b2bdac0f7b1..885c7f470e80 100644 --- a/src/content-linter/tests/unit/image-alt-text-exclude-start-words.ts +++ b/src/content-linter/tests/unit/image-alt-text-exclude-start-words.ts @@ -39,9 +39,8 @@ describe(imageAltTextExcludeStartWords.names.join(' - '), () => { ].join('\n') const result = await runRule(imageAltTextExcludeStartWords, { strings: { markdown } }) const errors = result.markdown - // This rule is not concerned with empty alt text - // That will be caught by the incorrect-alt-text-empty rule - // So technically, it's not imageAltTextEndPunctuation's problem. + // This rule is not concerned with empty alt text. The + // incorrect-alt-text-length rule catches that instead. expect(errors.length).toBe(0) }) }) diff --git a/src/content-linter/tests/unit/internal-links-no-lang.ts b/src/content-linter/tests/unit/internal-links-no-lang.ts index e883adf5a55c..66d8eaf970bb 100644 --- a/src/content-linter/tests/unit/internal-links-no-lang.ts +++ b/src/content-linter/tests/unit/internal-links-no-lang.ts @@ -29,7 +29,7 @@ describe(internalLinksNoLang.names.join(' - '), () => { // a // means the link is external 'These are the [Docs](//ja/actions) we need.', 'This is the [actions Docs](/actions)', - // A link that starts with a language code + // Starts with a path segment that is not a language code '[Enterprise](/enterprise/overview)', ].join('\n') const result = await runRule(internalLinksNoLang as Rule, { strings: { markdown } }) diff --git a/src/content-linter/tests/unit/internal-links-old-version.ts b/src/content-linter/tests/unit/internal-links-old-version.ts index 84c0510dd77e..39e4e4590de6 100644 --- a/src/content-linter/tests/unit/internal-links-old-version.ts +++ b/src/content-linter/tests/unit/internal-links-old-version.ts @@ -24,7 +24,7 @@ describe(internalLinksOldVersion.names.join(' - '), () => { const markdown = [ // External links with enterprise in them '[External link](https://someservice.com/enterprise/1.0/admin/yes)', - // Current versioning links is excluded from this test + // Current versioning links are excluded from this test '[New versioning](/github/site-policy/enterprise/2.2/yes)', ].join('\n') const result = await runRule(internalLinksOldVersion as Rule, { strings: { markdown } }) diff --git a/src/content-linter/tests/unit/lint-report-exclusions.ts b/src/content-linter/tests/unit/lint-report-exclusions.ts index 7879f14a3987..e9fe1bd8c6d3 100644 --- a/src/content-linter/tests/unit/lint-report-exclusions.ts +++ b/src/content-linter/tests/unit/lint-report-exclusions.ts @@ -51,7 +51,6 @@ describe('content linter configuration', () => { function shouldIncludeInReport(flaw: LintFlaw): boolean { const allRuleNames = getAllRuleNames(flaw) - // Check if severity should be included if (reportingConfig.includeSeverities.includes(flaw.severity)) { return true } diff --git a/src/content-linter/tests/unit/liquid-ifversion-versions.ts b/src/content-linter/tests/unit/liquid-ifversion-versions.ts index 9b602487cda2..7a3c831d3b8d 100644 --- a/src/content-linter/tests/unit/liquid-ifversion-versions.ts +++ b/src/content-linter/tests/unit/liquid-ifversion-versions.ts @@ -114,8 +114,7 @@ describe(liquidIfversionVersions.names.join(' - '), () => { }) test.skip('ifversion using feature based version extended with shortname all versions', async () => { - // That `features/volvo.yml` contains `fpt:'*', ghec:'*'` - // so combined with the + // That `features/volvo.yml` contains `fpt:'*', ghec:'*'`. const markdown = ` {% ifversion volvo or ghes %}{% endif %} ` diff --git a/src/content-linter/tests/unit/liquid-syntax.ts b/src/content-linter/tests/unit/liquid-syntax.ts index 2cc769ce9bbd..a503b869def0 100644 --- a/src/content-linter/tests/unit/liquid-syntax.ts +++ b/src/content-linter/tests/unit/liquid-syntax.ts @@ -3,7 +3,7 @@ import { describe, expect, test } from 'vitest' import { runRule } from '../../lib/init-test' import { frontmatterLiquidSyntax, liquidSyntax } from '../../lib/linting-rules/liquid-syntax' -// Configure the test figure to not split frontmatter and content +// Configure the test fixture to not split frontmatter and content const fmOptions = { markdownlintOptions: { frontMatter: null } } describe(frontmatterLiquidSyntax.names.join(' - '), () => { diff --git a/src/content-linter/tests/unit/liquid-versioning.ts b/src/content-linter/tests/unit/liquid-versioning.ts index 2711ce83ee9e..0d142a66b43d 100644 --- a/src/content-linter/tests/unit/liquid-versioning.ts +++ b/src/content-linter/tests/unit/liquid-versioning.ts @@ -58,7 +58,7 @@ describe(liquidIfVersionTags.names.join(' - '), () => { '{% ifversion ghes < 2.9 %}', // Incorrect syntax '{% ifversion ghec or ifversion fpt %}', - // Typo - should be not ghec + // Typo: should be `not ghec` '{% ifversion no ghec %}', ] const result = await runRule(liquidIfVersionTags, { diff --git a/src/content-linter/tests/unit/rai-app-card-structure.ts b/src/content-linter/tests/unit/rai-app-card-structure.ts index 82394e1ad94c..ef1f838dacc1 100644 --- a/src/content-linter/tests/unit/rai-app-card-structure.ts +++ b/src/content-linter/tests/unit/rai-app-card-structure.ts @@ -3,11 +3,7 @@ import { describe, expect, test } from 'vitest' import { runRule } from '../../lib/init-test' import { raiAppCardStructure } from '../../lib/linting-rules/rai-app-card-structure' -// --------------------------------------------------------------------------- -// Helpers -// --------------------------------------------------------------------------- - -/** A minimal valid RAI card with all required H2s, H3s, and reusables. */ +// A minimal valid RAI card with all required H2s, H3s, and reusables. function validCard(): string { return [ '---', @@ -102,9 +98,7 @@ function validCard(): string { } describe(raiAppCardStructure.names.join(' - '), () => { - // ----------------------------------------------------------------------- - // Happy path & filtering - // ----------------------------------------------------------------------- + // Happy path and filtering test('valid RAI card produces zero errors', async () => { const markdown = validCard() @@ -128,9 +122,7 @@ describe(raiAppCardStructure.names.join(' - '), () => { expect(errors.length).toBe(0) }) - // ----------------------------------------------------------------------- - // One negative test per validator — proves each code path fires - // ----------------------------------------------------------------------- + // One negative test per validator, to prove each code path fires test('missing a required H2 section reports an error', async () => { const markdown = validCard() diff --git a/src/content-linter/tests/unit/rule-filtering.ts b/src/content-linter/tests/unit/rule-filtering.ts index 7488c5f94b76..0da64c2b514a 100644 --- a/src/content-linter/tests/unit/rule-filtering.ts +++ b/src/content-linter/tests/unit/rule-filtering.ts @@ -1,7 +1,6 @@ import { describe, test, expect, vi } from 'vitest' import { shouldIncludeRule } from '../../scripts/lint-content' -// Mock the get-rules module to provide test data for rule definitions vi.mock('../../lib/helpers/get-rules', () => ({ allRules: [ { diff --git a/src/content-linter/tests/unit/search-replace.ts b/src/content-linter/tests/unit/search-replace.ts index a498e4b98bbb..2cc4cd9f17d5 100644 --- a/src/content-linter/tests/unit/search-replace.ts +++ b/src/content-linter/tests/unit/search-replace.ts @@ -79,7 +79,6 @@ describe(searchReplace.names.join(' - '), () => { markdownlintOptions: { frontMatter: null }, // Include frontmatter in linting }) const errors = result.markdown - // Should find 3 TODOCS occurrences in frontmatter expect(errors.length).toBe(3) expect(errors[0].lineNumber).toBe(2) // title: TODOCS expect(errors[1].lineNumber).toBe(3) // shortTitle: TODOCS @@ -102,7 +101,6 @@ describe(searchReplace.names.join(' - '), () => { markdownlintOptions: { frontMatter: null }, // Include frontmatter in linting }) const errors = result.markdown - // Should find 4 TODOCS occurrences total (2 in frontmatter + 2 in content) expect(errors.length).toBe(4) expect(errors[0].lineNumber).toBe(2) // title: TODOCS expect(errors[1].lineNumber).toBe(3) // intro: TODOCS diff --git a/src/content-render/liquid/data.ts b/src/content-render/liquid/data.ts index bafe90d364a0..44c0151cd40b 100644 --- a/src/content-render/liquid/data.ts +++ b/src/content-render/liquid/data.ts @@ -91,7 +91,6 @@ function handleIndent(tagToken: TagToken, text: string): string { // keep the blockquote character on every successive line. const blockquoteRegexp = /^\n?([ \t]*>[ \t]?)/ function handleBlockquote(tagToken: TagToken, text: string): string { - // If the text isn't multiline, skip if (text.split('\n').length <= 1) return text // If the line with the liquid tag starts with a blockquote... @@ -100,7 +99,6 @@ function handleBlockquote(tagToken: TagToken, text: string): string { const inputLine = input.split('\n').find((line) => line.includes(content)) if (!inputLine || !blockquoteRegexp.test(inputLine)) return text - // Keep the character on successive lines const match = inputLine.match(blockquoteRegexp) if (!match) return text const start = match[0] diff --git a/src/content-render/liquid/engine.ts b/src/content-render/liquid/engine.ts index 2d10036d94f8..11418f9cfd9a 100644 --- a/src/content-render/liquid/engine.ts +++ b/src/content-render/liquid/engine.ts @@ -77,9 +77,6 @@ engine.registerFilter('render_liquid', function (this: FilterScope, input: unkno return engine.parseAndRender(input, this.context.environments) }) -/** - * Convert the input to a slug - */ engine.registerFilter('slugify', (input: string): string => { const slugger = new GithubSlugger() return slugger.slug(input) diff --git a/src/content-render/liquid/ifversion.ts b/src/content-render/liquid/ifversion.ts index adc1b16d5ccb..38c724517a18 100644 --- a/src/content-render/liquid/ifversion.ts +++ b/src/content-render/liquid/ifversion.ts @@ -95,21 +95,19 @@ export default class Ifversion extends Tag { for (const branch of this.branches) { let resolvedBranchCond = branch.cond - // Resolve "not" keywords in the conditional, if any. resolvedBranchCond = this.handleNots(resolvedBranchCond) // Resolve special operators in the conditional, if any. // This will replace syntax like `fpt or ghes < 3.0` with `fpt or true` or `fpt or false`. resolvedBranchCond = this.handleOperators(resolvedBranchCond) - // Resolve version names to boolean values for Markdown API context. - // This will replace syntax like `fpt or ghec` with `true or false` based on current version. - // Only apply this transformation in Markdown API context to avoid breaking existing functionality. + // Replace syntax like `fpt or ghec` with `true or false` based on the current + // version. Only done for the Markdown API, where the version names would + // otherwise be undefined. if ((ctx.environments as IfversionEnvironments).markdownRequested) { resolvedBranchCond = this.handleVersionNames(resolvedBranchCond) } - // Use Liquid's native function for the final evaluation. const cond = yield new Value(resolvedBranchCond, this.liquid).value(ctx, ctx.opts.lenientIf) if (isTruthy(cond, ctx)) { @@ -125,7 +123,6 @@ export default class Ifversion extends Tag { const condArray = resolvedBranchCond.split(' ') - // Find the first index in the array that contains "not". const notIndex = condArray.findIndex((el: string) => el === 'not') // E.g., ['not', 'fpt'] @@ -156,7 +153,6 @@ export default class Ifversion extends Tag { // If this conditional contains multiple parts using `or` or `and`, get only the conditional with operators. const condArray = resolvedBranchCond.split(' ') - // Find the first index in the array that contains an operator. const operatorIndex = condArray.findIndex((el: string) => supportedOperators.find((op: string) => el === op), ) @@ -164,7 +160,6 @@ export default class Ifversion extends Tag { // E.g., ['ghes', '<', '3.1'] const condParts = condArray.slice(operatorIndex - 1, operatorIndex + 2) - // Assign to vars. const [versionShortName, operator, releaseToEvaluate] = condParts // Make sure the operator is supported and the release number matches `\d\d?\.\d\d?` @@ -219,17 +214,12 @@ export default class Ifversion extends Tag { return resolvedBranchCond } - // Split the condition into tokens for processing const tokens = resolvedBranchCond.split(/\s+/) const processedTokens = tokens.map((token: string) => { - // Check if the token is a version short name (fpt, ghec, ghes, ghae) const versionShortNames = ['fpt', 'ghec', 'ghes', 'ghae'] if (versionShortNames.includes(token)) { - // Transform version names to boolean values for Markdown API - // This fixes the original issue where version names were undefined in API context return token === this.currentVersionObj!.shortName ? 'true' : 'false' } - // Return the token unchanged if it's not a version name return token }) diff --git a/src/content-render/liquid/indented-data-reference.ts b/src/content-render/liquid/indented-data-reference.ts index 63ad23e1c942..b5b7f589d73b 100644 --- a/src/content-render/liquid/indented-data-reference.ts +++ b/src/content-render/liquid/indented-data-reference.ts @@ -47,7 +47,6 @@ const IndentedDataReference = { assert(parseInt(numSpaces) || numSpaces === '0', '"spaces=NUMBER" must include a number') - // Get the referenced value from the context const text: string | undefined = getDataByLanguage( dataReference, scope.environments.currentLanguage, @@ -63,7 +62,6 @@ const IndentedDataReference = { return } - // add spaces to each line const renderedReferenceWithIndent: string = text.replace(/^/gm, ' '.repeat(parseInt(numSpaces))) return this.liquid.parseAndRender(renderedReferenceWithIndent, scope.environments) diff --git a/src/content-render/liquid/octicon.ts b/src/content-render/liquid/octicon.ts index ca5a40fa7379..aacb8927e791 100644 --- a/src/content-render/liquid/octicon.ts +++ b/src/content-render/liquid/octicon.ts @@ -25,7 +25,6 @@ const Octicon = { throw new TokenizationError(SyntaxHelp, tagToken) } - // Memoize the icon this.icon = match.groups.icon // Breaking change in octicons 12 // https://github.com/primer/octicons/releases/tag/v12.0.0 @@ -36,29 +35,24 @@ const Octicon = { this.options = {} - // Memoize any options passed if (match.groups.options) { let optionsMatch: RegExpExecArray | null - // Loop through each option matching the OptionsSyntax regex while ((optionsMatch = OptionsSyntax.exec(match.groups.options))) { // Pull out the key/value ([0] is the whole input) const [, key, value] = optionsMatch this.options[key] = value - // Alias label to aria-label if (key === 'label') this.options['aria-label'] = value } } }, async render(): Promise { - // Throw an error if the requested octicon does not exist. if (!Object.prototype.hasOwnProperty.call(octicons, this.icon)) { throw new Error(`Octicon ${this.icon} does not exist`) } - // Auto-generate aria-label if not provided // Replace non-alphanumeric characters with spaces and append " icon" if (!this.options['aria-label']) { const defaultLabel = `${this.icon.toLowerCase().replace(/[^a-z0-9]+/gi, ' ')} icon` diff --git a/src/content-render/liquid/prompt.ts b/src/content-render/liquid/prompt.ts index dbba2b28b3c4..1df9e1bcd28a 100644 --- a/src/content-render/liquid/prompt.ts +++ b/src/content-render/liquid/prompt.ts @@ -1,4 +1,3 @@ -// src/content-render/liquid/prompt.ts // Defines {% prompt %}…{% endprompt %} to wrap its content in and append the Copilot icon. import octicons from '@primer/octicons' @@ -15,7 +14,6 @@ interface LiquidTag { export const Prompt: LiquidTag = { type: 'block', - // Collect everything until {% endprompt %} parse(tagToken: TagToken, remainTokens: TopLevelToken[]): void { this.templates = [] const stream = this.liquid.parser.parseStream(remainTokens) @@ -28,12 +26,10 @@ export const Prompt: LiquidTag = { stream.start() }, - // Render the inner Markdown, wrap in , then append the SVG *render(scope: unknown): Generator { const content = yield this.liquid.renderer.renderTemplates(this.templates, scope) const contentString = String(content) - // build a URL with the prompt text encoded as query parameter const promptParam: string = encodeURIComponent(contentString) const href: string = `https://github.com/copilot?prompt=${promptParam}` // Use murmur hash for deterministic ID (avoids hydration mismatch) diff --git a/src/content-render/scripts/add-content-type.ts b/src/content-render/scripts/add-content-type.ts index 588b484bdaf0..f6286ea7b4a2 100644 --- a/src/content-render/scripts/add-content-type.ts +++ b/src/content-render/scripts/add-content-type.ts @@ -107,7 +107,6 @@ function processFile(filePath: string, scriptOptions: ScriptOptions) { return { processed: true, updated: false } } - // Check if we're actually changing an existing contentType const isChangingContentType = data.contentType && data.contentType !== newContentType const isAddingContentType = !data.contentType @@ -119,7 +118,6 @@ function processFile(filePath: string, scriptOptions: ScriptOptions) { console.log(`Adding contentType '${newContentType}' on ${relativePath}`) } - // Only update if there's actually a change needed if (isChangingContentType || isAddingContentType) { data.contentType = newContentType } else { @@ -127,7 +125,6 @@ function processFile(filePath: string, scriptOptions: ScriptOptions) { return { processed: true, updated: false } } - // Write the file back fs.writeFileSync( filePath, frontmatter.stringify( @@ -183,7 +180,6 @@ function determineContentType(relativePath: string): string { return LANDING_TYPE } - // Classify anything else as 'other'. return OTHER_TYPE } diff --git a/src/content-render/scripts/cta-builder.ts b/src/content-render/scripts/cta-builder.ts index bd633c63dbb6..96ca90b65f00 100644 --- a/src/content-render/scripts/cta-builder.ts +++ b/src/content-render/scripts/cta-builder.ts @@ -31,7 +31,6 @@ type CTASchemaProperties = { [K in keyof CTAParams]-?: CTASchemaProperty } -// Conversion mappings from old CTA format to new schema const ctaToTypeMapping: Record = { 'GHEC trial': 'trial', 'Copilot trial': 'trial', @@ -51,18 +50,15 @@ const ctaToPlanMapping: Record = { 'GHEC trial': 'enterprise', } -// Keywords that suggest a button context vs inline text link const buttonKeywords = ['landing', 'signup', 'download', 'trial'] const program = new Command() -// CLI setup program .name('cta-builder') .description('Create a properly formatted Call-to-Action URL with tracking parameters.') .version('1.0.0') -// Add conversion command program .command('convert') .description('Convert old CTA URLs to new schema format') @@ -72,7 +68,6 @@ program convertUrls(options) }) -// Add validation command program .command('validate') .description('Validate a CTA URL against the schema') @@ -81,7 +76,6 @@ program validateUrl(options) }) -// Add programmatic build command program .command('build') .description('Build a CTA URL programmatically with flags (outputs URL only)') @@ -94,7 +88,6 @@ program buildProgrammaticCTA(options) }) -// Default to interactive mode program.action(() => { interactiveBuilder() }) @@ -104,7 +97,6 @@ if (import.meta.url === `file://${process.argv[1]}`) { program.parse() } -// Helper function to select from lettered options async function selectFromOptions( paramName: string, message: string, @@ -139,7 +131,6 @@ async function selectFromOptions( } } -// Helper function to confirm yes/no async function confirmChoice( message: string, promptFn: (question: string) => Promise, @@ -161,7 +152,6 @@ async function confirmChoice( } } -// Extract CTA parameters from a URL function extractCTAParams(url: string): CTAParams { const urlObj = new URL(url) const ctaParams: CTAParams = {} @@ -224,7 +214,6 @@ function validateCTAParams(params: CTAParams): { isValid: boolean; errors: strin } } -// Build URL with CTA parameters function buildCTAUrl(baseUrl: string, params: CTAParams): string { const url = new URL(baseUrl) @@ -237,17 +226,15 @@ function buildCTAUrl(baseUrl: string, params: CTAParams): string { return url.toString() } -// Convert old CTA URL to new schema format export function convertOldCTAUrl(oldUrl: string): { newUrl: string; notes: string[] } { const notes: string[] = [] try { const url = new URL(oldUrl) - // Build new parameters const newParams: CTAParams = {} - // First, check if any of the new params already exist, and preserve those if so + // Preserve any new-style params that are already on the URL. for (const [key, value] of url.searchParams.entries()) { for (const param of Object.keys(ctaSchema.properties)) { if (key === param && key in ctaSchema.properties) { @@ -264,17 +251,14 @@ export function convertOldCTAUrl(oldUrl: string): { newUrl: string; notes: strin } } - // Try to convert old params to new params const refCta = url.searchParams.get('ref_cta') || '' const refLoc = url.searchParams.get('ref_loc') || '' - // Map ref_product if (!newParams.ref_product) { newParams.ref_product = inferProductFromUrl(oldUrl, refCta) notes.push(`- Missing ref_product - made an inference, manually update if needed`) } - // Map ref_type if (!newParams.ref_type) { newParams.ref_type = ctaToTypeMapping[refCta] || 'engagement' if (!ctaToTypeMapping[refCta]) { @@ -282,13 +266,11 @@ export function convertOldCTAUrl(oldUrl: string): { newUrl: string; notes: strin } } - // Map ref_style if (!newParams.ref_style) { newParams.ref_style = inferStyleFromContext(refLoc) notes.push(`- Missing ref_style - made an inference, manually update if needed`) } - // Map ref_plan (optional) if (!newParams.ref_plan) { if (ctaToPlanMapping[refCta]) { newParams.ref_plan = ctaToPlanMapping[refCta] @@ -298,12 +280,10 @@ export function convertOldCTAUrl(oldUrl: string): { newUrl: string; notes: strin // Build new URL - preserve all existing parameters except old ref_ parameters const newUrl = new URL(url.toString()) - // Remove old CTA parameters newUrl.searchParams.delete('ref_cta') newUrl.searchParams.delete('ref_loc') newUrl.searchParams.delete('ref_page') - // Add new CTA parameters for (const [key, value] of Object.entries(newParams)) { if (value) { newUrl.searchParams.set(key, value) @@ -343,7 +323,6 @@ function inferProductFromUrl(url: string, refCta: string): string { } catch { // Fallback if url isn't valid: leave hostname empty } - // Strict hostname check for desktop.github.com if (hostname === 'desktop.github.com' || refCta.includes('desktop')) { return 'desktop' } @@ -361,7 +340,6 @@ function inferProductFromUrl(url: string, refCta: string): string { ) { return 'ghec' } - // Default fallback return 'copilot' } @@ -372,15 +350,12 @@ function inferStyleFromContext(refLoc: string): string { return isButton ? 'button' : 'text' } -// Interactive CTA builder async function interactiveBuilder(): Promise { - // Create readline interface for interactive mode const rl = readline.createInterface({ input: process.stdin, output: process.stdout, }) - // Helper function to prompt user (scoped to this function) function prompt(question: string): Promise { return new Promise((resolve) => { rl.question(question, (answer) => { @@ -392,7 +367,6 @@ async function interactiveBuilder(): Promise { try { console.log(chalk.blue.bold('šŸš€ Guided CTA URL builder\n')) - // Get base URL with validation let baseUrl = '' while (!baseUrl) { const input = await prompt('Enter the base URL (e.g., https://github.com/features/copilot): ') @@ -406,7 +380,6 @@ async function interactiveBuilder(): Promise { const params: CTAParams = {} - // Required parameters console.log(chalk.white(`\nRequired parameters:`)) const schemaProps = ctaSchema.properties as CTASchemaProperties @@ -442,7 +415,6 @@ async function interactiveBuilder(): Promise { } } - // Validate parameters const validation = validateCTAParams(params) if (!validation.isValid) { @@ -454,7 +426,6 @@ async function interactiveBuilder(): Promise { return } - // Build and display URL const ctaUrl = buildCTAUrl(baseUrl, params) console.log(chalk.green('\nāœ… CTA URL generated successfully!')) @@ -477,7 +448,6 @@ async function interactiveBuilder(): Promise { } } -// Convert URLs command handler async function convertUrls(options: { url?: string; quiet?: boolean }): Promise { try { if (!options.quiet) { @@ -540,7 +510,6 @@ async function convertUrls(options: { url?: string; quiet?: boolean }): Promise< // The convert command doesn't use readline, so script should exit naturally } -// Validate URLs command handler async function validateUrl(options: { url?: string }): Promise { try { console.log(chalk.blue.bold('CTA URL validator')) @@ -549,7 +518,6 @@ async function validateUrl(options: { url?: string }): Promise { console.log(chalk.white('\nValidating URL:')) console.log(chalk.gray(options.url)) - // Extract CTA parameters from URL let ctaParams: CTAParams try { ctaParams = extractCTAParams(options.url) @@ -558,7 +526,6 @@ async function validateUrl(options: { url?: string }): Promise { return } - // Check if URL has any CTA parameters if (Object.keys(ctaParams).length === 0) { console.log(chalk.yellow('\nā„¹ļø No CTA parameters found in URL')) return @@ -598,7 +565,6 @@ async function validateUrl(options: { url?: string }): Promise { } } -// Programmatic build command handler async function buildProgrammaticCTA(options: { url: string product: string @@ -607,7 +573,6 @@ async function buildProgrammaticCTA(options: { plan?: string }): Promise { try { - // Validate base URL let baseUrl: string try { baseUrl = new URL(options.url).toString() @@ -618,19 +583,16 @@ async function buildProgrammaticCTA(options: { process.exit(1) } - // Build CTA parameters object const params: CTAParams = { ref_product: options.product, ref_type: options.type, ref_style: options.style, } - // Add optional parameters if (options.plan) { params.ref_plan = options.plan } - // Validate parameters against schema const validation = validateCTAParams(params) if (!validation.isValid) { // Output validation errors to stderr and exit with error code @@ -640,7 +602,6 @@ async function buildProgrammaticCTA(options: { process.exit(1) } - // Build and output the URL (stdout only) const ctaUrl = buildCTAUrl(baseUrl, params) console.log(ctaUrl) } catch (error) { diff --git a/src/content-render/scripts/liquid-tags.ts b/src/content-render/scripts/liquid-tags.ts index e15a3e3ab6f4..e24fdf6cfc73 100644 --- a/src/content-render/scripts/liquid-tags.ts +++ b/src/content-render/scripts/liquid-tags.ts @@ -11,7 +11,6 @@ import path from 'path' import { load, dump } from 'js-yaml' import chalk from 'chalk' -// Type definitions interface ExpandOptions { paths: string[] verbose?: boolean @@ -30,7 +29,6 @@ interface LiquidReference { endIndex: number } -// Constants const ROOT = process.env.ROOT || '.' const DATA_ROOT = path.resolve(path.join(ROOT, 'data')) const REUSABLES_ROOT = path.join(DATA_ROOT, 'reusables') @@ -43,14 +41,9 @@ function getErrorMessage(error: unknown): string { // Regex pattern to match expanded content blocks const EXPANDED_PATTERN = /(.+?)/gs -/** - * Get the file path for a data reference - * - * Validates and normalizes the incoming dataPath to prevent path traversal - * and ensure the final resolved path remains within the expected root. - */ +// Validates and normalizes the incoming dataPath to prevent path traversal +// and ensure the final resolved path remains within the expected root. function getDataFilePath(type: 'reusable' | 'variable', dataPath: string): string { - // Basic validation of the raw dataPath if (path.isAbsolute(dataPath)) { throw new Error(`Invalid ${type} data path: absolute paths are not allowed: ${dataPath}`) } @@ -89,9 +82,6 @@ function getDataFilePath(type: 'reusable' | 'variable', dataPath: string): strin } } -/** - * Convert a file path back to data path format (for consistent verbose output) - */ function convertFilePathToDataPath(filePath: string): string { const normalizedPath = path.normalize(filePath) @@ -134,9 +124,6 @@ program program.parse() -/** - * Get allowed types based on command options - */ function getAllowedTypes(options: ExpandOptions): Array<'reusable' | 'variable'> { if (options.reusablesOnly && options.variablesOnly) { console.log( @@ -155,13 +142,9 @@ function getAllowedTypes(options: ExpandOptions): Array<'reusable' | 'variable'> return ['variable'] } - // Default: process both types return ['reusable', 'variable'] } -/** - * Expand Liquid data references in content files - */ async function expandReferences(options: ExpandOptions): Promise { const { paths, verbose, markers, shallow } = options // markers will be true by default, false when --no-markers is used @@ -217,7 +200,6 @@ async function expandReferences(options: ExpandOptions): Promise { } } - // Check for remaining references const remainingRefs = findLiquidReferences(expandedContent, allowedTypes) hasRemainingRefs = remainingRefs.length > 0 @@ -278,9 +260,6 @@ async function expandReferences(options: ExpandOptions): Promise { } } -/** - * Restore content by restoring original Liquid statements from HTML comments - */ async function restoreReferences(options: ExpandOptions): Promise { const { paths, verbose } = options const allowedTypes = getAllowedTypes(options) @@ -306,7 +285,6 @@ async function restoreReferences(options: ExpandOptions): Promise { const content = fs.readFileSync(filePath, 'utf-8') - // Check for content edits before restoring const hasEdits = await detectContentEdits(content, verbose, allowedTypes) if (hasEdits) { console.log( @@ -361,9 +339,6 @@ async function restoreReferences(options: ExpandOptions): Promise { } } -/** - * Expand all Liquid data references in file content - */ async function expandFileContent( content: string, filePath: string, @@ -421,14 +396,10 @@ async function expandFileContent( } } - // Note: Remaining reference detection is now handled in expandReferences function for recursive mode - + // expandReferences handles remaining-reference detection in recursive mode. return expandedContent } -/** - * Detect if expanded content has been edited by comparing with original data - */ async function detectContentEdits( content: string, verbose?: boolean, @@ -441,7 +412,6 @@ async function detectContentEdits( const [, type, dataPath, resolvedContent] = match const refType = type as 'reusable' | 'variable' - // Only check if this type is allowed if (!allowedTypes || allowedTypes.includes(refType)) { try { // Load the original content from data files @@ -478,9 +448,6 @@ async function detectContentEdits( return hasEdits } -/** - * Load data value from file system (helper for edit detection) - */ function loadDataValue(type: 'reusable' | 'variable', dataPath: string): string | null { try { const targetPath = getDataFilePath(type, dataPath) @@ -498,7 +465,6 @@ function loadDataValue(type: 'reusable' | 'variable', dataPath: string): string const yamlContent = fs.readFileSync(targetPath, 'utf8') const data = load(yamlContent) as Record - // Navigate to the nested property const pathParts = dataPath.split('.') let current: unknown = data for (let i = 1; i < pathParts.length; i++) { @@ -517,9 +483,6 @@ function loadDataValue(type: 'reusable' | 'variable', dataPath: string): string return null } -/** - * Restore content by restoring original Liquid statements - */ function restoreFileContent( content: string, verbose?: boolean, @@ -528,7 +491,6 @@ function restoreFileContent( return content.replace(EXPANDED_PATTERN, (match, type, dataPath) => { const refType = type as 'reusable' | 'variable' - // Only restore if this type is allowed if (!allowedTypes || allowedTypes.includes(refType)) { const originalLiquid = `{% data ${type}s.${dataPath} %}` @@ -539,15 +501,10 @@ function restoreFileContent( return originalLiquid } - // Return unchanged if type is not allowed return match }) } -/** - * Update data files with content from expanded blocks - * Returns array of file paths that were updated - */ function updateDataFiles( filePath: string, verbose?: boolean, @@ -564,7 +521,6 @@ function updateDataFiles( return [] } - // Group updates by file path const updatesByFile = new Map() for (const update of updates) { const key = `${update.type}:${update.path}` @@ -576,7 +532,6 @@ function updateDataFiles( const updatedFiles: string[] = [] - // Apply updates to each data file for (const [key, contents] of updatesByFile) { const [type, dataPath] = key.split(':') const targetFilePath = applyDataUpdates( @@ -594,9 +549,6 @@ function updateDataFiles( return updatedFiles } -/** - * Extract data updates from expanded content blocks - */ function extractDataUpdates( content: string, allowedTypes?: Array<'reusable' | 'variable'>, @@ -608,13 +560,11 @@ function extractDataUpdates( const [, type, dataPath, resolvedContent] = match const refType = type as 'reusable' | 'variable' - // Only include if this type is allowed if (!allowedTypes || allowedTypes.includes(refType)) { // Check if this content was actually changed before including it try { const originalContent = loadDataValue(refType, dataPath.trim()) if (originalContent !== null && resolvedContent.trim() !== originalContent.trim()) { - // Only add to updates if content was actually changed updates.push({ type: refType, path: dataPath.trim(), @@ -635,10 +585,6 @@ function extractDataUpdates( return updates } -/** - * Apply updates to a specific data file - * Returns the file path if file was updated, null otherwise - */ function applyDataUpdates( type: 'reusable' | 'variable', dataPath: string, @@ -648,7 +594,6 @@ function applyDataUpdates( ): string | null { const targetPath = getDataFilePath(type, dataPath) - // Check if file exists if (!fs.existsSync(targetPath)) { if (verbose) { console.log(chalk.red(` Error: Data file not found: ${targetPath}`)) @@ -701,7 +646,6 @@ function applyDataUpdates( const yamlContent = fs.readFileSync(targetPath, 'utf8') const data = load(yamlContent) as Record - // Navigate to the nested property const pathParts = dataPath.split('.') const propertyPath = pathParts.slice(1) // Skip the file name @@ -713,7 +657,6 @@ function applyDataUpdates( current = current[propertyPath[i]] as Record } - // Update the final property const finalKey = propertyPath[propertyPath.length - 1] if (contents.length > 1) { console.log( @@ -728,7 +671,6 @@ function applyDataUpdates( const finalYaml = hasTrailingNewline && !yamlOutput.endsWith('\n') ? `${yamlOutput}\n` : yamlOutput - // Write back to file fs.writeFileSync(targetPath, finalYaml) if (verbose) { console.log(chalk.green(` Updated: ${type}s.${dataPath}`)) @@ -743,9 +685,6 @@ function applyDataUpdates( } } -/** - * Find all Liquid data references in content - */ function findLiquidReferences( content: string, allowedTypes?: Array<'reusable' | 'variable'>, @@ -761,7 +700,6 @@ function findLiquidReferences( const [original, type, dataPath] = match const refType = type.slice(0, -1) as 'reusable' | 'variable' // Remove 's' from end - // Only include if this type is allowed if (types.includes(refType)) { references.push({ original, @@ -776,9 +714,6 @@ function findLiquidReferences( return references } -/** - * Resolve a single Liquid data reference to its content - */ async function resolveLiquidReference( ref: LiquidReference, verbose?: boolean, @@ -798,9 +733,6 @@ async function resolveLiquidReference( return null } -/** - * Resolve a reusable reference by reading the markdown file - */ async function resolveReusable(reusablePath: string, verbose?: boolean): Promise { const filePath = getDataFilePath('reusable', reusablePath) @@ -826,9 +758,6 @@ async function resolveReusable(reusablePath: string, verbose?: boolean): Promise } } -/** - * Resolve a variable reference by reading from YAML files - */ async function resolveVariable(variablePath: string, verbose?: boolean): Promise { const pathParts = variablePath.split('.') @@ -866,7 +795,6 @@ async function resolveVariable(variablePath: string, verbose?: boolean): Promise } } - // Convert value to string if (typeof value === 'string') { return value } else if (value !== null && value !== undefined) { diff --git a/src/content-render/scripts/move-by-content-type.ts b/src/content-render/scripts/move-by-content-type.ts index c64ef0ed27fc..e7773d92d881 100644 --- a/src/content-render/scripts/move-by-content-type.ts +++ b/src/content-render/scripts/move-by-content-type.ts @@ -26,7 +26,6 @@ const contentTypeToDir = (contentType: string): string => { const validContentTypeDirs = new Set(CONTENT_TYPES.map(contentTypeToDir)) -// Helper: Should we skip this index.md file from processing? function shouldSkipIndexFile(filePath: string): boolean { const relativePath = path.relative(process.cwd(), filePath) const parts = relativePath.split(path.sep) @@ -44,17 +43,14 @@ function shouldSkipIndexFile(filePath: string): boolean { return false } -// Helper: Calculate target directory for a file function calculateTarget(filePath: string, contentType: string, productDir: string) { const relativePath = path.relative(process.cwd(), filePath) const parts = relativePath.split(path.sep) const contentIndex = parts.indexOf('content') const fileName = path.basename(filePath) - // Determine target content-type directory const targetContentType = contentTypeToDir(contentType) - // Calculate target path if (targetContentType === 'how-tos') { // Preserve subdirectory structure for how-tos const pathAfterProduct = parts.slice(contentIndex + 2, -1) @@ -85,9 +81,7 @@ program .description('Reorganize content files into subdirectories based on their contentType property') .argument('[paths...]', 'Content paths to process') .action(async (paths: string[]) => { - // ==================== - // 1. GATHER FILES - // ==================== + // Gather files. const filesToProcess: string[] = [] if (paths?.length > 0) { for (const p of paths) { @@ -104,9 +98,6 @@ program console.log(chalk.white(`Processing ${filesToProcess.length} files...\n`)) - // ==================== - // 2. ANALYZE & PLAN MOVES - // ==================== console.log(chalk.white('Analyzing files...\n')) const filesToMove: FileMove[] = [] @@ -125,7 +116,6 @@ program continue } - // Read and validate contentType const fileContent = await fs.readFile(filePath, 'utf-8') const { data } = readFrontmatter(fileContent) @@ -145,7 +135,6 @@ program continue } - // Validate contentType if (!CONTENT_TYPES.includes(contentType)) { skipped.push({ file: relativePath, reason: `Invalid contentType: ${contentType}` }) console.log( @@ -154,7 +143,6 @@ program continue } - // Get product directory if (contentIndex === -1 || contentIndex + 1 >= parts.length) { console.log( chalk.yellow(`⚠ Skipping ${relativePath}: Cannot determine product directory`), @@ -168,13 +156,10 @@ program if (contentType === 'rai') productsWithRai.add(productName) - // Calculate target const { targetDir, targetPath } = calculateTarget(filePath, contentType, productDir) - // Skip if already in correct location if (path.dirname(filePath) === targetDir) continue - // Skip if target exists try { await fs.access(targetPath) skipped.push({ file: relativePath, reason: 'Target already exists' }) @@ -184,7 +169,6 @@ program // Good, doesn't exist } - // Track this move filesToMove.push({ filePath, targetDir, targetPath, contentType }) const relativeTargetDir = path.relative(process.cwd(), targetDir) @@ -209,9 +193,6 @@ program } } - // ==================== - // 3. ENSURE STANDARD DIRECTORIES - // ==================== console.log(chalk.white('Ensuring standard content-type directories exist...\n')) // Add standard content-type directories for each affected product @@ -236,9 +217,6 @@ program } } - // ==================== - // 4. CREATE PLACEHOLDERS - // ==================== console.log(chalk.white('Creating placeholder index.md files...\n')) const newPlaceholders: string[] = [] @@ -295,9 +273,6 @@ contentType: ${placeholderContentType} } } - // ==================== - // 5. GENERATE INTROS - // ==================== if (newPlaceholders.length > 0) { console.log(chalk.white('\nGenerating intros for placeholder files...\n')) @@ -337,9 +312,6 @@ contentType: ${placeholderContentType} } } - // ==================== - // 6. MOVE FILES - // ==================== console.log(chalk.white('\nMoving files...\n')) const moved: Array<{ file: string; from: string; to: string }> = [] @@ -397,7 +369,6 @@ contentType: ${placeholderContentType} } } - // Move regular files for (const file of regularFiles) { try { await fs.mkdir(file.targetDir, { recursive: true }) @@ -427,7 +398,6 @@ contentType: ${placeholderContentType} } } - // Delete source subdirectory index files for (const sourcePath of indexFilesToDeleteLater) { try { await fs.unlink(sourcePath) @@ -437,7 +407,6 @@ contentType: ${placeholderContentType} } } - // Move top-level index files for (const file of topLevelIndexFiles) { try { await fs.mkdir(file.targetDir, { recursive: true }) @@ -462,9 +431,6 @@ contentType: ${placeholderContentType} } } - // ==================== - // 7. CLEANUP & UPDATE - // ==================== console.log( chalk.white('\nCleaning up old directories and updating parent index.md files...\n'), ) @@ -498,7 +464,6 @@ contentType: ${placeholderContentType} console.log(chalk.yellow(`⚠ Could not read product directory ${productDir}: ${error}`)) } - // Update product index.md const productIndexPath = path.join(productDir, 'index.md') try { const content = await fs.readFile(productIndexPath, 'utf-8') @@ -507,7 +472,6 @@ contentType: ${placeholderContentType} if (data) { let updated = false - // Build children array const productRelativePath = path.relative(process.cwd(), productDir) const newChildren: string[] = [] for (const ct of CONTENT_TYPES.map(contentTypeToDir)) { @@ -520,7 +484,6 @@ contentType: ${placeholderContentType} updated = true } - // Add redirects for deleted directories const deletedPaths = deletedByProduct.get(productName) || [] if (deletedPaths.length > 0) { if (!data.redirect_from) data.redirect_from = [] @@ -551,9 +514,6 @@ contentType: ${placeholderContentType} } } - // ==================== - // 8. SORT CHILDREN ARRAYS - // ==================== console.log(chalk.white('\nSorting children arrays...\n')) for (const dirPath of targetDirs) { @@ -612,9 +572,6 @@ contentType: ${placeholderContentType} } } - // ==================== - // 9. SUMMARY - // ==================== console.log(chalk.white(`\n${'='.repeat(60)}`)) console.log(chalk.white('Summary:')) console.log(chalk.white(` Moved: ${moved.length} files`)) diff --git a/src/content-render/scripts/move-content.ts b/src/content-render/scripts/move-content.ts index 6bbdcb1a0635..004cb880363d 100755 --- a/src/content-render/scripts/move-content.ts +++ b/src/content-render/scripts/move-content.ts @@ -33,7 +33,6 @@ import escapeStringRegexp from 'escape-string-regexp' import fm from '@/frame/lib/frontmatter' import readFrontmatter from '@/frame/lib/read-frontmatter' -// Type definitions interface MoveOptions { verbose: boolean undo: boolean @@ -156,8 +155,7 @@ async function main(opts: MoveOptions, nameTuple: string[]) { if (!fs.existsSync(indexFilePath)) { throw new Error(`${oldPath} does not have an index.md file`) } - // Gather individual files by walking `oldPath` recursively - // The second argument is + // Gather individual files by walking `oldPath` recursively. const files = findFilesInFolder(oldPath, newPath, opts) // First take care of the `git mv` (or regular rename) part. 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 cb423a475038..c0827117caa9 100644 --- a/src/content-render/scripts/reusables-cli/find/potential-uses.ts +++ b/src/content-render/scripts/reusables-cli/find/potential-uses.ts @@ -26,7 +26,6 @@ export function findPotentialUses({ const filesThatCouldUseReusable: FilesWithLineNumbers = [] const filesThatCouldUseReusableSimilar: FilesWithSimilarity = [] - // Read all content & data files into memory const allFileContents = allFilePaths.map((filePath) => { return { filePath, @@ -69,7 +68,6 @@ export function findPotentialUses({ const indices = findIndicesOfSubstringInString(reusableContents.trim(), fileContents) if (indices.length > 0) { - // Find line numbers of each index in fileContents const lineNumbers = indices.map((index) => fileContents.slice(0, index).split('\n').length) filesThatCouldUseReusable.push({ diff --git a/src/content-render/scripts/reusables-cli/find/used.ts b/src/content-render/scripts/reusables-cli/find/used.ts index 589b87e12e6c..6f56c31512d6 100644 --- a/src/content-render/scripts/reusables-cli/find/used.ts +++ b/src/content-render/scripts/reusables-cli/find/used.ts @@ -33,7 +33,6 @@ export function findUsed(reusablePath: string, { absolute }: { absolute: boolean const indices = getIndicesOfLiquidVariable(reusableLiquidVar, fileContents) if (indices.length > 0) { - // Find line numbers of each index in fileContents const lineNumbers = indices.map((index) => fileContents.slice(0, index).split('\n').length) filesWithReusables.push({ diff --git a/src/content-render/scripts/reusables-cli/shared.ts b/src/content-render/scripts/reusables-cli/shared.ts index 792f48828bdc..c04e24725d15 100644 --- a/src/content-render/scripts/reusables-cli/shared.ts +++ b/src/content-render/scripts/reusables-cli/shared.ts @@ -48,7 +48,6 @@ export function getAllContentFilePaths() { return [...allContentFiles, ...allDataFiles] } -// Get the string that represents the reusable in the content files export function getReusableLiquidString(reusablePath: string): string { const relativePath = path.relative(reusablesDirectory, reusablePath) return `reusables.${relativePath.slice(0, -3).split('/').join('.')}` @@ -73,7 +72,6 @@ export function getIndicesOfLiquidVariable(liquidVariable: string, fileContents: return indices } -// Find the path to a reusable file. export function resolveReusablePath(reusablePath: string): string { // Try .md if extension is not provided if (!reusablePath.endsWith('.md') && !reusablePath.endsWith('.yml')) { @@ -150,7 +148,6 @@ export function findSimilarSubStringInString(substr: string, str: string) { } } - // Normalize the similarity score return Math.round((similarityScore / substrSentences.length) * corpus.length) } diff --git a/src/content-render/scripts/update-filepaths.ts b/src/content-render/scripts/update-filepaths.ts index f86693010639..7760d5c7b441 100755 --- a/src/content-render/scripts/update-filepaths.ts +++ b/src/content-render/scripts/update-filepaths.ts @@ -101,13 +101,11 @@ async function processFile( const isDirectory = isDirectoryCheck(file) - // Assess the frontmatter and other conditions to determine if we want to process the path. const processPage: boolean = determineProcessStatus(data, isDirectory, scriptOptions) if (!processPage) return null let stringToSlugify: string = data.shortTitle || data.title - // Check if we need to process Liquid if (stringToSlugify.includes('{%')) { stringToSlugify = await renderContent(stringToSlugify, context, { textOnly: true }) } @@ -118,7 +116,6 @@ async function processFile( // Fall back to title if shortTitle doesn't exist. const slug: string = slugger.slug(decode(stringToSlugify)) - // Get the basename, depending on whether it's a file or dir. let basename: string if (isDirectory) { // Where: content location = content/foobar/index.md @@ -133,12 +130,10 @@ async function processFile( // If slug and basename already match, all set here. Return early. if (slug === basename) return null - // Build the new path based on file type. const newPath = isDirectory ? path.join(path.dirname(path.dirname(file)), slug, 'index.md') : path.join(path.dirname(file), `${slug}.md`) - // Get relative paths and adjust for directories. const getContentPath = (filePath: string): string => { const relativePath = path.relative(process.cwd(), filePath) return isDirectory ? path.dirname(relativePath) : relativePath @@ -187,26 +182,21 @@ function sortFiles(filesArray: string[]): string[] { // 2. Deepest subdirectory path // 3. Shallowest subdirectory path (up to category level, e.g., content/product/category) return filesArray.toSorted((a, b) => { - // If A is a file and B is a directory, A comes first (negative) if (!isDirectoryCheck(a) && isDirectoryCheck(b)) { return -1 } - // If A is a directory and B is a file, B comes first (positive) if (isDirectoryCheck(a) && !isDirectoryCheck(b)) { return 1 } - // If A and B are both files, neutral if (!isDirectoryCheck(a) && !isDirectoryCheck(b)) { return 0 } - // If both are directories, sort by depth (deepest first) 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 } - // This should never be reached, but return 0 for safety return 0 }) } @@ -246,20 +236,16 @@ function determineProcessStatus( isDirectory: boolean, scriptOptions: ScriptOptions, ): boolean { - // Assess the conditions in this order: - // If it's a directory AND we're excluding dirs, do not process it no matter what. + // A directory is never processed when dirs are excluded, whatever else is set. if (isDirectory && scriptOptions.excludeDirs) { return false } - // If the force option is passed, process it no matter what. if (scriptOptions.force) { return true } - // If the page has the override set, do not process it. if (data.allowTitleToDifferFromFilename) { return false } - // In all other cases, process it. return true } diff --git a/src/content-render/tests/annotate.ts b/src/content-render/tests/annotate.ts index 20083164d3bf..c47a78d6fb76 100644 --- a/src/content-render/tests/annotate.ts +++ b/src/content-render/tests/annotate.ts @@ -22,33 +22,27 @@ describe('annotate', () => { const res = await renderContent(example) const $ = load(res) - // Check that the annotation structure is rendered correctly const annotation = $('.annotate') expect(annotation.length).toBe(1) expect(annotation.hasClass('beside')).toBe(true) - // Check annotation header exists const header = $('.annotate-header') expect(header.length).toBe(1) - // Check both beside and inline modes are rendered const beside = $('.annotate-beside') const inline = $('.annotate-inline') expect(beside.length).toBe(1) expect(inline.length).toBe(1) - // Check that we have the correct number of annotation rows const rows = $('.annotate-row') expect(rows.length).toBe(2) - // Check that each row has both code and note sections rows.each((i, row) => { const $row = $(row) expect($row.find('.annotate-code').length).toBe(1) expect($row.find('.annotate-note').length).toBe(1) }) - // Check specific content of the annotations const notes = $('.annotate-note p') const noteTexts = notes.map((i, el) => $(el).text()).get() expect(noteTexts).toEqual([ @@ -56,7 +50,6 @@ describe('annotate', () => { 'Add the pull_request event, so that the workflow runs automatically\nevery time a pull request is created.', ]) - // Check code content const codes = $('.annotate-code pre') const codeTexts = codes.map((i, el) => $(el).text()).get() expect(codeTexts).toEqual([ @@ -157,7 +150,6 @@ on: [push] const rows = $('.annotate-row') const notes = $('.annotate-note', rows) - // Check that AUTOTITLE links were resolved to actual titles const firstNote = notes.eq(0).html() const secondNote = notes.eq(1).html() diff --git a/src/content-render/tests/copilot-code-blocks.ts b/src/content-render/tests/copilot-code-blocks.ts index 51209f95663e..2dfe76d6b4c2 100644 --- a/src/content-render/tests/copilot-code-blocks.ts +++ b/src/content-render/tests/copilot-code-blocks.ts @@ -18,11 +18,8 @@ describe('code-header plugin', () => { const html = await renderContent(markdown) - // Should keep copilot as the language (not convert to text without copy meta) expect(html).toContain('language-copilot') - // Should NOT wrap in code-example div since no copy meta expect(html).not.toContain('code-example') - // Should NOT have header since no copy meta expect(html).not.toContain(' { const html = await renderContent(markdown) - // Should be wrapped in code-example div expect(html).toContain('code-example') - // Should have header with copy button expect(html).toContain(' { const html = await renderContent(markdown) - // Should be wrapped in code-example div expect(html).toContain('code-example') - // Should have header expect(html).toContain(' { const html = await renderContent(markdown) - // Should be wrapped in code-example div expect(html).toContain('code-example') - // Should have header with copy button expect(html).toContain(' { let mockContext: Context beforeEach(async () => { - // Set up file system mocking fs = await import('fs') originalReadFileSync = fs.default.readFileSync originalExistsSync = fs.default.existsSync fs.default.existsSync = () => true - // Set up basic mock context mockContext = { currentLanguage: 'en', currentVersion: 'free-pro-team@latest', @@ -30,13 +28,11 @@ describe('link error line numbers', () => { }) afterEach(() => { - // Restore original functions fs.default.readFileSync = originalReadFileSync fs.default.existsSync = originalExistsSync }) test('reports correct line numbers for broken AUTOTITLE links', async () => { - // Test content with frontmatter followed by content with a broken link const template = `--- title: Test Page version: 1.0 @@ -72,7 +68,6 @@ More content here.` fullPath: '/fake/test-file-2.md', } as unknown as Context['page'] - // Test with more extensive frontmatter const template = `--- title: Another Test Page description: This is a test @@ -109,7 +104,6 @@ Content with a [AUTOTITLE](/another/nonexistent/page) link.` fullPath: '/fake/no-frontmatter.md', } as unknown as Context['page'] - // Test content without frontmatter const template = `# Simple Title This is content without frontmatter. @@ -148,7 +142,6 @@ title: Message Test } catch (error) { expect(error).toBeInstanceOf(TitleFromAutotitleError) - // Check that the new error message format is used expect((error as TitleFromAutotitleError).message).toContain( 'could not be resolved in one or more versions', ) @@ -157,7 +150,6 @@ title: Message Test ) expect((error as TitleFromAutotitleError).message).toContain('/test/broken/link') - // Check that the old error message format is NOT used expect((error as TitleFromAutotitleError).message).not.toContain('Unable to find Page by') expect((error as TitleFromAutotitleError).message).not.toContain('To fix it, look at') } diff --git a/src/content-render/tests/liquid-tags.ts b/src/content-render/tests/liquid-tags.ts index 0209be92c090..db28d494733b 100644 --- a/src/content-render/tests/liquid-tags.ts +++ b/src/content-render/tests/liquid-tags.ts @@ -10,16 +10,13 @@ describe('liquid-tags script integration tests', () => { vi.setConfig({ testTimeout: 60 * 1000 }) beforeEach(async () => { - // Create test directory await fs.mkdir(testContentDir, { recursive: true }) }) afterEach(async () => { - // Clean up test files await fs.rm(testContentDir, { recursive: true, force: true }) }) - // Helper function to run script commands async function runScript(args: string): Promise<{ output: string; exitCode: number }> { let output = '' let exitCode = 0 @@ -41,7 +38,6 @@ describe('liquid-tags script integration tests', () => { } test('expand command should complete successfully with basic content', async () => { - // Create a test file with liquid reference const testFile = path.join(testContentDir, 'basic-test.md') const testContent = `--- title: Test @@ -54,11 +50,9 @@ This uses {% data variables.product.prodname_dotcom %} in content. const { output, exitCode } = await runScript(`expand --paths "${testFile}"`) - // Should complete without error expect(exitCode, `Script failed with output: ${output}`).toBe(0) expect(output.length).toBeGreaterThan(0) - // Check that the file was modified const expandedContent = await fs.readFile(testFile, 'utf8') expect(expandedContent).not.toBe(testContent) expect(expandedContent).toContain('GitHub') // Should expand to actual fixture value @@ -75,16 +69,13 @@ This uses {% data variables.product.prodname_dotcom %} in content. await fs.writeFile(testFile, originalContent) - // First expand await runScript(`expand --paths "${testFile}"`) - // Then restore const { output, exitCode } = await runScript(`restore --paths "${testFile}"`) expect(exitCode, `Restore script failed with output: ${output}`).toBe(0) expect(output.length).toBeGreaterThan(0) - // Should be back to original liquid tags const restoredContent = await fs.readFile(testFile, 'utf8') expect(restoredContent).toContain('{% data variables.product.prodname_dotcom %}') expect(restoredContent).not.toContain('GitHub') diff --git a/src/content-render/tests/prompt-id.ts b/src/content-render/tests/prompt-id.ts index ce7441e8832b..71e046b0fd48 100644 --- a/src/content-render/tests/prompt-id.ts +++ b/src/content-render/tests/prompt-id.ts @@ -55,7 +55,6 @@ describe('generatePromptId', () => { expect(typeof id).toBe('string') expect(id.length).toBeGreaterThan(0) - // Should be different from just the prompt text alone expect(id).not.toBe(generatePromptId(promptText)) }) diff --git a/src/content-render/tests/prompt.ts b/src/content-render/tests/prompt.ts index 65de51fea1c3..e8e250e166e1 100644 --- a/src/content-render/tests/prompt.ts +++ b/src/content-render/tests/prompt.ts @@ -6,18 +6,14 @@ describe('prompt tag', () => { const input: string = 'Here is your prompt: {% prompt %}example prompt text{% endprompt %}.' const output: string = await renderContent(input) - // Should have code element with ID expect(output).toContain(' { expect(reusableFiles).toHaveLength(1) expect(reusableFiles[0]).toMatch(/test\.md$/) } finally { - // Clean up fs.rmSync(tempDir, { recursive: true, force: true }) } }) @@ -128,11 +115,9 @@ describe('orphaned features detection', () => { expect(fs.existsSync(path.join(featuresDir, 'used-in-variables.yml'))).toBe(true) expect(fs.existsSync(path.join(featuresDir, 'truly-orphaned.yml'))).toBe(true) - // Check that the variable file references the feature const variableContent = fs.readFileSync(path.join(variablesDir, 'test.yml'), 'utf-8') expect(variableContent).toContain('used-in-variables') - // Verify that the getVariableFiles function would find this file const variableFiles = getVariableFiles(variablesDir) expect(variableFiles.length).toBeGreaterThan(0) @@ -152,7 +137,6 @@ describe('orphaned features detection', () => { const tempDir = path.join(__dirname, 'temp-mixed-target-files') fs.mkdirSync(tempDir, { recursive: true }) - // Create files that both functions might encounter fs.writeFileSync( path.join(tempDir, 'variables.yml'), 'var: {% ifversion test-feature %}enabled{% endif %}', @@ -174,11 +158,9 @@ describe('orphaned features detection', () => { expect(reusableFiles).toHaveLength(1) expect(reusableFiles[0]).toMatch(/reusable\.md$/) - // Verify no cross-contamination expect(variableFiles.some((f) => f.endsWith('.md'))).toBe(false) expect(reusableFiles.some((f) => f.endsWith('.yml'))).toBe(false) } finally { - // Clean up fs.rmSync(tempDir, { recursive: true, force: true }) } }) diff --git a/src/fixtures/playwright.config.ts b/src/fixtures/playwright.config.ts index 9ba48a14ccaf..7d4e382171aa 100644 --- a/src/fixtures/playwright.config.ts +++ b/src/fixtures/playwright.config.ts @@ -1,11 +1,5 @@ import { defineConfig } from '@playwright/test' -/** - * Read environment variables from file. - * https://github.com/motdotla/dotenv - */ -// require('dotenv').config(); - const CI = Boolean(JSON.parse(process.env.CI || 'false')) const PLAYWRIGHT_START_SERVER_COMMAND = @@ -36,7 +30,6 @@ const EXPECT_TIMEOUT = process.env.PLAYWRIGHT_EXPECT_TIMEOUT */ export default defineConfig({ testDir: './tests', - /* Maximum time one test can run for. */ timeout: TIMEOUT, expect: { /** @@ -45,13 +38,9 @@ export default defineConfig({ */ timeout: EXPECT_TIMEOUT, }, - /* Run tests in files in parallel */ fullyParallel: true, - /* Fail the build on CI if you accidentally left test.only in the source code. */ forbidOnly: !!process.env.CI, - /* Retry on CI only */ retries: RETRIES, - /* Opt out of parallel tests on CI. */ workers: process.env.PLAYWRIGHT_WORKERS ? JSON.parse(process.env.PLAYWRIGHT_WORKERS) : CI @@ -63,14 +52,12 @@ export default defineConfig({ use: { /* Maximum time each action such as `click()` can take. Defaults to 0 (no limit). */ actionTimeout: 0, - /* Base URL to use in actions like `await page.goto('/')`. */ baseURL: 'http://localhost:4000', /* Collect trace when retrying the failed test. See https://playwright.dev/docs/trace-viewer */ trace: 'on-first-retry', }, - /* Configure projects for major browsers */ projects: [ // { // name: 'chromium', @@ -125,7 +112,6 @@ export default defineConfig({ /* Folder for test artifacts such as screenshots, videos, traces, etc. */ // outputDir: 'test-results/', - /* Run your local dev server before starting the tests */ webServer: { command: PLAYWRIGHT_START_SERVER_COMMAND, port: 4000, diff --git a/src/fixtures/tests/api-article-body.ts b/src/fixtures/tests/api-article-body.ts index 05006793549c..fb4d3de6d23a 100644 --- a/src/fixtures/tests/api-article-body.ts +++ b/src/fixtures/tests/api-article-body.ts @@ -55,9 +55,7 @@ describe('article body api', () => { const res = await get(makeURL('/en/get-started/start-your-journey/api-article-body-test-page')) expect(res.statusCode).toBe(200) - // Should not contain frontmatter expect(res.body).not.toMatch(/^---/) - // Should have at least one heading expect(res.body).toMatch(/^#{1,6}\s+\w+/m) }) @@ -80,21 +78,17 @@ describe('article body api', () => { expect(res.statusCode).toBe(200) expect(res.headers['content-type']).toContain('text/markdown') - // Should contain all tool-specific content variants expect(res.body).toContain('
') expect(res.body).toContain('
') expect(res.body).toContain('
') - // Should contain the actual content from each tool expect(res.body).toContain('This is webui content') expect(res.body).toContain('This is cli content') expect(res.body).toContain('This is desktop content') - // Should contain tool-specific sections expect(res.body).toContain('Webui section specific content') expect(res.body).toContain('Desktop section specific content') - // Verify multiple instances of the same tool are preserved const webuiMatches = res.body.match(/
/g) const desktopMatches = res.body.match(/
/g) expect(webuiMatches).toBeDefined() @@ -108,11 +102,9 @@ describe('article body api', () => { expect(res.statusCode).toBe(200) expect(res.headers['content-type']).toContain('text/markdown') - // Should contain both webui and codespaces tool content expect(res.body).toContain('
') expect(res.body).toContain('
') - // Should contain the actual content from both tools expect(res.body).toContain('Under your repository name, click **Pull requests**') expect(res.body).toContain('Open the pull request in your codespace') expect(res.body).toContain( @@ -122,7 +114,6 @@ describe('article body api', () => { 'After reviewing the files, you can submit your review directly from Codespaces', ) - // Verify both tools appear in multiple sections const webuiMatches = res.body.match(/
/g) const codespacesMatches = res.body.match(/
/g) expect(webuiMatches).toBeDefined() @@ -139,7 +130,6 @@ describe('article body api', () => { ), ) - // Skip test if page doesn't exist in fixture environment if (res.statusCode === 404) { console.log('Production page not available in fixture environment, skipping test') return @@ -147,11 +137,9 @@ describe('article body api', () => { expect(res.statusCode).toBe(200) - // Verify the fix is working - codespaces content should now be present const hasCodespacesContent = res.body.includes('
') expect(hasCodespacesContent).toBe(true) - // Also verify that webui content is still present expect(res.body).toContain('
') }) @@ -164,7 +152,6 @@ describe('article body api', () => { ), ) - // Skip test if page doesn't exist in fixture environment if (res.statusCode === 404) { console.log( 'Production page not available in fixture environment, skipping issue verification test', @@ -179,11 +166,9 @@ describe('article body api', () => { expect(res.body).toContain('
') expect(res.body).toContain('
') - // Verify specific codespaces content that was missing before the fix expect(res.body).toContain('GitHub Codespaces') expect(res.body).toContain('Open the pull request in a codespace') - // Ensure both tools are rendered with their respective content const webuiMatches = res.body.match(/
/g) const codespacesMatches = res.body.match(/
/g) diff --git a/src/fixtures/tests/categories-and-subcategory.ts b/src/fixtures/tests/categories-and-subcategory.ts index fcbf6375987d..24aa47af6056 100644 --- a/src/fixtures/tests/categories-and-subcategory.ts +++ b/src/fixtures/tests/categories-and-subcategory.ts @@ -17,7 +17,7 @@ describe('subcategories', () => { expect( hrefs.every((href: string) => href.startsWith('/en/get-started/start-your-journey/')), ).toBeTruthy() - // The all resolve to a 200 OK without redirects + // 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() }) @@ -53,7 +53,7 @@ describe('categories', () => { // 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() - // The all resolve to a 200 OK without redirects + // 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/playwright-a11y.spec.ts b/src/fixtures/tests/playwright-a11y.spec.ts index 0eacad14ed50..e3e24039c38d 100644 --- a/src/fixtures/tests/playwright-a11y.spec.ts +++ b/src/fixtures/tests/playwright-a11y.spec.ts @@ -54,8 +54,8 @@ for (const pageName of Object.keys(pages)) { // 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 - // — matching the guard every search test in playwright-rendering.spec.ts uses. + // 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. test.skip(!SEARCH_TESTS, 'No local Elasticsearch, no tests involving search') test('expanded filter disclosure passes axe', async ({ page }) => { diff --git a/src/fixtures/tests/playwright-rendering.spec.ts b/src/fixtures/tests/playwright-rendering.spec.ts index 461b8e3643a6..c5e18a7880b8 100644 --- a/src/fixtures/tests/playwright-rendering.spec.ts +++ b/src/fixtures/tests/playwright-rendering.spec.ts @@ -145,12 +145,9 @@ test('do a search from home page and click on "Foo" page', async ({ page }) => { await page.goto('/') await turnOffExperimentsInPage(page) - // Use the search overlay await page.locator('[data-testid="search"]:visible').click() await page.getByTestId('overlay-search-input').fill('serve playwright') - // Wait for search results to load await page.waitForTimeout(1000) - // Click "View more results" to get to the search page await page.getByText('View more results').click() await expect(page).toHaveURL( @@ -202,9 +199,7 @@ test('open search, and select a general search article', async ({ page }) => { // Let new suggestions load const searchOverlay = page.getByTestId('general-autocomplete-suggestions') await expect(searchOverlay.getByText('For Playwright')).toBeVisible() - // Navigate to general search item, "For Playwright" await page.keyboard.press('ArrowDown') - // Select the general search item, "For Playwright" await page.keyboard.press('Enter') // We should now be on the page for "For Playwright" @@ -241,9 +236,7 @@ test('open search, and get auto-complete results', async ({ page }) => { await expect(searchInput).toBeVisible() await expect(searchInput).toBeEnabled() - // Type the text "rest" into the search input await searchInput.fill('rest') - // For for 1 second for the suggestions to load await page.waitForTimeout(1000) // Ask AI suggestions @@ -267,12 +260,9 @@ test('search from enterprise-cloud and filter by top-level Fooing', async ({ pag await page.goto('/enterprise-cloud@latest') await turnOffExperimentsInPage(page) - // Use the search overlay await page.locator('[data-testid="search"]:visible').click() await page.getByTestId('overlay-search-input').fill('fixture') - // Wait for search results to load await page.waitForTimeout(1000) - // Click "View more results" to get to the search page await page.getByText('View more results').click() // Now we're on the search results page, apply the filter @@ -309,7 +299,7 @@ test.describe('platform picker', () => { }) test('minitoc matches picker', async ({ page }) => { - // default platform set to windows in fixture fronmatter + // The fixture frontmatter defaults the platform to Windows. await page.goto('/get-started/liquid/platform-specific') await turnOffExperimentsInPage(page) await expect( @@ -450,12 +440,10 @@ test('navigate with side bar into article inside a subcategory inside a category }) test('sidebar custom link functionality works', async ({ page }) => { - // Test that sidebar functionality is not broken by custom links feature await page.goto('/get-started') await expect(page).toHaveTitle(/Getting started with HubGit/) - // Verify that regular sidebar navigation still works by clicking on known sections await page.getByTestId('product-sidebar').getByText('Start your journey').click() await page.getByTestId('product-sidebar').getByText('Hello World').click() await expect(page).toHaveURL(/\/en\/get-started\/start-your-journey\/hello-world/) @@ -550,7 +538,6 @@ test.describe('hover cards', () => { ), ).toBeVisible() - // Press Escape to close it await page.keyboard.press('Escape') await expect( page.getByText( @@ -574,7 +561,6 @@ test.describe('hover cards', () => { ), ).toBeVisible() - // click the Esc key to close the hovercard await page.keyboard.press('Escape') await expect( page.getByText( @@ -603,7 +589,7 @@ test.describe('test nav at different viewports', () => { await expect(page.getByTestId('breadcrumbs-bar')).toBeVisible() // breadcrumbs show up in one of the pages that use the AutomatedPage - // component (e.g. graphql, audit log, etc.) -- we test the webhooks + // component (e.g. graphql, audit log). This one uses the webhooks // reference page here await page.goto('/webhooks/webhook-events-and-payloads') await expect(page.getByTestId('breadcrumbs-bar')).toBeVisible() @@ -627,7 +613,7 @@ test.describe('test nav at different viewports', () => { height: 700, }) - // Opening the mobile nav must still render the doc-tree drawer -- before the + // Opening the mobile nav must still render the doc-tree drawer. Before the // fix, `collapsed` short-circuited the sidebar to null while the open state // hid the content column, leaving a blank area with no drawer. await page.getByTestId('sidebar-mobile-toggle').click() @@ -651,7 +637,7 @@ test.describe('test nav at different viewports', () => { const nav = page.locator('[data-container="nav"]') await expect(nav).toHaveAttribute('data-mobile-open', 'true') - // Resize up to the desktop breakpoint -- the inline nav should close and the + // Resize up to the desktop breakpoint. The inline nav should close and the // fixed desktop rail (326px) should take over rather than the full-width // mobile markup persisting over the page. await page.setViewportSize({ @@ -669,16 +655,13 @@ test.describe('test nav at different viewports', () => { }) await page.goto('/get-started/foo/bar') - // version picker should be visible await page.getByTestId('version-picker').getByRole('button').click() expect((await page.getByRole('menuitemradio').all()).length).toBeGreaterThan(0) await expect(page.getByRole('menuitemradio', { name: 'Enterprise Cloud' })).toBeVisible() - // language picker is visible await page.getByRole('button', { name: 'Select language: current language is English' }).click() await expect(page.getByRole('menuitemradio', { name: 'English' })).toBeVisible() - // header sign up button is visible await expect(page.getByTestId('header-signup')).toBeVisible() }) @@ -707,19 +690,15 @@ test.describe('test nav at different viewports', () => { }) await page.goto('/get-started/foo/bar') - // version picker is visible await page.getByTestId('version-picker').getByRole('button').click() expect((await page.getByRole('menuitemradio').all()).length).toBeGreaterThan(0) await expect(page.getByRole('menuitemradio', { name: 'Enterprise Cloud' })).toBeVisible() - // language picker is in mobile menu await page.getByTestId('mobile-menu').click() await expect(page.getByRole('menuitemradio', { name: 'English' })).toBeVisible() - // sign up button is in mobile menu await expect(page.getByTestId('mobile-signup')).toBeVisible() - // hamburger button for sidebar overlay is visible await expect(page.getByTestId('sidebar-mobile-toggle')).toBeVisible() await page.getByTestId('sidebar-mobile-toggle').click() await expect(page.getByTestId('sidebar')).toBeVisible() @@ -732,23 +711,17 @@ test.describe('test nav at different viewports', () => { }) await page.goto('/get-started/foo/bar') - // header sign-up button is not visible await expect(page.getByTestId('header-signup')).not.toBeVisible() - // language picker is not visible await expect(page.getByTestId('language-picker')).not.toBeVisible() - // version picker is visible await expect(page.getByTestId('version-picker').getByRole('button')).toBeVisible() - // language picker is in mobile menu await page.getByTestId('mobile-menu').click() await expect(page.getByRole('menuitemradio', { name: 'English' })).toBeVisible() - // sign up button is in mobile menu await expect(page.getByTestId('mobile-signup')).toBeVisible() - // hamburger button for sidebar overlay is visible await expect(page.getByTestId('sidebar-mobile-toggle')).toBeVisible() await page.getByTestId('sidebar-mobile-toggle').click() await expect(page.getByTestId('sidebar')).toBeVisible() @@ -762,27 +735,20 @@ test.describe('test nav at different viewports', () => { await page.goto('/get-started/foo/bar') await turnOffExperimentsInPage(page) - // header sign-up button is not visible await expect(page.getByTestId('header-signup')).not.toBeVisible() - // language picker is not visible await expect(page.getByTestId('language-picker')).not.toBeVisible() - // version picker is not visible await expect(page.getByTestId('version-picker').getByRole('button')).not.toBeVisible() - // version picker is in mobile menu await expect(page.getByTestId('version-picker')).not.toBeVisible() await page.getByTestId('mobile-menu').click() await expect(page.getByTestId('open-mobile-menu').getByTestId('version-picker')).toBeVisible() - // language picker is in mobile menu await expect(page.getByTestId('open-mobile-menu').getByTestId('language-picker')).toBeVisible() - // sign up button is in mobile menu await expect(page.getByTestId('mobile-signup')).toBeVisible() - // hamburger button for sidebar overlay is visible await expect(page.getByTestId('sidebar-mobile-toggle')).toBeVisible() await page.getByTestId('sidebar-mobile-toggle').click() await expect(page.getByTestId('sidebar')).toBeVisible() @@ -798,12 +764,9 @@ test.describe('test nav at different viewports', () => { await page.goto('/get-started/foo/bar') await turnOffExperimentsInPage(page) - // Use the search overlay await page.locator('[data-testid="mobile-search-button"]:visible').click() await page.getByTestId('overlay-search-input').fill('serve playwright') - // Wait for search results to load await page.waitForTimeout(1000) - // Click "View more results" to get to the search page await page.getByText('View more results').click() await expect(page).toHaveURL( @@ -822,12 +785,9 @@ test.describe('test nav at different viewports', () => { await page.goto('/get-started/foo/bar') await turnOffExperimentsInPage(page) - // Use the search overlay await page.locator('[data-testid="mobile-search-button"]:visible').click() await page.getByTestId('overlay-search-input').fill('serve playwright') - // Wait for search results to load await page.waitForTimeout(1000) - // Click "View more results" to get to the search page await page.getByText('View more results').click() await expect(page).toHaveURL( @@ -841,7 +801,7 @@ test.describe('secondary-bar breadcrumb scroller', () => { // The secondary bar (and its breadcrumb scroller) only renders at wide // viewports, and the fixture trail is short enough to fit there, so we cap the // scroller width to force a deterministic overflow independent of title - // lengths — then exercise the chevrons. + // lengths, then exercise the chevrons. test('chevrons scroll one crumb at a time instead of jumping to the ends', async ({ page }) => { // Smooth-scroll settle waits across several chevron clicks add up past the // default 5s cap. diff --git a/src/frame/components/CodeTabsGroup.tsx b/src/frame/components/CodeTabsGroup.tsx index 2d326d26eb03..d6b701399090 100644 --- a/src/frame/components/CodeTabsGroup.tsx +++ b/src/frame/components/CodeTabsGroup.tsx @@ -27,11 +27,11 @@ import { useTranslation } from '@/languages/components/useTranslation' // React-native replacement for the imperative CodeTabs enhancer (#6619). The old // component scanned `#article-contents` for `.ghd-codetabs`, inserted a foreign // `.ghd-codetabs-nav` mountPoint as the container's first child, portaled a nav -// into it, and toggled panel attributes — destructive surgery on React-owned -// nodes that breaks on client-side navigation teardown. Instead, the article body -// hast maps each `.ghd-codetabs` container to , which reads its -// `.ghd-codetab` panel children straight from props and renders the nav + panels -// itself. No DOM scanning, no portal, no foreign nodes. +// into it, and toggled panel attributes. Those mutations are destructive surgery +// on React-owned nodes and break on client-side navigation teardown. Instead, the +// article body hast maps each `.ghd-codetabs` container to , which +// reads its `.ghd-codetab` panel children straight from props and renders the nav +// and panels itself. No DOM scanning, no portal, no foreign nodes. // // The selected language lives in CodeLanguageContext so multiple code-tab groups // on one page stay in sync and share the language cookie, matching the previous diff --git a/src/frame/components/DefaultLayout.module.scss b/src/frame/components/DefaultLayout.module.scss index f6dea1f56509..d71ccc296c89 100644 --- a/src/frame/components/DefaultLayout.module.scss +++ b/src/frame/components/DefaultLayout.module.scss @@ -14,7 +14,7 @@ } } -// When the inline mobile/tablet nav is open, it takes over the viewport — hide +// When the inline mobile/tablet nav is open, it takes over the viewport, so hide // the content column so the full-width rail isn't squeezed beside it. Above the // xxl breakpoint the rail is a fixed-width sibling, so content always shows. .contentHiddenForNav { diff --git a/src/frame/components/DefaultLayout.tsx b/src/frame/components/DefaultLayout.tsx index 133e2f7b4467..ea174a327aa8 100644 --- a/src/frame/components/DefaultLayout.tsx +++ b/src/frame/components/DefaultLayout.tsx @@ -117,9 +117,7 @@ export const DefaultLayout = (props: Props) => { return getCategoryImageUrl('default') } - // Helper function to build API article URLs with proper query parameter handling function buildApiArticleUrl(apiPath: string): string { - // Parse router.asPath to separate pathname and query parameters const [pathname, queryString] = router.asPath.split('?') const fullPathname = `/${router.locale}${pathname}` const queryParams = queryString ? `&${queryString}` : '' @@ -256,7 +254,7 @@ const LayoutBody = ({ children }: LayoutBodyProps) => { const { collapsed, mobileNavOpen } = useSidebarCollapsed() const { currentProduct } = useMainContext() // Matches SidebarNav's own gate rather than testing router.route. There are two search - // pages — src/pages/search.tsx and src/pages/[versionId]/search.tsx — so a route test + // pages, src/pages/search.tsx and src/pages/[versionId]/search.tsx, so a route test // for '/search' misses every versioned search URL, and this check would then disagree // with SidebarNav about whether the rail is a facet rail. const isSearchResultsPage = currentProduct?.id === 'search' @@ -266,8 +264,8 @@ const LayoutBody = ({ children }: LayoutBodyProps) => { // gets an earlier split of its own. Route-gated, so no other page moves.
{/* `collapsed` is the desktop rail-collapse state (persisted). The inline - mobile nav is independent, so still render the sidebar when it's open — - otherwise opening the mobile nav while the desktop rail is collapsed + mobile nav is independent, so still render the sidebar when it's open. + Otherwise opening the mobile nav while the desktop rail is collapsed hides the content column (contentHiddenForNav) with no drawer to show, so the open nav displays a blank area instead of the doc tree. diff --git a/src/frame/components/HighlightedCode.tsx b/src/frame/components/HighlightedCode.tsx index 4e5d2c7a31a6..db1d48207990 100644 --- a/src/frame/components/HighlightedCode.tsx +++ b/src/frame/components/HighlightedCode.tsx @@ -10,8 +10,8 @@ import cx from 'classnames' // React-native replacement for the imperative ClientSideHighlightJS enhancer // (#6619). The old enhancer scanned the document for `[data-highlight] code` and -// called `hljs.highlightElement`, which REPLACES the ``'s innerHTML — -// destructive on a React-owned node. Instead, components that render code +// called `hljs.highlightElement`, which REPLACES the ``'s innerHTML. +// That is destructive on a React-owned node. Instead, components that render code // (RestCodeSamples, Webhook) use , which highlights with // `lowlight` (the hast-based highlighter behind rehype-highlight) and renders the // tokens as real React elements via `toJsxRuntime`. No innerHTML, no DOM scan. diff --git a/src/frame/components/UtmPreserver.tsx b/src/frame/components/UtmPreserver.tsx index e92aad49f6bb..a6805d4bf64a 100644 --- a/src/frame/components/UtmPreserver.tsx +++ b/src/frame/components/UtmPreserver.tsx @@ -5,7 +5,6 @@ export const UtmPreserver = () => { const router = useRouter() useEffect(() => { - // Extract UTM parameters from current URL const getUtmParams = (): URLSearchParams => { const urlParams = new URLSearchParams(window.location.search) const utmParams = new URLSearchParams() @@ -22,7 +21,6 @@ export const UtmPreserver = () => { const utmParams = getUtmParams() if (utmParams.toString() === '') return - // Check if a link should have UTM parameters preserved const shouldPreserveUtm = (url: string): boolean => { const lowercaseUrl = url.toLowerCase() @@ -35,7 +33,6 @@ export const UtmPreserver = () => { return hasProtocol && isGithubCom && !isDocsGithubCom } - // Add UTM parameters to a URL const addUtmParamsToUrl = (url: string, params: URLSearchParams): string => { try { const urlObj = new URL(url) @@ -46,12 +43,10 @@ export const UtmPreserver = () => { return urlObj.toString() } catch { - // If URL parsing fails, return original URL return url } } - // Apply UTM parameters to relevant links const applyUtmToLinks = (): void => { const links = document.querySelectorAll('a[href]') @@ -62,7 +57,6 @@ export const UtmPreserver = () => { } } - // Handle click events for dynamic link modification const handleLinkClick = (event: Event): void => { const link = (event.target as Element)?.closest('a') as HTMLAnchorElement if (!link || !link.href) return @@ -72,7 +66,6 @@ export const UtmPreserver = () => { } } - // Apply UTM parameters immediately to existing links applyUtmToLinks() // Also handle clicks for any dynamically added links @@ -86,13 +79,11 @@ export const UtmPreserver = () => { router.events.on('routeChangeComplete', handleRouteChange) - // Cleanup return () => { document.removeEventListener('click', handleLinkClick, true) router.events.off('routeChangeComplete', handleRouteChange) } }, [router.asPath, router.events]) - // This component doesn't render anything return null } diff --git a/src/frame/components/context/ArticleContext.tsx b/src/frame/components/context/ArticleContext.tsx index 2b84f53b0a95..3582e7047469 100644 --- a/src/frame/components/context/ArticleContext.tsx +++ b/src/frame/components/context/ArticleContext.tsx @@ -55,7 +55,7 @@ const PagePathToVaFlowMapping: Record = { 'pages_ssl_check', } -// Request type for context extraction — uses Record for the page +// Request type for context extraction. Uses Record for the page // because the Page type doesn't include all runtime-computed properties. interface ContextRequest { context: { diff --git a/src/frame/components/context/CategoryLandingContext.tsx b/src/frame/components/context/CategoryLandingContext.tsx index 41da55f48f96..d92d382fb29d 100644 --- a/src/frame/components/context/CategoryLandingContext.tsx +++ b/src/frame/components/context/CategoryLandingContext.tsx @@ -32,7 +32,7 @@ export const useCategoryLandingContext = (): CategoryLandingContextT => { return context } -// Request type for context extraction — uses Record for the page +// Request type for context extraction. Uses Record for the page // because the Page type doesn't include all runtime-computed properties. interface ContextRequest { context: { diff --git a/src/frame/components/context/TocLandingContext.tsx b/src/frame/components/context/TocLandingContext.tsx index 7864f3bbbf5b..e69284f5b784 100644 --- a/src/frame/components/context/TocLandingContext.tsx +++ b/src/frame/components/context/TocLandingContext.tsx @@ -27,7 +27,7 @@ export const useTocLandingContext = (): TocLandingContextT => { return context } -// Request type for context extraction — uses Record for the page +// Request type for context extraction. Uses Record for the page // because the Page type doesn't include all runtime-computed properties. interface ContextRequest { context: { diff --git a/src/frame/components/lib/prefetch.ts b/src/frame/components/lib/prefetch.ts index 84f7b3833a0c..7213b3cacca0 100644 --- a/src/frame/components/lib/prefetch.ts +++ b/src/frame/components/lib/prefetch.ts @@ -4,7 +4,7 @@ import type { useRouter } from 'next/router' type Router = ReturnType // Session-lived de-dupe set. Module scope (not a per-component useRef) so it -// survives the sidebar's per-navigation remount — otherwise the cache would +// survives the sidebar's per-navigation remount. Otherwise the cache would // reset every nav and "de-duped per href" would only hold within a single page. // Bounded by the number of distinct internal hrefs the user hovers/focuses. const prefetchedHrefs = new Set() diff --git a/src/frame/components/lib/toggle-annotations.ts b/src/frame/components/lib/toggle-annotations.ts index cdce002b7821..628391ec7c1e 100644 --- a/src/frame/components/lib/toggle-annotations.ts +++ b/src/frame/components/lib/toggle-annotations.ts @@ -8,15 +8,9 @@ enum annotationMode { Inline = 'inline', } -/** - * Validates if a given mode is one of expected annotation modes. If no acceptable mode is found, a default mode is returned.. Optionally, returns a default mode. - * @param mode The mode to validate, ideally "#annotation-beside" or "#annotation-inline" - * @param leaveNull Alters the return value of this function. If false, the function will return the mode that was passed in or, in the case of null, the default mode. If true, the function will return null instead of using the default mode. - * @returns The validated mode, or null if leaveNull is true and no valid mode is found. - */ +// Returns the mode if it is 'beside' or 'inline', otherwise falls back to Beside. function validateMode(mode?: string) { if (mode === annotationMode.Beside || mode === annotationMode.Inline) return mode - // default to Beside else return annotationMode.Beside } @@ -24,15 +18,13 @@ export default function toggleAnnotation() { const annotationButtons = Array.from(document.querySelectorAll('.annotate-toggle button')) if (!annotationButtons.length) return - const cookie = validateMode(Cookies.get(ANNOTATE_MODE_COOKIE_NAME)) // will default to beside + const cookie = validateMode(Cookies.get(ANNOTATE_MODE_COOKIE_NAME)) displayAnnotationMode(annotationButtons, cookie) - // this loop adds event listeners for both the annotation buttons for (const annotationBtn of annotationButtons) { annotationBtn.addEventListener('click', (evt) => { evt.preventDefault() - // validate the annotation mode and set the cookie with the valid mode const validMode = validateMode(annotationBtn.getAttribute('value')!) Cookies.set(ANNOTATE_MODE_COOKIE_NAME, validMode!) sendEvent({ @@ -41,14 +33,15 @@ export default function toggleAnnotation() { preference_value: validMode, }) - // set and display the annotation mode setActive(annotationButtons, validMode) displayAnnotationMode(annotationButtons, validMode) }) } } -// sets the active element's aria-current, if no targetMode is set we default to "Beside", errors if it can't set either Beside or the passed in targetMode +// Sets aria-current on every button whose value matches the validated mode, and +// clears it from the rest. Missing or invalid modes validate to Beside. Throws if +// no button matches. function setActive(annotationButtons: Array, targetMode?: string) { const activeElements: Array = [] targetMode = validateMode(targetMode) @@ -69,7 +62,6 @@ function setActive(annotationButtons: Array, targetMode?: string) { return activeElements } -// displays the chosen annotation mode function displayAnnotationMode(annotationBtnItems: Array, targetMode?: string) { if (!targetMode || targetMode === annotationMode.Beside) { for (const el of annotationBtnItems) { diff --git a/src/frame/components/page-footer/Contribution.tsx b/src/frame/components/page-footer/Contribution.tsx index 5fdb93ab3243..0468a0dfd5f8 100644 --- a/src/frame/components/page-footer/Contribution.tsx +++ b/src/frame/components/page-footer/Contribution.tsx @@ -10,7 +10,7 @@ export const Contribution = () => { : 'https://github.com/github/docs' // Heading and body styling comes from the footer column rules in - // SupportSection.module.scss — the Docs 2026 design renders these as plain body + // SupportSection.module.scss. The Docs 2026 design renders these as plain body // text rather than a bold heading plus muted copy. return (
diff --git a/src/frame/components/page-footer/DocsFooter.module.scss b/src/frame/components/page-footer/DocsFooter.module.scss index 740cd196179c..bd9d633f43b8 100644 --- a/src/frame/components/page-footer/DocsFooter.module.scss +++ b/src/frame/components/page-footer/DocsFooter.module.scss @@ -1,7 +1,7 @@ // In dark mode the footer sits on the repo's canvas (#0d1117) while brand's // canvas-subtle comes from brand's own palette (#0f1511). Those are near-identical, // so the controls would lose their fill and read as bare outlines. Use Primer's -// surfaces there instead — they're built to contrast with this background. Selector +// surfaces there instead: they're built to contrast with this background. Selector // pattern follows octicon-table-optimization.scss. @mixin footer-control-surfaces-dark { --color-btn-bg: var(--color-canvas-subtle); @@ -21,8 +21,8 @@ // a visibly darker slab. --brand-footer-bg-color: var(--bgColor-default, var(--color-canvas-default)); - // The design draws all four footer controls — Yes, No, Make a contribution and - // Back to top — on brand's subtle canvas with a subtle border (#f2f5f3 on #d2d9d4, + // The design draws all four footer controls (Yes, No, Make a contribution and + // Back to top) on brand's subtle canvas with a subtle border (#f2f5f3 on #d2d9d4, // which is exactly what these brand tokens resolve to). // // Yes/No and the contribution CTA are Primer `.btn`s, so set the variables they @@ -48,7 +48,7 @@ } // The band above supplies the visual break, and the design puts a 2px emphasized - // rule directly beneath it — on the footer's own top edge. Brand's default here is + // rule directly beneath it, on the footer's own top edge. Brand's default here is // a much brighter green (scale-green-3), so this stays an explicit override. border-top: 2px solid var(--brand-color-text-emphasized); @@ -62,8 +62,8 @@ } // Help region: the columns own their padding, so the section adds none. Without - // this the columns sit 32px below the rule that introduces them, and — because the - // rules between rows are column borders — those rules would stop short of the + // this the columns sit 32px below the rule that introduces them, and because the + // rules between rows are column borders, those rules would stop short of the // footer edges instead of spanning it as the design shows. The inline inset moves // onto the columns in SupportSection.module.scss so content stays aligned with the // top and bottom bars. @@ -107,7 +107,7 @@ // Narrow: the design stacks the legal links above the copyright, and gives each its // own band separated by a rule that spans the footer. So the bottom section hands its -// padding to the two rows — the same move the help region makes — which lets the rule +// padding to the two rows, the same move the help region makes, which lets the rule // between them reach the edges while both rows stay inset and aligned with each other. @media (max-width: 767px) { .docsFooter > div:has(.bottomRow) > section { diff --git a/src/frame/components/page-footer/DocsFooter.tsx b/src/frame/components/page-footer/DocsFooter.tsx index 995040c7429a..d2c29cdbd782 100644 --- a/src/frame/components/page-footer/DocsFooter.tsx +++ b/src/frame/components/page-footer/DocsFooter.tsx @@ -16,8 +16,8 @@ import styles from './DocsFooter.module.scss' // The design puts the legal links in the *bottom* row beside the copyright. // MinimalFooter.Link children render in the top row instead, and the two rows live // in separate DOM subtrees so no amount of CSS moves one into the other. Passing the -// links through `copyrightStatement` — which accepts a ReactElement and renders in -// the bottom row — gets the designed layout without overriding brand internals. +// links through `copyrightStatement`, which accepts a ReactElement and renders in +// the bottom row, gets the designed layout without overriding brand internals. // It also sidesteps the component's hard cap of five links. // // Note `copyrightStatement` is rendered inside a , so everything here diff --git a/src/frame/components/page-footer/FooterDivider.module.scss b/src/frame/components/page-footer/FooterDivider.module.scss index fe72c7ba7aad..8abcc81481f6 100644 --- a/src/frame/components/page-footer/FooterDivider.module.scss +++ b/src/frame/components/page-footer/FooterDivider.module.scss @@ -17,7 +17,7 @@ // Scroll reveal. `.armed` is only applied by the component once it knows scripting is // running and the band is still below the fold, so the resting state above stays // visible for everyone else. Opacity and transform are compositor-only, so the rise -// does not reflow the footer beneath it — and MinimalFooter is positioned, so the +// does not reflow the footer beneath it. MinimalFooter is positioned, so the // band slides under it rather than over it. .armed { opacity: 0; diff --git a/src/frame/components/page-footer/FooterDivider.tsx b/src/frame/components/page-footer/FooterDivider.tsx index 4e5ad2202c9b..f592227610d2 100644 --- a/src/frame/components/page-footer/FooterDivider.tsx +++ b/src/frame/components/page-footer/FooterDivider.tsx @@ -7,7 +7,7 @@ import styles from './FooterDivider.module.scss' // Purely ornamental, so it carries no accessible name and is hidden from AT. // // It fades and rises into place the first time it scrolls into view. The band is -// only *armed* — that is, hidden — once we know scripting is running and it is still +// only *armed* (hidden) once we know scripting is running and it is still // below the fold, so it can never be left permanently invisible: without JavaScript, // with reduced motion, or on a page short enough that the footer is already on // screen, it just renders in place with no animation. diff --git a/src/frame/components/page-footer/Support.tsx b/src/frame/components/page-footer/Support.tsx index 2411831f0378..97978e67036e 100644 --- a/src/frame/components/page-footer/Support.tsx +++ b/src/frame/components/page-footer/Support.tsx @@ -7,7 +7,7 @@ import { useMainContext } from '@/frame/components/context/MainContext' import styles from './SupportSection.module.scss' // The footer's help column. Expert services and Blog moved here out of the legal -// strip in the Docs 2026 design — the design groups these with the other help +// strip in the Docs 2026 design, which groups these with the other help // destinations. // // Below the 2-column breakpoint the design collapses this into a disclosure. Rather diff --git a/src/frame/components/page-footer/SupportSection.module.scss b/src/frame/components/page-footer/SupportSection.module.scss index 452fc4e16c37..e674cc087c9d 100644 --- a/src/frame/components/page-footer/SupportSection.module.scss +++ b/src/frame/components/page-footer/SupportSection.module.scss @@ -1,6 +1,6 @@ // The footer help region: feedback, contribution, and support links. This lives in -// MinimalFooter's `centerComponent` slot, so it supplies its own column chrome — -// 24px padding and rules — rather than the page container it used to sit in. +// MinimalFooter's `centerComponent` slot, so it supplies its own column chrome +// (24px padding and rules) rather than the page container it used to sit in. // // The design reflows in three stages rather than simply narrowing three columns: // @@ -10,7 +10,7 @@ // narrow (<768) everything stacked, support collapsed into a disclosure // // MinimalFooter already draws a rule above this whole region, so the first row must -// not add one — two rules a hair apart read as a mistake. Only rows after the first +// not add one: two rules a hair apart read as a mistake. Only rows after the first // carry a top rule. $footer-rule: 1px solid @@ -24,8 +24,8 @@ $footer-rule: 1px solid .column { // Vertical padding is the design's 24px. The inline inset instead mirrors // MinimalFooter's own container padding (20px, 32px from 48rem up), because that - // container's padding was dropped for this section so the row rules — which are - // column borders — span the footer edge to edge. Keeping the same values here + // container's padding was dropped for this section so the row rules, which are + // column borders, span the footer edge to edge. Keeping the same values here // leaves column content aligned with the top and bottom bars. padding-block: var(--base-size-24); padding-inline: var(--base-size-20); @@ -34,7 +34,7 @@ $footer-rule: 1px solid color: var(--brand-color-text-default); // Survey and Contribution each render their own

. The design shows these as - // plain body text, not bold headings — restyled here rather than in those + // plain body text, not bold headings. Restyled here rather than in those // components so they stay free of footer-specific classes. Element selectors also // out-specify the Primer utility classes they carry. h3 { @@ -106,14 +106,14 @@ $footer-rule: 1px solid } // A second-row rule only makes sense when there is a first row above it. Any of - // these columns can be absent — site-policy and deprecated pages drop the survey, - // non-English drops the contribution CTA — and without this gate the surviving + // these columns can be absent: site-policy and deprecated pages drop the survey, + // non-English drops the contribution CTA. Without this gate the surviving // columns would draw a rule directly beneath MinimalFooter's own separator. .supportGrid:has(.surveyColumn) .column:not(.surveyColumn) { border-top: $footer-rule; } - // Divider between the two columns that share a row — keyed to position rather than + // Divider between the two columns that share a row, keyed to position rather than // to which component it is, so it lands correctly whichever columns render. .column:not(.surveyColumn) + .column { border-left: $footer-rule; diff --git a/src/frame/components/page-footer/SupportSection.tsx b/src/frame/components/page-footer/SupportSection.tsx index f84de6924f08..282b1ee61972 100644 --- a/src/frame/components/page-footer/SupportSection.tsx +++ b/src/frame/components/page-footer/SupportSection.tsx @@ -11,7 +11,7 @@ import { useTranslation } from '@/languages/components/useTranslation' import styles from './SupportSection.module.scss' // Renders inside MinimalFooter's `centerComponent` slot, so it no longer owns a page -// container or a section heading — the footer supplies that chrome. +// container or a section heading. The footer supplies that chrome. // // Columns carry their own class rather than relying on nth-child, because any of the // three can be hidden (site-policy pages drop the survey, non-English drops the diff --git a/src/frame/components/page-header/BreadcrumbsScroller.module.scss b/src/frame/components/page-header/BreadcrumbsScroller.module.scss index cfee24334cdf..daa100f3a333 100644 --- a/src/frame/components/page-header/BreadcrumbsScroller.module.scss +++ b/src/frame/components/page-header/BreadcrumbsScroller.module.scss @@ -14,7 +14,7 @@ // The chevrons overlay the ends of the scroll area (absolutely positioned) // rather than sitting in the flex flow. This keeps the scroll area full-width -// and constant — so its scrollable range never shifts when a chevron toggles — +// and constant, so its scrollable range never shifts when a chevron toggles, // while a hidden chevron leaves NO reserved gap at that edge. Each chevron // carries the secondary bar's own background so a crumb scrolling underneath is // masked rather than showing through the icon. @@ -72,11 +72,11 @@ // Constant 16px on both ends: keeps a crumb off the border at the scroll // extremes, and mid-scroll the chevron overlays the end of the trail (its // solid background masks whatever passes underneath). Keeping this padding - // fixed — never toggled with the chevrons — means the scroll content's width + // fixed, never toggled with the chevrons, means the scroll content's width // never changes, so toggling a chevron can't reflow the trail or nudge the // scroll position (no hop, no mount bounce, no click-lands-short). padding: 0 16px; - scrollbar-width: none; // Firefox — hide the horizontal scrollbar + scrollbar-width: none; // Firefox: hide the horizontal scrollbar -ms-overflow-style: none; &::-webkit-scrollbar { @@ -85,7 +85,7 @@ // Make the brand Breadcrumbs