From 1c8a215f1a36f84bbdf8ceba170ab47c5b08f895 Mon Sep 17 00:00:00 2001 From: Kevin Heis Date: Thu, 16 Oct 2025 09:20:36 -0700 Subject: [PATCH] Migrate 4 JavaScript files to TypeScript (#57982) --- content/README.md | 2 +- ...sing-markdown-and-liquid-in-github-docs.md | 2 +- ...uid-versioning.js => liquid-versioning.ts} | 82 ++++++---- src/content-render/unified/processor.ts | 38 +++-- ...-local-links.js => rewrite-local-links.ts} | 150 +++++++++++------- ...{build-changelog.js => build-changelog.ts} | 97 ++++++++--- .../{oneof-handling.js => oneof-handling.ts} | 54 +++++-- 7 files changed, 287 insertions(+), 138 deletions(-) rename src/content-linter/lib/linting-rules/{liquid-versioning.js => liquid-versioning.ts} (76%) rename src/content-render/unified/{rewrite-local-links.js => rewrite-local-links.ts} (67%) rename src/graphql/tests/{build-changelog.js => build-changelog.ts} (76%) rename src/webhooks/tests/{oneof-handling.js => oneof-handling.ts} (83%) diff --git a/content/README.md b/content/README.md index eca61d0b3205..1498e8ab17bc 100644 --- a/content/README.md +++ b/content/README.md @@ -375,7 +375,7 @@ Links to docs in the `docs-internal` repository must start with a product ID (li Image paths must start with `/assets` and contain the entire filepath including the file extension. For example, `/assets/images/help/settings/settings-account-delete.png`. -The links to Markdown pages undergo some transformations on the server side to match the current page's language and version. The handling for these transformations lives in [`src/content-render/unified/rewrite-local-links.js`](/src/content-render/unified/rewrite-local-links.js). +The links to Markdown pages undergo some transformations on the server side to match the current page's language and version. The handling for these transformations lives in [`src/content-render/unified/rewrite-local-links.ts`](/src/content-render/unified/rewrite-local-links.ts). For example, if you include the following link in a content file: diff --git a/content/contributing/writing-for-github-docs/using-markdown-and-liquid-in-github-docs.md b/content/contributing/writing-for-github-docs/using-markdown-and-liquid-in-github-docs.md index 2457af7c509e..1df97a512bf1 100644 --- a/content/contributing/writing-for-github-docs/using-markdown-and-liquid-in-github-docs.md +++ b/content/contributing/writing-for-github-docs/using-markdown-and-liquid-in-github-docs.md @@ -442,7 +442,7 @@ Links to docs in the `docs` repository must start with a product ID (like `/acti Image paths must start with `/assets` and contain the entire filepath including the file extension. For example, `/assets/images/help/settings/settings-account-delete.png`. -The links to Markdown pages undergo some transformations on the server side to match the current page's language and version. The handling for these transformations lives in [`lib/render-content/plugins/rewrite-local-links`](https://github.com/github/docs/blob/main/src/content-render/unified/rewrite-local-links.js). +The links to Markdown pages undergo some transformations on the server side to match the current page's language and version. The handling for these transformations lives in [`lib/render-content/plugins/rewrite-local-links`](https://github.com/github/docs/blob/main/src/content-render/unified/rewrite-local-links.ts). For example, if you include the following link in a content file: diff --git a/src/content-linter/lib/linting-rules/liquid-versioning.js b/src/content-linter/lib/linting-rules/liquid-versioning.ts similarity index 76% rename from src/content-linter/lib/linting-rules/liquid-versioning.js rename to src/content-linter/lib/linting-rules/liquid-versioning.ts index 1f41f5df6865..7a1e0e10b2aa 100644 --- a/src/content-linter/lib/linting-rules/liquid-versioning.js +++ b/src/content-linter/lib/linting-rules/liquid-versioning.ts @@ -1,5 +1,6 @@ import semver from 'semver' import { TokenKind } from 'liquidjs' +// @ts-ignore - markdownlint-rule-helpers doesn't provide TypeScript declarations import { addError } from 'markdownlint-rule-helpers' import { getRange, addFixErrorDetail } from '../helpers/utils' @@ -9,9 +10,17 @@ import allowedVersionOperators from '@/content-render/liquid/ifversion-supported import { getDeepDataByLanguage } from '@/data-directory/lib/get-data' import getApplicableVersions from '@/versions/lib/get-applicable-versions' import { getLiquidTokens, getPositionData } from '../helpers/liquid-utils' +import type { RuleParams, RuleErrorCallback } from '@/content-linter/types' -const allShortnames = Object.keys(allVersionShortnames) -const getAllPossibleVersionNames = memoize(() => { +interface Feature { + versions: Record + [key: string]: any +} + +type AllFeatures = Record + +const allShortnames: string[] = Object.keys(allVersionShortnames) +const getAllPossibleVersionNames = memoize((): Set => { // This function might appear "slow" but it's wrapped in a memoizer // so it's only every executed once for all files that the // Liquid linting rule functions on. @@ -21,20 +30,22 @@ const getAllPossibleVersionNames = memoize(() => { return new Set([...Object.keys(getAllFeatures()), ...allShortnames]) }) -const getAllFeatures = memoize(() => getDeepDataByLanguage('features', 'en', process.env.ROOT)) +const getAllFeatures = memoize( + (): AllFeatures => getDeepDataByLanguage('features', 'en', process.env.ROOT) as AllFeatures, +) -const allVersionNames = Object.keys(allVersions) +const allVersionNames: string[] = Object.keys(allVersions) -function isAllVersions(versions) { +function isAllVersions(versions: string[]): boolean { if (versions.length === allVersionNames.length) { return versions.every((version) => allVersionNames.includes(version)) } return false } -function memoize(func) { - let cached = null - return () => { +function memoize(func: () => T): () => T { + let cached: T | null = null + return (): T => { if (!cached) { cached = func() } @@ -47,14 +58,14 @@ export const liquidIfTags = { description: 'Liquid `ifversion` tags should be used instead of `if` tags when the argument is a valid version', tags: ['liquid', 'versioning'], - function: (params, onError) => { + function: (params: RuleParams, onError: RuleErrorCallback) => { const content = params.lines.join('\n') const tokens = getLiquidTokens(content).filter( (token) => token.kind === TokenKind.Tag && token.name === 'if' && - token.args.split(/\s+/).some((arg) => getAllPossibleVersionNames().has(arg)), + token.args.split(/\s+/).some((arg: string) => getAllPossibleVersionNames().has(arg)), ) for (const token of tokens) { @@ -77,7 +88,7 @@ export const liquidIfVersionTags = { names: ['GHD020', 'liquid-ifversion-tags'], description: 'Liquid `ifversion` tags should contain valid version names as arguments', tags: ['liquid', 'versioning'], - function: (params, onError) => { + function: (params: RuleParams, onError: RuleErrorCallback) => { const content = params.lines.join('\n') const tokens = getLiquidTokens(content) .filter((token) => token.kind === TokenKind.Tag) @@ -105,10 +116,10 @@ export const liquidIfVersionTags = { }, } -function validateIfversionConditionals(cond, possibleVersionNames) { - const validateVersion = (version) => possibleVersionNames.has(version) +function validateIfversionConditionals(cond: string, possibleVersionNames: Set): string[] { + const validateVersion = (version: string): boolean => possibleVersionNames.has(version) - const errors = [] + const errors: string[] = [] // Where `cond` is an array of strings, where each string may have one of the following space-separated formats: // * Length 1: `` (example: `fpt`) @@ -150,14 +161,16 @@ function validateIfversionConditionals(cond, possibleVersionNames) { if (strParts.length === 3) { const [version, operator, release] = strParts const hasSemanticVersioning = Object.values(allVersions).some( - (v) => (v.hasNumberedReleases || v.internalLatestRelease) && v.shortName === version, + (v) => v.hasNumberedReleases && v.shortName === version, ) if (!hasSemanticVersioning) { errors.push( `Found "${version}" inside "${cond}" with a "${operator}" operator, but "${version}" does not support semantic comparisons"`, ) } - if (!allowedVersionOperators.includes(operator)) { + // Using 'as any' because the operator is a runtime string value that we validate, + // but the allowedVersionOperators array has a more specific type that TypeScript can't infer + if (!allowedVersionOperators.includes(operator as any)) { errors.push( `Found a "${operator}" operator inside "${cond}", but "${operator}" is not supported`, ) @@ -187,7 +200,10 @@ function validateIfversionConditionals(cond, possibleVersionNames) { // The reason this function is exported is because it's sufficiently // complex that it needs to be tested in isolation. -export function validateIfversionConditionalsVersions(cond, allFeatures) { +export function validateIfversionConditionalsVersions( + cond: string, + allFeatures: AllFeatures, +): string[] { // Suppose the cond is `ghes >3.1 or some-cool-feature` we need to open // that `some-cool-feature` and if that has `{ghes:'>3.0', ghec:'*', fpt:'*'}` // then *combined* versions will be `{ghes:'>3.0', ghec:'*', fpt:'*'}`. @@ -198,9 +214,9 @@ export function validateIfversionConditionalsVersions(cond, allFeatures) { return [] } - const errors = [] - const versions = {} - let hasFutureLessThan = false + const errors: string[] = [] + const versions: Record = {} + let hasFutureLessThan: boolean = false for (const part of cond.split(/\sor\s/)) { // For example `fpt or not ghec` or `not ghes or ghec or not fpt` if (/(^|\s)not(\s|$)/.test(part)) { @@ -223,7 +239,7 @@ export function validateIfversionConditionalsVersions(cond, allFeatures) { } } - const applicableVersions = [] + const applicableVersions: string[] = [] try { applicableVersions.push(...getApplicableVersions(versions)) } catch { @@ -238,12 +254,16 @@ export function validateIfversionConditionalsVersions(cond, allFeatures) { return errors } -function getVersionsObject(part, allFeatures) { - const versions = {} +function getVersionsObject(part: string, allFeatures: AllFeatures): Record { + const versions: Record = {} if (part in allFeatures) { for (const [shortName, version] of Object.entries(allFeatures[part].versions)) { - const versionOperator = - version in allFeatures ? getVersionsObject(version, allFeatures) : version + // Using 'as any' for recursive getVersionsObject call because it can return either + // a string or a nested Record, but we flatten it to string for this context + const versionOperator: string = + version in allFeatures + ? (getVersionsObject(version, allFeatures) as any) + : (version as string) if (shortName in versions) { versions[shortName] = lowestVersion(versionOperator, versions[shortName]) } else { @@ -254,19 +274,23 @@ function getVersionsObject(part, allFeatures) { versions[part] = '*' } else if (allShortnames.some((v) => part.startsWith(v))) { const shortNamed = allShortnames.find((v) => part.startsWith(v)) - const rest = part.replace(shortNamed, '').trim() - versions[shortNamed] = rest + if (shortNamed) { + const rest = part.replace(shortNamed, '').trim() + versions[shortNamed] = rest + } } else { throw new Error(`The version '${part}' is neither a short version name or a feature name`) } return versions } -function lowestVersion(version1, version2) { +function lowestVersion(version1: string, version2: string): string { if (version1 === '*' || version2 === '*') { return '*' } - if (semver.lt(semver.minVersion(version1), semver.minVersion(version2))) { + const min1 = semver.minVersion(version1) + const min2 = semver.minVersion(version2) + if (min1 && min2 && semver.lt(min1, min2)) { return version1 } else { return version2 diff --git a/src/content-render/unified/processor.ts b/src/content-render/unified/processor.ts index 8b51bba4e119..39e9d1ff49f0 100644 --- a/src/content-render/unified/processor.ts +++ b/src/content-render/unified/processor.ts @@ -40,7 +40,9 @@ export function createProcessor(context: Context): UnifiedProcessor { .use(gfm) // Markdown AST below vvv .use(parseInfoString) - .use(rewriteLocalLinks, context) + // Using 'as any' because rewriteLocalLinks is a factory function that takes context + // and returns a transformer, but TypeScript's unified plugin types don't handle this pattern + .use(rewriteLocalLinks as any, context) .use(emoji) // Markdown AST above ^^^ .use(remark2rehype, { allowDangerousHtml: true }) @@ -87,20 +89,28 @@ export function createProcessor(context: Context): UnifiedProcessor { } export function createMarkdownOnlyProcessor(context: Context): UnifiedProcessor { - return unified() - .use(remarkParse) - .use(gfm) - .use(rewriteLocalLinks, context) - .use(remarkStringify) as UnifiedProcessor + return ( + unified() + .use(remarkParse) + .use(gfm) + // Using 'as any' because rewriteLocalLinks is a factory function that takes context + // and returns a transformer, but TypeScript's unified plugin types don't handle this pattern + .use(rewriteLocalLinks as any, context) + .use(remarkStringify) as UnifiedProcessor + ) } export function createMinimalProcessor(context: Context): UnifiedProcessor { - return unified() - .use(remarkParse) - .use(gfm) - .use(rewriteLocalLinks, context) - .use(remark2rehype, { allowDangerousHtml: true }) - .use(slug) - .use(raw) - .use(html) as UnifiedProcessor + return ( + unified() + .use(remarkParse) + .use(gfm) + // Using 'as any' because rewriteLocalLinks is a factory function that takes context + // and returns a transformer, but TypeScript's unified plugin types don't handle this pattern + .use(rewriteLocalLinks as any, context) + .use(remark2rehype, { allowDangerousHtml: true }) + .use(slug) + .use(raw) + .use(html) as UnifiedProcessor + ) } diff --git a/src/content-render/unified/rewrite-local-links.js b/src/content-render/unified/rewrite-local-links.ts similarity index 67% rename from src/content-render/unified/rewrite-local-links.js rename to src/content-render/unified/rewrite-local-links.ts index 795abd0605b8..2e942c355659 100644 --- a/src/content-render/unified/rewrite-local-links.js +++ b/src/content-render/unified/rewrite-local-links.ts @@ -1,7 +1,6 @@ -// When updating this file to typescript, -// update links in content/contributing as well - import path from 'path' +import type { Link, LinkReference, Definition, Text } from 'mdast' +import type { Node } from 'unist' import stripAnsi from 'strip-ansi' import { visit } from 'unist-util-visit' @@ -15,6 +14,7 @@ import { allVersions } from '@/versions/lib/all-versions' import removeFPTFromPath from '@/versions/lib/remove-fpt-from-path' import readJsonFile from '@/frame/lib/read-json-file' import findPage from '@/frame/lib/find-page' +import type { Context, Page } from '@/types' const isProd = process.env.NODE_ENV === 'production' @@ -26,15 +26,18 @@ const LOG_ERROR_ANNOTATIONS = CI || Boolean(JSON.parse(process.env.LOG_ERROR_ANNOTATIONS || 'false')) const supportedPlans = new Set(Object.values(allVersions).map((v) => v.plan)) -const externalRedirects = readJsonFile('./src/redirects/lib/external-sites.json') +const externalRedirects = readJsonFile('./src/redirects/lib/external-sites.json') as Record< + string, + string +> // The reason we "memoize" which lines we've logged is because the same // error might happen more than once in the whole space of one CI run. -const _logged = new Set() +const _logged = new Set() // Printing this to stdout in this format, will automatically be picked up // by Actions to turn that into a PR inline annotation. -function logError(file, line, message, title = 'Error') { +function logError(file: string, line: number, message: string, title = 'Error') { if (LOG_ERROR_ANNOTATIONS) { const hash = `${file}:${line}:${message}` if (_logged.has(hash)) return @@ -56,21 +59,26 @@ const AUTOTITLE = /^\s*AUTOTITLE\s*$/ // which we use to know that we need to fall back to English. export class TitleFromAutotitleError extends Error {} -// Matches any link nodes with an href that starts with `/` -const matcherInternalLinks = (node) => node.type === 'link' && node.url && node.url.startsWith('/') +interface LinkNode extends Link { + originalHref?: string + _originalHref?: string +} -// Matches any link nodes with an href that starts with `#` -const matcherAnchorLinks = (node) => node.type === 'link' && node.url && node.url.startsWith('#') +interface NodeToProcess { + url: string + child: Text + originalHref?: string +} // Content authors write links like `/some/article/path`, but they need to be // rewritten on the fly to match the current language and page version -export default function rewriteLocalLinks(context) { - const { currentLanguage, autotitleLanguage, currentVersion } = context - // There's no languageCode or version passed, so nothing to do - if (!currentLanguage || !currentVersion) return - - return async function (tree) { - const nodes = [] +export default function rewriteLocalLinks(context?: Context) { + return async function (tree: Node): Promise { + if (!context) return + const { currentLanguage, autotitleLanguage, currentVersion } = context + // There's no languageCode or version passed, so nothing to do + if (!currentLanguage || !currentVersion) return + const nodes: NodeToProcess[] = [] // For links using linkReference and definition, we must // first get the list of definitions and later resolve @@ -83,20 +91,24 @@ export default function rewriteLocalLinks(context) { // [Some link](/abc/123) // And then we can treat it like a regular 'link'; // see https://github.github.com/gfm/#link-reference-definitions for spec - const definitions = new Map() - visit(tree, 'definition', (node) => { - definitions.set(node.identifier, node) + const definitions = new Map() + visit(tree, 'definition', (node: Node) => { + const defNode = node as Definition + definitions.set(defNode.identifier, defNode) }) - visit(tree, 'linkReference', (node) => { - const definition = definitions.get(node.identifier) + visit(tree, 'linkReference', (node: Node) => { + const linkRefNode = node as LinkReference + const definition = definitions.get(linkRefNode.identifier) if (definition) { // Replace the LinkReference node with a Link node - node.type = 'link' - node.url = definition.url - node.title = definition.title + // Using 'as any' because we're mutating the node type at runtime, + // which TypeScript doesn't allow as LinkReference and Link are incompatible types + ;(linkRefNode as any).type = 'link' + ;(linkRefNode as any).url = definition.url + ;(linkRefNode as any).title = definition.title } else { - console.warn(`Definition not found for identifier: ${node.identifier}`) + console.warn(`Definition not found for identifier: ${linkRefNode.identifier}`) } }) @@ -105,21 +117,37 @@ export default function rewriteLocalLinks(context) { } } -async function processTree(tree, language, version, nodes, context) { +async function processTree( + tree: Node, + language: string, + version: string, + nodes: NodeToProcess[], + context: Context, +) { // internal links begin with `/something` - visit(tree, matcherInternalLinks, (node) => { - processLinkNode(node, language, version, nodes) + visit(tree, 'link', (node: Node) => { + const linkNode = node as Link + if (linkNode.url && linkNode.url.startsWith('/')) { + processLinkNode(linkNode, language, version, nodes) + } }) if (!isProd) { // handles anchor links - visit(tree, matcherAnchorLinks, (node) => { - for (const child of node.children || []) { - if (child.value && AUTOTITLE.test(child.value)) { - throw new Error( - `Found anchor link with text AUTOTITLE ('${node.url}'). ` + - 'Update the anchor link with text that is not AUTOTITLE.', - ) + visit(tree, 'link', (node: Node) => { + const linkNode = node as Link + if (linkNode.url && linkNode.url.startsWith('#')) { + for (const child of linkNode.children || []) { + if ( + child.type === 'text' && + (child as Text).value && + AUTOTITLE.test((child as Text).value) + ) { + throw new Error( + `Found anchor link with text AUTOTITLE ('${linkNode.url}'). ` + + 'Update the anchor link with text that is not AUTOTITLE.', + ) + } } } }) @@ -128,25 +156,27 @@ async function processTree(tree, language, version, nodes, context) { // nodes[] contains all the link nodes that need new titles // and now we call to get those titles await Promise.all( - nodes.map(({ url, child, originalHref }) => + nodes.map(({ url, child, originalHref }: NodeToProcess) => getNewTitleSetter(child, url, context, originalHref), ), ) } -function processLinkNode(node, language, version, nodes) { - const newHref = getNewHref(node, language, version) +function processLinkNode(node: Link, language: string, version: string, nodes: NodeToProcess[]) { + const linkNode = node as LinkNode + const newHref = getNewHref(linkNode, language, version) if (newHref) { - node.originalHref = node.url - node.url = newHref + linkNode.originalHref = linkNode.url + linkNode.url = newHref } - for (const child of node.children) { - if (child.value) { - if (AUTOTITLE.test(child.value)) { + for (const child of linkNode.children) { + if (child.type === 'text' && (child as Text).value) { + const textChild = child as Text + if (AUTOTITLE.test(textChild.value)) { nodes.push({ - url: node.url, - child, - originalHref: node._originalHref, + url: linkNode.url, + child: textChild, + originalHref: linkNode._originalHref, }) } else if ( // This means CI and local dev @@ -155,13 +185,14 @@ function processLinkNode(node, language, version, nodes) { language === 'en' ) { // Throw if the link text *almost* is AUTOTITLE + const textChild = child as Text if ( - child.value.toUpperCase() === 'AUTOTITLE' || - distance(child.value.toUpperCase(), 'AUTOTITLE') <= 2 + textChild.value.toUpperCase() === 'AUTOTITLE' || + distance(textChild.value.toUpperCase(), 'AUTOTITLE') <= 2 ) { throw new Error( - `Found link text '${child.value}', expected 'AUTOTITLE'. ` + - `Find the mention of the link text '${child.value}' and change it to 'AUTOTITLE'. Case matters.`, + `Found link text '${textChild.value}', expected 'AUTOTITLE'. ` + + `Find the mention of the link text '${textChild.value}' and change it to 'AUTOTITLE'. Case matters.`, ) } } @@ -169,26 +200,31 @@ function processLinkNode(node, language, version, nodes) { } } -async function getNewTitleSetter(child, href, context, originalHref) { +async function getNewTitleSetter( + child: Text, + href: string, + context: Context, + originalHref?: string, +) { child.value = await getNewTitle(href, context, child, originalHref) } -async function getNewTitle(href, context, child, originalHref) { - const page = findPage(href, context.pages, context.redirects) +async function getNewTitle(href: string, context: Context, child: Text, originalHref?: string) { + const page = findPage(href, context.pages, context.redirects) as Page | undefined if (!page) { // The child.position.start.line is 1-based and already represents the line number // in the original file (including frontmatter), so no offset adjustment is needed - const line = child.position.start.line + const line = child.position?.start.line || 1 const linkText = originalHref || href const message = `The link '${linkText}' could not be resolved in one or more versions of the documentation. Make sure that this link can be reached from all versions of the documentation it appears in. (Line: ${line})` - logError(context.page.fullPath, line, message, 'Link Resolution Error') + logError(context.page!.fullPath, line, message, 'Link Resolution Error') throw new TitleFromAutotitleError(message) } return await page.renderProp('title', context, { textOnly: true }) } -function getNewHref(node, languageCode, version) { +function getNewHref(node: LinkNode, languageCode: string, version: string): string | undefined { const { url } = node // Exceptions to link rewriting if (url.startsWith('/assets')) return diff --git a/src/graphql/tests/build-changelog.js b/src/graphql/tests/build-changelog.ts similarity index 76% rename from src/graphql/tests/build-changelog.js rename to src/graphql/tests/build-changelog.ts index 2dd5c98609de..2c050a05291b 100644 --- a/src/graphql/tests/build-changelog.js +++ b/src/graphql/tests/build-changelog.ts @@ -14,10 +14,46 @@ import { } from '../scripts/build-changelog' import readJsonFile from '@/frame/lib/read-json-file' -const expectedChangelogEntry = readJsonFile('src/graphql/tests/fixtures/changelog-entry.json') -const expectedUpdatedChangelogFile = readJsonFile( +interface Preview { + title: string + description: string + toggled_by: string + announcement: any + updates: any + toggled_on: string[] + owning_teams: string[] +} + +interface UpcomingChange { + location: string + description: string + date: string +} + +interface ChangelogEntry { + [key: string]: any +} + +interface IgnoredChange { + type: string + [key: string]: any +} + +interface IgnoredChangesSummary { + totalCount: number + typeCount: number + types: Array<{ + type: string + count: number + }> +} + +const expectedChangelogEntry: ChangelogEntry = readJsonFile( + 'src/graphql/tests/fixtures/changelog-entry.json', +) as ChangelogEntry +const expectedUpdatedChangelogFile: ChangelogEntry[] = readJsonFile( 'src/graphql/tests/fixtures/updated-changelog-file.json', -) +) as ChangelogEntry[] describe('creating a changelog from old schema and new schema', () => { afterEach(() => { @@ -44,7 +80,13 @@ describe('creating a changelog from old schema and new schema', () => { // This should generate TypeDescriptionAdded change type // which should be silently ignored if not in CHANGES_TO_REPORT - const entry = await createChangelogEntry(oldSchemaString, newSchemaString, [], [], []) + const entry: ChangelogEntry | null = await createChangelogEntry( + oldSchemaString, + newSchemaString, + [], + [], + [], + ) // Should return null since TypeDescriptionAdded is not in CHANGES_TO_REPORT // and will be silently ignored without throwing an error @@ -72,7 +114,13 @@ describe('creating a changelog from old schema and new schema', () => { // This should generate DirectiveUsage* change types that are not in CHANGES_TO_REPORT // The system should silently ignore these and not throw errors - const entry = await createChangelogEntry(oldSchemaString, newSchemaString, [], [], []) + const entry: ChangelogEntry | null = await createChangelogEntry( + oldSchemaString, + newSchemaString, + [], + [], + [], + ) // Should return null since directive usage changes are typically ignored expect(entry).toBeNull() @@ -123,14 +171,15 @@ describe('creating a changelog from old schema and new schema', () => { - Query.previewField owning_teams: - '@github/engineering' -`) +`) as Preview[] const oldUpcomingChanges = yaml.load(` upcoming_changes: - location: EnterprisePendingCollaboratorEdge.isUnlicensed description: '\`isUnlicensed\` will be removed.' date: '2021-01-01T00:00:00+00:00' -`).upcoming_changes +`) as { upcoming_changes: UpcomingChange[] } + const oldUpcomingChangesArray: UpcomingChange[] = oldUpcomingChanges.upcoming_changes const newUpcomingChanges = yaml.load(` upcoming_changes: @@ -140,14 +189,15 @@ upcoming_changes: - location: EnterprisePendingCollaboratorEdge.isUnlicensed description: '\`isUnlicensed\` will be removed.' date: '2021-01-01T00:00:00+00:00' -`).upcoming_changes +`) as { upcoming_changes: UpcomingChange[] } + const newUpcomingChangesArray: UpcomingChange[] = newUpcomingChanges.upcoming_changes - const entry = await createChangelogEntry( + const entry: ChangelogEntry | null = await createChangelogEntry( oldSchemaString, newSchemaString, previews, - oldUpcomingChanges, - newUpcomingChanges, + oldUpcomingChangesArray, + newUpcomingChangesArray, ) expect(entry).toEqual(expectedChangelogEntry) }) @@ -158,7 +208,13 @@ upcoming_changes: i: Int! }` - const nullEntry = await createChangelogEntry(schemaString, schemaString, [], [], []) + const nullEntry: ChangelogEntry | null = await createChangelogEntry( + schemaString, + schemaString, + [], + [], + [], + ) expect(nullEntry).toBeNull() }) }) @@ -189,14 +245,14 @@ describe('updating the changelog file', () => { const testTargetPath = 'src/graphql/tests/fixtures/example-changelog.json' const previousContents = await fs.readFile(testTargetPath) - const exampleEntry = { someStuff: true } + const exampleEntry: ChangelogEntry = { someStuff: true } const expectedDate = '2020-11-20' MockDate.set(expectedDate) prependDatedEntry(exampleEntry, testTargetPath) - const newContents = await fs.readFile(testTargetPath, 'utf8') + const newContents: string = await fs.readFile(testTargetPath, 'utf8') // reset the file: - await fs.writeFile(testTargetPath, previousContents) + await fs.writeFile(testTargetPath, previousContents.toString()) expect(exampleEntry).toEqual({ someStuff: true, date: expectedDate }) expect(JSON.parse(newContents)).toEqual(expectedUpdatedChangelogFile) @@ -223,7 +279,7 @@ describe('ignored changes tracking', () => { // This should generate a TypeDescriptionAdded change type that gets ignored await createChangelogEntry(oldSchemaString, newSchemaString, [], [], []) - const ignoredChanges = getLastIgnoredChanges() + const ignoredChanges: IgnoredChange[] = getLastIgnoredChanges() expect(ignoredChanges.length).toBe(1) expect(ignoredChanges[0].type).toBe('TYPE_DESCRIPTION_ADDED') }) @@ -250,10 +306,11 @@ describe('ignored changes tracking', () => { const summary = getIgnoredChangesSummary() expect(summary).toBeTruthy() - expect(summary.totalCount).toBe(2) - expect(summary.typeCount).toBe(1) - expect(summary.types[0].type).toBe('DIRECTIVE_USAGE_FIELD_DEFINITION_ADDED') - expect(summary.types[0].count).toBe(2) + const typedSummary = summary as IgnoredChangesSummary + expect(typedSummary.totalCount).toBe(2) + expect(typedSummary.typeCount).toBe(1) + expect(typedSummary.types[0].type).toBe('DIRECTIVE_USAGE_FIELD_DEFINITION_ADDED') + expect(typedSummary.types[0].count).toBe(2) }) test('returns null summary when no changes ignored', async () => { diff --git a/src/webhooks/tests/oneof-handling.js b/src/webhooks/tests/oneof-handling.ts similarity index 83% rename from src/webhooks/tests/oneof-handling.js rename to src/webhooks/tests/oneof-handling.ts index 41a28826ef1d..ddc52ba49080 100644 --- a/src/webhooks/tests/oneof-handling.js +++ b/src/webhooks/tests/oneof-handling.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from 'vitest' -import { getBodyParams } from '../../rest/scripts/utils/get-body-params' +import { getBodyParams, type TransformedParam } from '../../rest/scripts/utils/get-body-params' describe('oneOf handling in webhook parameters', () => { test('should handle oneOf fields correctly for secret_scanning_alert_location details', async () => { @@ -122,15 +122,19 @@ describe('oneOf handling in webhook parameters', () => { }, } - const result = await getBodyParams(mockSchema, true) + const result: TransformedParam[] = await getBodyParams(mockSchema, true) // Find the location parameter - const locationParam = result.find((param) => param.name === 'location') + const locationParam: TransformedParam | undefined = result.find( + (param) => param.name === 'location', + ) expect(locationParam).toBeDefined() expect(locationParam?.childParamsGroups).toBeDefined() // Find the details parameter within location - const detailsParam = locationParam?.childParamsGroups?.find((param) => param.name === 'details') + const detailsParam: TransformedParam | undefined = locationParam?.childParamsGroups?.find( + (param) => param.name === 'details', + ) expect(detailsParam).toBeDefined() expect(detailsParam?.type).toBe('object') @@ -139,11 +143,19 @@ describe('oneOf handling in webhook parameters', () => { expect(detailsParam?.childParamsGroups?.length).toBeGreaterThan(1) // Check that we have the expected oneOf objects - const childParams = detailsParam?.childParamsGroups || [] - const commitParam = childParams.find((param) => param.name === 'commit') - const issueTitleParam = childParams.find((param) => param.name === 'issue_title') - const issueBodyParam = childParams.find((param) => param.name === 'issue_body') - const issueCommentParam = childParams.find((param) => param.name === 'issue_comment') + const childParams: TransformedParam[] = detailsParam?.childParamsGroups || [] + const commitParam: TransformedParam | undefined = childParams.find( + (param) => param.name === 'commit', + ) + const issueTitleParam: TransformedParam | undefined = childParams.find( + (param) => param.name === 'issue_title', + ) + const issueBodyParam: TransformedParam | undefined = childParams.find( + (param) => param.name === 'issue_body', + ) + const issueCommentParam: TransformedParam | undefined = childParams.find( + (param) => param.name === 'issue_comment', + ) expect(commitParam).toBeDefined() expect(commitParam?.description).toContain("commit' secret scanning location type") @@ -193,9 +205,11 @@ describe('oneOf handling in webhook parameters', () => { }, } - const result = await getBodyParams(mockSchemaWithoutTitles, true) + const result: TransformedParam[] = await getBodyParams(mockSchemaWithoutTitles, true) - const detailsParam = result.find((param) => param.name === 'details') + const detailsParam: TransformedParam | undefined = result.find( + (param) => param.name === 'details', + ) expect(detailsParam).toBeDefined() expect(detailsParam?.childParamsGroups?.length).toBe(2) @@ -244,18 +258,26 @@ describe('oneOf handling in webhook parameters', () => { }, } - const result = await getBodyParams(mockNestedOneOfSchema, true) + const result: TransformedParam[] = await getBodyParams(mockNestedOneOfSchema, true) - const wrapperParam = result.find((param) => param.name === 'wrapper') + const wrapperParam: TransformedParam | undefined = result.find( + (param) => param.name === 'wrapper', + ) expect(wrapperParam).toBeDefined() - const innerParam = wrapperParam?.childParamsGroups?.find((param) => param.name === 'inner') + const innerParam: TransformedParam | undefined = wrapperParam?.childParamsGroups?.find( + (param) => param.name === 'inner', + ) expect(innerParam).toBeDefined() expect(innerParam?.oneOfObject).toBe(true) expect(innerParam?.childParamsGroups?.length).toBe(2) - const optionA = innerParam?.childParamsGroups?.find((param) => param.name === 'option_a') - const optionB = innerParam?.childParamsGroups?.find((param) => param.name === 'option_b') + const optionA: TransformedParam | undefined = innerParam?.childParamsGroups?.find( + (param) => param.name === 'option_a', + ) + const optionB: TransformedParam | undefined = innerParam?.childParamsGroups?.find( + (param) => param.name === 'option_b', + ) expect(optionA).toBeDefined() expect(optionA?.description).toContain('Option A description')