From 3fd9efe4f60593cbab9faf0a431fab4cc3aa8c58 Mon Sep 17 00:00:00 2001 From: Joey Farina Date: Sat, 17 Oct 2026 16:38:57 -0700 Subject: [PATCH 1/3] feat(cli): check each authored doc against its published authoring type A foundation check, `checkDocTypes`, reads each doc file's value (the export its root reads) and checks it the way TypeScript checks `value satisfies Type` against the type `@astryxdesign/cli/authoring` publishes for its kind, with the project's own TypeScript. Each unknown field, missing required field, or value outside its field's type is a warning naming the doc, line, field, and a fix. A value outside its type, an unknown field, or a nested object missing a field is set aside so the same run reports what it hid. Translations are checked against their translation types; one that carries props overlays the base doc, so none of its fields is required. Without TypeScript, or with one older than the docs need, nothing is checked and the report says why. The published `TemplateDoc` type admits an integration template's own `category`, which the loader always accepted and bare `astryx template` groups by. `astryx doctor integration validate` runs it in the follow-up PR, which also adds the repository test over Core's own docs. --- .changeset/template-doc-category.md | 7 + .../cli/authoring/doctypes/template/parse.mjs | 7 +- .../doctypes/template/template.doc.mjs | 6 +- .../cli/authoring/doctypes/template/type.ts | 8 +- .../cli/foundation/discovery/doc-types.mjs | 1178 +++++++++++++++++ .../foundation/discovery/doc-types.test.mjs | 537 ++++++++ 6 files changed, 1734 insertions(+), 9 deletions(-) create mode 100644 .changeset/template-doc-category.md create mode 100644 packages/cli/foundation/discovery/doc-types.mjs create mode 100644 packages/cli/foundation/discovery/doc-types.test.mjs diff --git a/.changeset/template-doc-category.md b/.changeset/template-doc-category.md new file mode 100644 index 0000000000000..13b21a7d03dbd --- /dev/null +++ b/.changeset/template-doc-category.md @@ -0,0 +1,7 @@ +--- +'@astryxdesign/cli': patch +--- + +[fix] The published `TemplateDoc` type admits an integration template's own `category`, which the loader always accepted and bare `astryx template` groups by; Core templates keep the `TemplateCategory` values. + +@josephfarina diff --git a/packages/cli/authoring/doctypes/template/parse.mjs b/packages/cli/authoring/doctypes/template/parse.mjs index dcd8cbe772e6e..d7322931fae21 100644 --- a/packages/cli/authoring/doctypes/template/parse.mjs +++ b/packages/cli/authoring/doctypes/template/parse.mjs @@ -76,9 +76,10 @@ const blockTemplateSchema = z /** * Templates as they load. Integration templates already published omit - * `displayName` and `aspectRatio` and group themselves under their own - * `category`, so the loader accepts those (template discovery falls back to an - * aspect ratio of 1). The published types keep asking authors for all three. + * `displayName` and `aspectRatio`, so the loader accepts those (template + * discovery falls back to an aspect ratio of 1); the published types keep + * asking authors for both. `category` is any string in both: an integration + * template groups itself under its own. * * @typedef {Omit * & {displayName?: string, category?: string}} LoadedPageTemplateDoc diff --git a/packages/cli/authoring/doctypes/template/template.doc.mjs b/packages/cli/authoring/doctypes/template/template.doc.mjs index 10b4c2799a792..cf38551d445aa 100644 --- a/packages/cli/authoring/doctypes/template/template.doc.mjs +++ b/packages/cli/authoring/doctypes/template/template.doc.mjs @@ -78,9 +78,9 @@ export const doc = { }, { name: 'category', - type: 'TemplateCategory', + type: 'TemplateCategory | string', description: - "Functional gallery category following a 'Group - Variant' convention (e.g. 'Dashboard - Analytics', 'Table - Basic', 'Form - Wizard'). The overview groups by the text before ' - '.", + "Functional category. Core templates use a TemplateCategory value following a 'Group - Variant' convention (e.g. 'Dashboard - Analytics', 'Table - Basic', 'Form - Wizard'); the overview groups by the text before ' - '. An integration template may use its own value, and bare `astryx template` groups templates by it.", }, { name: 'isHiddenFromOverview', @@ -162,7 +162,7 @@ export default { notes: [ { type: 'prose', - text: 'When it loads, a template may leave out `displayName` and `aspectRatio` and use its own `category`, as integration templates already published do; a block with no `aspectRatio` previews at 1. Write to the type anyway; it is the contract.', + text: 'When it loads, a template may leave out `displayName` and `aspectRatio`, as integration templates already published do; a block with no `aspectRatio` previews at 1. Write to the type anyway; it is the contract.', }, { type: 'prose', diff --git a/packages/cli/authoring/doctypes/template/type.ts b/packages/cli/authoring/doctypes/template/type.ts index 662346050d7dc..9de096ae0a097 100644 --- a/packages/cli/authoring/doctypes/template/type.ts +++ b/packages/cli/authoring/doctypes/template/type.ts @@ -56,10 +56,12 @@ export interface BaseTemplateDoc extends AuthoredDocGraphFields { * browsable template galleries like the craft browser. */ scaffold?: boolean; - /** Functional category for the docsite Templates overview gallery. - * Templates are grouped by the part before `" - "` (e.g. `"Dashboard"`). + /** Functional category. Core templates use a `TemplateCategory` value, and + * the docsite Templates overview gallery groups them by the part before + * `" - "` (e.g. `"Dashboard"`). An integration template may use its own + * value instead: bare `astryx template` groups templates by this field. * Independent of CLI discovery, which uses `name`/`description`. */ - category?: TemplateCategory; + category?: TemplateCategory | (string & {}); /** Boolean opt-out for templates that shouldn't appear on the Templates * overview gallery. The template stays available via the CLI and diff --git a/packages/cli/foundation/discovery/doc-types.mjs b/packages/cli/foundation/discovery/doc-types.mjs new file mode 100644 index 0000000000000..3258e5c629f69 --- /dev/null +++ b/packages/cli/foundation/discovery/doc-types.mjs @@ -0,0 +1,1178 @@ +// Copyright (c) Meta Platforms, Inc. and affiliates. + +/** + * @file Check each authored doc against its published authoring type. + * + * The compiler is the project's own `typescript`, resolved from the + * directories the caller names. Without it nothing is checked and the report + * says why: it never passes silently. The types are the ones this CLI + * publishes as `@astryxdesign/cli/authoring`, so a doc is checked against what + * the CLI that reads it documents. + * + * Each doc file's value (the export its root reads) is checked the way + * TypeScript checks `value satisfies Type` against the published type for its + * kind, so a doc passes exactly when `tsc` would accept it with that type, and + * an unknown field, a missing required field, and a value outside a field's + * type are each reported: + * - components root: `ComponentDoc` (read as TypeScript reads the union, with + * the member the doc's shape names, so messages are about that member); its + * `docsZh` and `docsDense` translations against `ComponentTranslationDoc`, + * or, with no field required, `ComponentDoc` when a translation carries + * props, as the CLI overlays them on the base doc; + * - hooks (Core): `HookDoc`, translations against `HookTranslationDoc`; + * - templates root: `TemplateDoc`; + * - docs root: the type the doc's `type` names (`ReferenceDoc` when it names + * none), and a `.doc..mjs` overlay against + * `ReferenceTranslationDoc`; + * - themes root: `ThemeDoc`. + * Every finding is a warning, names the field, and says how to fix it. + * + * @input Doc files with the root each was found in, the package root, and the + * directories to resolve `typescript` from. + * @output A DocTypesReport. + * @position Foundation discovery: it reads doc files and returns a report, + * and prints nothing itself. + */ + +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import {fileURLToPath} from 'node:url'; +import {resolveProjectTypeScript} from './project-typescript.mjs'; + +/** + * @typedef {'components' | 'hooks' | 'templates' | 'docs' | 'docs-overlay' | 'themes'} DocRoot + */ + +/** + * @typedef {object} DocFile + * @property {string} path absolute path to the doc file + * @property {DocRoot} root the contribution root it was found in + */ + +/** + * @typedef {'unknown_field' | 'missing_field' | 'wrong_type'} DocTypeProblem + */ + +/** + * @typedef {object} DocTypeFinding + * @property {string} doc the doc file, relative to the package root + * @property {number} line 1-based line of the field (or of the doc value) + * @property {string} export the export checked (`default`, `docs`, `docsZh`, …) + * @property {string} type the authoring type it was checked against + * @property {string} field the field's path in the doc, such as + * `usage.bestPractices[2].guidance`; empty for the doc value as a whole + * @property {DocTypeProblem} problem + * @property {'warning'} severity + * @property {string} message what TypeScript reports, shortened + * @property {string} fix what to change + */ + +/** + * @typedef {object} DocTypesUnchecked + * @property {string} doc the doc file, relative to the package root + * @property {string} reason why it was not checked + */ + +/** + * @typedef {object} DocTypesReport + * @property {'checked' | 'skipped'} status `skipped` means nothing was checked, + * and `reason` says why + * @property {string} [reason] + * @property {string} [typescript] the version of the compiler that checked + * @property {number} docs how many doc values were checked + * @property {DocTypeFinding[]} findings + * @property {DocTypesUnchecked[]} unchecked doc files with nothing to check, + * or not fully checked + */ + +/** The line both surfaces print when the compiler is missing. */ +export const DOC_TYPES_TYPESCRIPT_MISSING_REASON = + 'TypeScript is not installed in this project, so docs were not checked against the authoring types. Install typescript and run this check again.'; + +const AUTHORING = '@astryxdesign/cli/authoring'; +const AUTHORING_TYPES = path.resolve( + path.dirname(fileURLToPath(import.meta.url)), + '..', + '..', + 'authoring', + 'index.d.ts', +); + +const TRANSLATIONS = ['docsZh', 'docsDense']; + +/** How each root's module names its doc, in the readers' precedence order. */ +const MAIN_EXPORTS = { + components: ['default', 'docs'], + hooks: ['default', 'docs'], + templates: ['default', 'doc'], + docs: ['docs', 'default'], + 'docs-overlay': [], + themes: ['default', 'doc', 'docs'], +}; + +/** The type a docs-root doc's `type` stamp names. */ +const DOCS_ROOT_TYPES = { + generic: 'ReferenceDoc', + namespace: 'NamespaceDoc', + function: 'FunctionDoc', + schema: 'SchemaDoc', + command: 'CommandDoc', + enum: 'EnumDoc', + theme: 'ThemeDoc', + component: 'ComponentDoc', + page: 'TemplateDoc', + block: 'TemplateDoc', +}; + +/** + * Each pass sets aside at least one more field, so a doc needs at most one + * pass per field. One still finding new problems after this many is listed as + * not fully checked. + */ +const MAX_PASSES = 50; + +/** TypeScript's assignability diagnostics: the only ones reported. */ +const RELEVANT = new Set([ + 1360, 2322, 2353, 2559, 2561, 2739, 2740, 2741, 2820, +]); + +/** + * @typedef {object} Target + * @property {string} exportName + * @property {any} node the doc value's expression + * @property {string} typeExpr the authoring type, as a type expression + * @property {string} typeName how findings name that type + * @property {any[]} blank nodes whose leading JSDoc annotations are blanked + * @property {Array<[number, number]>} blankRanges source ranges to blank + */ + +/** + * @param {string} name + */ +const authoring = name => `import('${AUTHORING}').${name}`; + +/** + * The top-level property names and `type` stamp of an object literal, or null + * when the value is not one. + * @param {any} ts + * @param {any} node + * @returns {{names: Set, stamp: string | null, componentsWithProps: boolean} | null} + */ +function shapeOf(ts, node) { + if (!ts.isObjectLiteralExpression(node)) return null; + const names = new Set(); + /** @type {string | null} */ + let stamp = null; + let componentsWithProps = false; + for (const property of node.properties) { + const key = property.name ? propertyName(ts, property.name) : null; + if (key == null) continue; + names.add(key); + if ( + key === 'type' && + ts.isPropertyAssignment(property) && + ts.isStringLiteralLike(property.initializer) + ) { + stamp = property.initializer.text; + } + if ( + key === 'components' && + ts.isPropertyAssignment(property) && + ts.isArrayLiteralExpression(property.initializer) + ) { + componentsWithProps = property.initializer.elements.some( + (/** @type {any} */ element) => + ts.isObjectLiteralExpression(element) && + element.properties.some( + (/** @type {any} */ p) => + p.name && propertyName(ts, p.name) === 'props', + ), + ); + } + } + return {names, stamp, componentsWithProps}; +} + +/** + * @param {any} ts + * @param {any} name + * @returns {string | null} + */ +function propertyName(ts, name) { + if ( + ts.isIdentifier(name) || + ts.isStringLiteralLike(name) || + ts.isNumericLiteral(name) || + ts.isPrivateIdentifier?.(name) + ) { + return name.text; + } + return null; +} + +/** + * The `ComponentDoc` member a doc's shape selects (`subComponentOf`, + * `components`, `props`), widened by every field the other members declare, + * typed `unknown`. That is how TypeScript reads a value against the union: + * a field any member declares is not unknown, and the value must be + * assignable to one member. Picking the member keeps the messages about it + * (`SingleComponentDoc requires displayName`) instead of whichever member + * TypeScript guesses. + * @param {{names: Set} | null} shape + * @returns {{typeExpr: string, typeName: string}} + */ +function componentType(shape) { + const union = authoring('ComponentDoc'); + const member = shape?.names.has('subComponentOf') + ? `Extract<${union}, {subComponentOf: string}>` + : shape && shape.names.has('components') && !shape.names.has('props') + ? authoring('MultiComponentDoc') + : shape && !shape.names.has('components') + ? authoring('SingleComponentDoc') + : null; + if (!member) return {typeExpr: union, typeName: 'ComponentDoc'}; + const allKeys = `(${union} extends infer U ? U extends unknown ? keyof U : never : never)`; + return { + typeExpr: `${member} & Partial, unknown>>`, + typeName: 'ComponentDoc', + }; +} + +/** + * The authoring type one doc value is checked against. + * @param {any} ts + * @param {DocRoot} root + * @param {string} exportName + * @param {any} node + * @returns {{typeExpr: string, typeName: string}} + */ +function expectedType(ts, root, exportName, node) { + const shape = shapeOf(ts, node); + const translation = TRANSLATIONS.includes(exportName); + if (root === 'components') { + if ( + translation && + !(shape?.names.has('props') || shape?.componentsWithProps) + ) { + return { + typeExpr: authoring('ComponentTranslationDoc'), + typeName: 'ComponentTranslationDoc', + }; + } + // A translation with props overlays the base doc, which supplies every + // field it leaves out: each field it writes must match, none is required. + if (translation) { + const full = componentType(shape); + return {typeExpr: `Partial<${full.typeExpr}>`, typeName: full.typeName}; + } + return componentType(shape); + } + if (root === 'hooks') { + if (translation && !shape?.names.has('params')) { + return { + typeExpr: authoring('HookTranslationDoc'), + typeName: 'HookTranslationDoc', + }; + } + if (translation) { + return { + typeExpr: `Partial<${authoring('HookDoc')}>`, + typeName: 'HookDoc', + }; + } + return {typeExpr: authoring('HookDoc'), typeName: 'HookDoc'}; + } + if (root === 'templates') { + return {typeExpr: authoring('TemplateDoc'), typeName: 'TemplateDoc'}; + } + if (root === 'docs-overlay') { + return { + typeExpr: authoring('ReferenceTranslationDoc'), + typeName: 'ReferenceTranslationDoc', + }; + } + if (root === 'themes') { + return {typeExpr: authoring('ThemeDoc'), typeName: 'ThemeDoc'}; + } + if (translation) { + return { + typeExpr: authoring('ReferenceTranslationDoc'), + typeName: 'ReferenceTranslationDoc', + }; + } + const named = + shape?.stamp != null + ? /** @type {Record} */ (DOCS_ROOT_TYPES)[shape.stamp] + : undefined; + if (named === 'ComponentDoc') return componentType(shape); + const typeName = named ?? 'ReferenceDoc'; + return {typeExpr: authoring(typeName), typeName}; +} + +/** + * The exported values of a module, by export name: each statement and the + * expression it exports. + * @param {any} ts + * @param {any} sf + * @returns {Map} + */ +function moduleExports(ts, sf) { + /** @type {Map} */ + const locals = new Map(); + /** @type {Map} */ + const exported = new Map(); + for (const statement of sf.statements) { + if (ts.isVariableStatement(statement)) { + const isExported = statement.modifiers?.some( + (/** @type {any} */ m) => m.kind === ts.SyntaxKind.ExportKeyword, + ); + for (const declaration of statement.declarationList.declarations) { + if (!ts.isIdentifier(declaration.name) || !declaration.initializer) + continue; + const entry = {statement, expression: declaration.initializer}; + locals.set(declaration.name.text, entry); + if (isExported) exported.set(declaration.name.text, entry); + } + } else if (ts.isExportAssignment(statement) && !statement.isExportEquals) { + exported.set('default', {statement, expression: statement.expression}); + } + } + for (const statement of sf.statements) { + if ( + ts.isExportDeclaration(statement) && + !statement.moduleSpecifier && + statement.exportClause && + ts.isNamedExports(statement.exportClause) + ) { + for (const element of statement.exportClause.elements) { + const local = (element.propertyName ?? element.name).text; + const entry = locals.get(local); + if (entry) exported.set(element.name.text, entry); + } + } + } + // `export default docs` exports a local's initializer. + for (const [name, entry] of exported) { + let expression = entry.expression; + for (let hop = 0; hop < 3 && ts.isIdentifier(expression); hop++) { + const local = locals.get(expression.text); + if (!local) break; + expression = local.expression; + exported.set(name, {statement: local.statement, expression}); + } + } + return exported; +} + +/** + * Strip the wrappers around a doc value (parentheses, `satisfies`, `as`, + * JSDoc casts), returning the value and the wrappers. + * @param {any} ts + * @param {any} expression + */ +function unwrap(ts, expression) { + /** @type {any[]} */ + const wrappers = []; + let node = expression; + for (;;) { + if ( + ts.isParenthesizedExpression(node) || + ts.isAsExpression(node) || + ts.isNonNullExpression(node) || + (ts.isSatisfiesExpression && ts.isSatisfiesExpression(node)) || + (ts.isTypeAssertionExpression && ts.isTypeAssertionExpression(node)) + ) { + wrappers.push(node); + node = node.expression; + continue; + } + return {node, wrappers}; + } +} + +/** + * The doc values one file holds, with the type each is checked against. + * @param {any} ts + * @param {any} sf + * @param {DocRoot} root + * @returns {Target[]} + */ +function targetsOf(ts, sf, root) { + const exported = moduleExports(ts, sf); + /** @type {string[]} */ + const names = []; + const main = MAIN_EXPORTS[root].find(name => exported.has(name)); + if (main) names.push(main); + if (root !== 'templates' && root !== 'themes') { + for (const name of TRANSLATIONS) if (exported.has(name)) names.push(name); + } + /** @type {Target[]} */ + const targets = []; + const seen = new Set(); + for (const exportName of names) { + const entry = /** @type {{statement: any, expression: any}} */ ( + exported.get(exportName) + ); + const {node, wrappers} = unwrap(ts, entry.expression); + if (seen.has(node)) continue; + seen.add(node); + /** @type {Array<[number, number]>} */ + const blankRanges = []; + for (const wrapper of wrappers) { + // ` satisfies X` / ` as X` after the value. + if ( + !ts.isParenthesizedExpression(wrapper) && + wrapper.end > wrapper.expression.end + ) { + if (!ts.isNonNullExpression(wrapper)) { + blankRanges.push([wrapper.expression.end, wrapper.end]); + } + } + } + targets.push({ + exportName, + node, + ...expectedType(ts, root, exportName, node), + blank: [entry.statement, ...wrappers], + blankRanges, + }); + } + return targets; +} + +/** + * The file's text with each doc value wrapped in a `satisfies` check of its + * type, the authors' own type annotations on it blanked (so a value is checked + * once, against the type for its root), and a map back to the original text. + * `removed` ranges (unknown fields already reported) are blanked too, and + * `casts` ranges (values already reported outside their type) are cast to + * `any`, so the next pass reports what TypeScript stopped at them to report. + * Blanking keeps every offset, so lines and fields map back unchanged. + * @param {any} ts + * @param {any} sf + * @param {Target[]} targets + * @param {boolean} isTs + * @param {Array<[number, number]>} [removed] + * @param {Array<[number, number]>} [casts] + */ +function instrument(ts, sf, targets, isTs, removed = [], casts = []) { + const text = sf.text; + const chars = text.split(''); + const blank = (/** @type {number} */ from, /** @type {number} */ to) => { + for (let i = from; i < to; i++) + if (chars[i] !== '\n' && chars[i] !== '\r') chars[i] = ' '; + }; + for (const [from, to] of removed) blank(from, to); + for (const target of targets) { + for (const node of target.blank) { + for (const range of ts.getLeadingCommentRanges(text, node.pos) ?? []) { + const comment = text.slice(range.pos, range.end); + if ( + comment.startsWith('/**') && + /@(?:type|satisfies)\b/.test(comment) + ) { + blank(range.pos, range.end); + } + } + } + for (const [from, to] of target.blankRanges) blank(from, to); + } + /** @type {Array<{at: number, text: string}>} */ + const insertions = []; + for (const target of targets) { + const start = target.node.getStart(sf); + if (isTs) { + insertions.push({at: start, text: '('}); + insertions.push({ + at: target.node.end, + text: `) satisfies ${target.typeExpr}`, + }); + } else { + insertions.push({ + at: start, + text: `/** @satisfies {${target.typeExpr}} */ (`, + }); + insertions.push({at: target.node.end, text: ')'}); + } + } + for (const [from, to] of casts) { + insertions.push({at: from, text: isTs ? '(' : '/** @type {any} */ ('}); + insertions.push({at: to, text: isTs ? ' as any)' : ')'}); + } + insertions.sort((a, b) => a.at - b.at); + let out = ''; + let cursor = 0; + for (const insertion of insertions) { + out += chars.slice(cursor, insertion.at).join('') + insertion.text; + cursor = insertion.at; + } + out += chars.slice(cursor).join(''); + /** @param {number} virtual */ + const toOriginal = virtual => { + let shift = 0; + for (const insertion of insertions) { + const virtualAt = insertion.at + shift; + if (virtual < virtualAt) return virtual - shift; + if (virtual < virtualAt + insertion.text.length) return insertion.at; + shift += insertion.text.length; + } + return virtual - shift; + }; + return {text: out, toOriginal}; +} + +/** + * The path from a doc value to the field at `pos`: `a.b[2].c`. + * @param {any} ts + * @param {any} sf + * @param {any} root + * @param {number} pos + */ +function fieldPathAt(ts, sf, root, pos) { + /** @type {Array} */ + const parts = []; + let node = root; + for (;;) { + /** @type {any} */ + let next; + ts.forEachChild(node, (/** @type {any} */ child) => { + if (!next && child.getStart(sf) <= pos && pos < child.end) next = child; + }); + if (!next) break; + if (ts.isArrayLiteralExpression(node)) + parts.push(node.elements.indexOf(next)); + if ( + (ts.isPropertyAssignment(next) || + ts.isShorthandPropertyAssignment(next) || + ts.isMethodDeclaration(next)) && + next.name + ) { + const key = propertyName(ts, next.name); + if (key != null) parts.push(key); + } + node = next; + } + return parts; +} + +/** @param {Array} parts */ +function formatPath(parts) { + return parts + .map((part, i) => + typeof part === 'number' ? `[${part}]` : i === 0 ? part : `.${part}`, + ) + .join(''); +} + +/** + * Shorten long quoted types in a TypeScript message. + * @param {string} message + */ +function shorten(message) { + return message + .replace(/\s+/g, ' ') + .replace(/'([^']{90,})'/g, (_, type) => `'${type.slice(0, 70)}…'`) + .trim(); +} + +/** + * Every message in a diagnostic's chain, outermost first. + * @param {any} ts + * @param {any} messageText + * @returns {string[]} + */ +function chainOf(ts, messageText) { + if (typeof messageText === 'string') return [messageText]; + /** @type {string[]} */ + const out = []; + /** @param {any} chain */ + const visit = chain => { + out.push(chain.messageText); + if (chain.next?.length) visit(chain.next[0]); + }; + visit(messageText); + return out; +} + +/** + * The source range of the property whose name holds `pos`, with the comma + * after it, or null. + * @param {any} ts + * @param {any} sf + * @param {any} root + * @param {number} pos + * @returns {[number, number] | null} + */ +function propertyRangeAt(ts, sf, root, pos) { + /** @type {any} */ + let found = null; + let node = root; + for (;;) { + /** @type {any} */ + let next; + ts.forEachChild(node, (/** @type {any} */ child) => { + if (!next && child.getStart(sf) <= pos && pos < child.end) next = child; + }); + if (!next) break; + if ( + (ts.isPropertyAssignment(next) || + ts.isShorthandPropertyAssignment(next) || + ts.isMethodDeclaration(next)) && + next.name && + next.name.getStart(sf) <= pos && + pos < next.name.end + ) { + found = next; + } + node = next; + } + if (!found) return null; + let end = found.end; + const text = sf.text; + let i = end; + while (i < text.length && /\s/.test(text[i])) i++; + if (text[i] === ',') end = i + 1; + return [found.getStart(sf), end]; +} + +/** + * The source range of the value a diagnostic at `pos` is about: the value of + * the property whose name holds `pos`, else the array element that holds it, + * or null. + * @param {any} ts + * @param {any} sf + * @param {any} root + * @param {number} pos + * @returns {[number, number] | null} + */ +function valueRangeAt(ts, sf, root, pos) { + /** @type {any} */ + let found = null; + let node = root; + for (;;) { + /** @type {any} */ + let next; + ts.forEachChild(node, (/** @type {any} */ child) => { + if (!next && child.getStart(sf) <= pos && pos < child.end) next = child; + }); + if (!next) break; + if ( + ts.isPropertyAssignment(next) && + next.name.getStart(sf) <= pos && + pos < next.name.end + ) { + found = next.initializer; + } else if (ts.isArrayLiteralExpression(node)) { + found = next; + } + node = next; + } + if (!found || found === root) return null; + return [found.getStart(sf), found.end]; +} + +/** + * The source range of the innermost object literal, below the doc value + * itself, that holds `pos`, or null. + * @param {any} ts + * @param {any} sf + * @param {any} root + * @param {number} pos + * @returns {[number, number] | null} + */ +function objectRangeAt(ts, sf, root, pos) { + /** @type {any} */ + let found = null; + let node = root; + for (;;) { + /** @type {any} */ + let next; + ts.forEachChild(node, (/** @type {any} */ child) => { + if (!next && child.getStart(sf) <= pos && pos < child.end) next = child; + }); + if (!next) break; + if (ts.isObjectLiteralExpression(next)) found = next; + node = next; + } + if (!found || found === root) return null; + return [found.getStart(sf), found.end]; +} + +/** + * The module a file re-exports one of `names` from (`export {default} from + * './x.mjs'`, or `export * from`), or null. + * @param {any} ts + * @param {any} sf + * @param {string[]} names + * @returns {string | null} + */ +function reexportSource(ts, sf, names) { + for (const statement of sf.statements) { + if ( + !ts.isExportDeclaration(statement) || + !statement.moduleSpecifier || + !ts.isStringLiteral(statement.moduleSpecifier) + ) { + continue; + } + const clause = statement.exportClause; + if ( + clause == null || + (ts.isNamedExports(clause) && + clause.elements.some( + /** @param {any} element */ element => + names.includes(element.name.text), + )) + ) { + return statement.moduleSpecifier.text; + } + } + return null; +} + +/** + * A message with each quoted object type that holds a set-aside `any` + * replaced by "this value". + * @param {string} message + */ +function unquoteSetAside(message) { + return message + .replace(/\btype '\{[^']*\bany\b[^']*\}'/g, 'this value') + .replace(/^Type '\{[^']*\bany\b[^']*\}'/, 'This value'); +} + +/** + * A property name as TypeScript prints it, without the quotes a quoted key + * keeps (`"keyword"`). + * @param {string} name + */ +function unquote(name) { + return name.replace(/^"(.*)"$/, '$1'); +} + +/** + * Findings for one diagnostic. + * @param {any} ts + * @param {any} diagnostic + * @param {Array} astPath + * @param {{root: DocRoot, doc: string, line: number, exportName: string, typeName: string}} where + * @returns {DocTypeFinding[]} + */ +function findingsFor(ts, diagnostic, astPath, where) { + // The widened member reads as the member itself. + const chain = chainOf(ts, diagnostic.messageText).map(message => + message.replace(/ & Partial + /^Types of property '([^']+)' are incompatible/.exec(message)?.[1], + ) + .filter(key => key != null); + const parts = [...astPath]; + for (const key of below) + if (parts[parts.length - 1] !== key) + parts.push(/** @type {string} */ (key)); + const leaf = chain[chain.length - 1]; + const base = { + doc: where.doc, + line: where.line, + export: where.exportName, + type: where.typeName, + severity: /** @type {const} */ ('warning'), + }; + + const missing = + /Property '([^']+)' is missing in type/.exec(leaf)?.[1] != null + ? [/Property '([^']+)' is missing in type/.exec(leaf)?.[1]] + : /is missing the following properties from type '[^']*': (.+?)(?:, and \d+ more)?\.?$/ + .exec(leaf)?.[1] + ?.split(', '); + if (missing?.length) { + const owner = /required in type '([^']+)'|from type '([^']+)'/.exec(leaf); + const named = owner?.[1] ?? owner?.[2]; + const ownerType = + named != null && /^[A-Za-z_$][\w$]*$/.test(named) + ? named + : where.typeName; + return missing.map(name => { + const field = formatPath([ + ...parts, + unquote(/** @type {string} */ (name)), + ]); + return { + ...base, + field, + problem: /** @type {const} */ ('missing_field'), + message: shorten(leaf), + fix: `Add \`${field}\`: ${shorten(ownerType)} requires it.`, + }; + }); + } + + const unknown = + /Object literal may only specify known properties, (?:and|but) '([^']+)' does not exist in type '([^']+)'(?:\. Did you mean to write '([^']+)'\?)?/.exec( + chain.find(message => + /Object literal may only specify known properties/.test(message), + ) ?? '', + ); + if (unknown) { + const key = unquote(unknown[1]); + const owner = /^[A-Za-z_$][\w$]*$/.test(unknown[2]) + ? unknown[2] + : where.typeName; + const suggestion = unknown[3] == null ? undefined : unquote(unknown[3]); + if (parts[parts.length - 1] !== key) parts.push(key); + const field = formatPath(parts); + return [ + { + ...base, + field, + problem: 'unknown_field', + message: shorten(leaf), + fix: suggestion + ? `Rename \`${field}\` to \`${suggestion}\`.` + : `Remove \`${field}\`: ${shorten(owner)} has no field \`${key}\`.`, + }, + ]; + } + + const field = formatPath(parts); + const didYouMean = /Did you mean '([^']+)'\?/.exec(chain.join(' '))?.[1]; + const assignable = [...chain] + .reverse() + .map(message => /is not assignable to type '([^']+)'/.exec(message)?.[1]) + .find(type => type != null); + let fix; + if (field === 'type' && where.root === 'components') { + fix = + "This doc is in the components root, so it is a component doc: set type: 'component', or move it to the docs root."; + } else if (didYouMean) { + fix = `Change \`${field || 'the doc'}\` to ${didYouMean}.`; + } else if (assignable) { + fix = `Change \`${field || 'the doc'}\` to a value of type ${shorten(`'${assignable}'`).slice(1, -1)}.`; + } else { + fix = `Make \`${field || 'the doc'}\` match ${where.typeName}.`; + } + return [ + { + ...base, + field, + problem: 'wrong_type', + message: shorten(leaf), + fix, + }, + ]; +} + +/** + * Check each doc file's values against the published authoring types. + * @param {DocFile[]} files + * @param {{packageDir: string, resolveFrom: Array}} options + * @returns {Promise} + */ +export async function checkDocTypes(files, options) { + const {packageDir, resolveFrom} = options; + // With no doc files there is nothing to check, TypeScript or not. + if (files.length === 0) { + return {status: 'checked', docs: 0, findings: [], unchecked: []}; + } + const compiler = resolveProjectTypeScript(resolveFrom); + if (!compiler) { + return { + status: 'skipped', + reason: DOC_TYPES_TYPESCRIPT_MISSING_REASON, + docs: 0, + findings: [], + unchecked: [], + }; + } + const {ts, version} = compiler; + const [major = 0, minor = 0] = String(ts.versionMajorMinor ?? version) + .split('.') + .map(Number); + const hasJs = files.some(file => !/\.ts$/.test(file.path)); + // Every doc is checked with `satisfies`, which needs TypeScript 4.9; its + // JSDoc form, for a JavaScript doc, needs 5.0. + const [needMajor, needMinor] = hasJs ? [5, 0] : [4, 9]; + if (major < needMajor || (major === needMajor && minor < needMinor)) { + return { + status: 'skipped', + reason: `TypeScript ${version} cannot check ${hasJs ? 'a JavaScript doc' : 'a doc'} against its type (that needs TypeScript ${needMajor}.${needMinor} or later), so docs were not checked. Upgrade typescript and run this check again.`, + docs: 0, + findings: [], + unchecked: [], + }; + } + const rel = (/** @type {string} */ file) => + path.relative(packageDir, file).split(path.sep).join('/'); + + /** + * @typedef {object} Entry + * @property {DocFile} file + * @property {any} sf the original source + * @property {Target[]} targets + * @property {boolean} isTs + * @property {{text: string, toOriginal: (v: number) => number}} virtual + * @property {any} virtualSf + * @property {Array<[number, number]>} removed + * @property {Array<[number, number]>} casts + * @property {Set} seen + */ + /** @type {Map} */ + const instrumented = new Map(); + /** @type {DocTypesUnchecked[]} */ + const unchecked = []; + for (const file of files) { + let text; + try { + text = fs.readFileSync(file.path, 'utf8'); + } catch (err) { + unchecked.push({ + doc: rel(file.path), + reason: `it cannot be read: ${err instanceof Error ? err.message : String(err)}`, + }); + continue; + } + const isTs = /\.ts$/.test(file.path); + const sf = ts.createSourceFile( + file.path, + text, + ts.ScriptTarget.Latest, + true, + isTs ? ts.ScriptKind.TS : ts.ScriptKind.JS, + ); + const targets = targetsOf(ts, sf, file.root); + if (targets.length === 0) { + const names = MAIN_EXPORTS[file.root]; + const from = reexportSource(ts, sf, [...names, ...TRANSLATIONS]); + unchecked.push({ + doc: rel(file.path), + reason: from + ? `it re-exports its doc from ${from}, which this check does not follow, so it was not checked` + : names.length > 0 + ? `it exports no doc the CLI reads (${names.join(' or ')})` + : `it exports no translation the CLI reads (${TRANSLATIONS.join(' or ')})`, + }); + continue; + } + const virtual = instrument(ts, sf, targets, isTs); + instrumented.set(path.resolve(file.path), { + file, + sf, + targets, + isTs, + virtual, + virtualSf: virtualSourceFile(ts, file.path, virtual.text, isTs), + removed: [], + casts: [], + seen: new Set(), + }); + } + + const options_ = { + allowJs: true, + checkJs: true, + noEmit: true, + skipLibCheck: true, + strict: true, + noImplicitAny: false, + target: ts.ScriptTarget.ES2022, + module: ts.ModuleKind.ESNext, + moduleResolution: ts.ModuleResolutionKind.Bundler, + jsx: ts.JsxEmit.ReactJSX, + resolveJsonModule: true, + esModuleInterop: true, + allowImportingTsExtensions: true, + types: [], + paths: {[AUTHORING]: [AUTHORING_TYPES]}, + }; + const host = ts.createCompilerHost(options_); + const getSourceFile = host.getSourceFile.bind(host); + // Every other file is parsed once, so a later pass reuses it. + /** @type {Map} */ + const parsed = new Map(); + host.getSourceFile = ( + /** @type {string} */ fileName, + /** @type {any} */ languageVersion, + /** @type {any} */ onError, + /** @type {any} */ shouldCreate, + ) => { + const entry = instrumented.get(path.resolve(fileName)); + if (entry) return entry.virtualSf; + if (!parsed.has(fileName)) { + parsed.set( + fileName, + getSourceFile(fileName, languageVersion, onError, shouldCreate), + ); + } + return parsed.get(fileName); + }; + const readFile = host.readFile.bind(host); + host.readFile = (/** @type {string} */ fileName) => + instrumented.get(path.resolve(fileName))?.virtual.text ?? + readFile(fileName); + + /** @type {DocTypeFinding[]} */ + const findings = []; + let checked = [...instrumented.values()].reduce( + (sum, entry) => sum + entry.targets.length, + 0, + ); + const rootNames = [...instrumented.values()].map(entry => entry.file.path); + // TypeScript stops at an object's first unknown field, and a value outside + // its type hides the rest of its object, so a doc is checked again with each + // unknown field blanked and each such value cast to `any`, until nothing new + // is found. The doc's own `type` is never cast: a wrong one means the doc is + // a different kind, and what the other kind would report is a guess. + /** @type {Entry[]} */ + let pending = [...instrumented.values()]; + /** @type {any} */ + let program; + for (let pass = 0; pending.length > 0; pass++) { + if (pass === MAX_PASSES) { + for (const entry of pending) { + unchecked.push({ + doc: rel(entry.file.path), + reason: `TypeScript still found new problems after ${MAX_PASSES} passes, so not every field was checked; fix the ones reported and run the check again`, + }); + } + break; + } + program = ts.createProgram({ + rootNames, + options: options_, + host, + oldProgram: program, + }); + /** @type {Entry[]} */ + const again = []; + for (const entry of pending) { + const virtualSf = program.getSourceFile(entry.file.path); + if (!virtualSf) continue; + const doc = rel(entry.file.path); + // Text the compiler cannot parse proves nothing about its types. + const [syntax] = program.getSyntacticDiagnostics(virtualSf); + if (syntax) { + if (pass === 0) checked -= entry.targets.length; + unchecked.push({ + doc, + reason: `TypeScript ${version} cannot parse it with its type check added (${ts.flattenDiagnosticMessageText(syntax.messageText, ' ')}), so ${pass === 0 ? 'it was not checked' : 'not every field was checked'}`, + }); + continue; + } + let changed = false; + for (const diagnostic of program.getSemanticDiagnostics(virtualSf)) { + if (!RELEVANT.has(diagnostic.code) || diagnostic.start == null) { + continue; + } + const pos = entry.virtual.toOriginal(diagnostic.start); + const target = entry.targets.find( + candidate => + candidate.node.getStart(entry.sf) <= pos && + pos <= candidate.node.end, + ); + if (!target) continue; + const astPath = fieldPathAt(ts, entry.sf, target.node, pos); + const line = entry.sf.getLineAndCharacterOfPosition(pos).line + 1; + for (const finding of findingsFor(ts, diagnostic, astPath, { + root: entry.file.root, + doc, + line, + exportName: target.exportName, + typeName: target.typeName, + })) { + if (finding.problem === 'unknown_field') { + const range = propertyRangeAt(ts, entry.sf, target.node, pos); + if (range && !entry.removed.some(([from]) => from === range[0])) { + entry.removed.push(range); + changed = true; + } + } else if ( + finding.problem === 'missing_field' && + /[.[]/.test(finding.field) + ) { + // A nested object missing a field hides the doc's other + // findings, so that object is set aside. + const range = + valueRangeAt(ts, entry.sf, target.node, pos) ?? + objectRangeAt(ts, entry.sf, target.node, pos); + if (range && !entry.casts.some(([from]) => from === range[0])) { + entry.casts.push(range); + changed = true; + } + } else if ( + finding.problem === 'wrong_type' && + finding.field !== 'type' + ) { + // A deeper `type` picks the shape of the object that holds it (a + // content block, a prop), so that whole object is set aside. + const range = /(^|\.)type$/.test(finding.field) + ? objectRangeAt(ts, entry.sf, target.node, pos) + : valueRangeAt(ts, entry.sf, target.node, pos); + if (range && !entry.casts.some(([from]) => from === range[0])) { + entry.casts.push(range); + changed = true; + } + } + const key = `${finding.export}\0${finding.field}\0${finding.problem}`; + if (entry.seen.has(key)) continue; + entry.seen.add(key); + // After a pass sets values aside, TypeScript quotes them as `any`: + // name the value instead of quoting text the doc does not have. + if (pass > 0) finding.message = unquoteSetAside(finding.message); + findings.push(finding); + } + } + if (changed) { + entry.virtual = instrument( + ts, + entry.sf, + entry.targets, + entry.isTs, + entry.removed, + entry.casts, + ); + entry.virtualSf = virtualSourceFile( + ts, + entry.file.path, + entry.virtual.text, + entry.isTs, + ); + again.push(entry); + } + } + pending = again; + } + findings.sort( + (a, b) => + a.doc.localeCompare(b.doc) || + a.line - b.line || + a.field.localeCompare(b.field), + ); + unchecked.sort((a, b) => a.doc.localeCompare(b.doc)); + return { + status: 'checked', + typescript: version, + docs: checked, + findings, + unchecked, + }; +} + +/** + * @param {any} ts + * @param {string} fileName + * @param {string} text + * @param {boolean} isTs + */ +function virtualSourceFile(ts, fileName, text, isTs) { + return ts.createSourceFile( + fileName, + text, + ts.ScriptTarget.Latest, + true, + isTs ? ts.ScriptKind.TS : ts.ScriptKind.JS, + ); +} diff --git a/packages/cli/foundation/discovery/doc-types.test.mjs b/packages/cli/foundation/discovery/doc-types.test.mjs new file mode 100644 index 0000000000000..ed17541b89f4e --- /dev/null +++ b/packages/cli/foundation/discovery/doc-types.test.mjs @@ -0,0 +1,537 @@ +// Copyright (c) Meta Platforms, Inc. and affiliates. + +/** + * @file Each doc against its published authoring type: unknown, missing, and + * wrongly typed fields, the root deciding the type, translations, the authors' + * own annotations, and the visible skip when the project has no TypeScript. + */ + +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import {createRequire} from 'node:module'; +import {afterEach, beforeEach, describe, expect, it} from 'vitest'; +import { + checkDocTypes, + DOC_TYPES_TYPESCRIPT_MISSING_REASON, +} from './doc-types.mjs'; + +const require = createRequire(import.meta.url); +const TYPESCRIPT_DIR = path.dirname(require.resolve('typescript/package.json')); + +/** @type {string} */ +let pkg; + +beforeEach(() => { + pkg = fs.mkdtempSync(path.join(os.tmpdir(), 'astryx-doc-types-')); +}); + +afterEach(() => { + fs.rmSync(pkg, {recursive: true, force: true}); +}); + +function withTypeScript() { + fs.mkdirSync(path.join(pkg, 'node_modules'), {recursive: true}); + fs.symlinkSync( + TYPESCRIPT_DIR, + path.join(pkg, 'node_modules', 'typescript'), + 'dir', + ); +} + +/** + * @param {string} rel + * @param {string} text + */ +function write(rel, text) { + const file = path.join(pkg, rel); + fs.mkdirSync(path.dirname(file), {recursive: true}); + fs.writeFileSync(file, text); + return file; +} + +/** + * @param {Array<[string, import('./doc-types.mjs').DocRoot]>} files + */ +function check(files) { + return checkDocTypes( + files.map(([rel, root]) => ({path: path.join(pkg, rel), root})), + {packageDir: pkg, resolveFrom: [pkg]}, + ); +} + +/** @param {import('./doc-types.mjs').DocTypesReport} report */ +const summary = report => + report.findings.map(f => `${f.doc} ${f.export} ${f.field} ${f.problem}`); + +const COMPONENT = { + type: 'component', + name: 'Badge', + displayName: 'Badge', + usage: {description: 'Shows a count.'}, + props: [ + {name: 'label', type: 'string', description: 'Text.', required: true}, + ], +}; + +describe('checkDocTypes', () => { + it('skips with a reason, and checks nothing, when the project has no TypeScript', async () => { + write( + 'components/Badge.doc.mjs', + `export default ${JSON.stringify(COMPONENT)};\n`, + ); + + const report = await check([['components/Badge.doc.mjs', 'components']]); + + expect(report).toEqual({ + status: 'skipped', + reason: DOC_TYPES_TYPESCRIPT_MISSING_REASON, + docs: 0, + findings: [], + unchecked: [], + }); + }); + + it('says there is nothing to check when the package has no doc files, with or without TypeScript', async () => { + const without = await check([]); + withTypeScript(); + const withTs = await check([]); + + for (const report of [without, withTs]) { + expect(report).toEqual({ + status: 'checked', + docs: 0, + findings: [], + unchecked: [], + }); + } + }); + + it('reports what a value outside its type hid in the same run', async () => { + withTypeScript(); + write( + 'components/Badge.doc.mjs', + [ + 'export default {', + " type: 'component',", + " name: 'Badge',", + " keyword: ['count'],", + " usage: {description: 'Shows a count.', bestPractices: [{guidance: 'yes', description: 'Do.'}]},", + " props: [{name: 'label', type: 'string', description: 'Text.'}],", + '};', + '', + ].join('\n'), + ); + + const report = await check([['components/Badge.doc.mjs', 'components']]); + + expect(summary(report)).toEqual([ + 'components/Badge.doc.mjs default displayName missing_field', + 'components/Badge.doc.mjs default keyword unknown_field', + 'components/Badge.doc.mjs default usage.bestPractices[0].guidance wrong_type', + ]); + expect(report.findings[0].fix).toBe( + 'Add `displayName`: SingleComponentDoc requires it.', + ); + }); + + it('sets aside an object whose own `type` is wrong, and reports what it hid', async () => { + withTypeScript(); + write( + 'components/Badge.doc.mjs', + [ + 'export default {', + " type: 'component',", + " name: 'Badge',", + " keyword: ['count'],", + " usage: {description: 'Shows a count.'},", + " props: [{name: 'label', type: 42, description: 'Text.'}],", + '};', + '', + ].join('\n'), + ); + write( + 'docs/setup.doc.mjs', + [ + 'export default {', + " type: 'generic',", + " name: 'setup',", + " title: 'Setup',", + " description: 'How.',", + ' extra: true,', + " sections: [{title: 'One', content: [{type: 'para', text: 'x'}]}],", + '};', + '', + ].join('\n'), + ); + + const report = await check([ + ['components/Badge.doc.mjs', 'components'], + ['docs/setup.doc.mjs', 'docs'], + ]); + + expect(summary(report)).toEqual([ + 'components/Badge.doc.mjs default displayName missing_field', + 'components/Badge.doc.mjs default keyword unknown_field', + 'components/Badge.doc.mjs default props[0].type wrong_type', + 'docs/setup.doc.mjs default extra unknown_field', + 'docs/setup.doc.mjs default sections[0].content[0].type wrong_type', + ]); + }); + + it('reports a missing top-level field that nested missing fields hid, and quotes no edited text', async () => { + withTypeScript(); + write( + 'components/Hide.doc.mjs', + [ + 'export default {', + " type: 'component',", + " name: 'Hide',", + ' usage: {bestPractices: []},', + " props: [{name: 'label', type: 'string'}],", + '};', + '', + ].join('\n'), + ); + write( + 'components/Bad.doc.mjs', + [ + 'export default {', + " type: 'component',", + " name: 'Bad',", + " displayName: 'Bad',", + " usage: {description: 'x', bestPractices: [{guidance: 42}]},", + ' props: [],', + '};', + '', + ].join('\n'), + ); + + const report = await check([ + ['components/Hide.doc.mjs', 'components'], + ['components/Bad.doc.mjs', 'components'], + ]); + + expect(summary(report)).toEqual([ + 'components/Bad.doc.mjs default usage.bestPractices[0].description missing_field', + 'components/Bad.doc.mjs default usage.bestPractices[0].guidance wrong_type', + 'components/Hide.doc.mjs default displayName missing_field', + 'components/Hide.doc.mjs default usage.description missing_field', + 'components/Hide.doc.mjs default props[0].description missing_field', + ]); + for (const finding of report.findings) { + expect(finding.message).not.toMatch(/\bany\b/); + } + }); + + it('checks a translation that carries props for its fields, not for what only the base doc needs', async () => { + withTypeScript(); + write( + 'components/Group.doc.mjs', + [ + 'export default {', + " type: 'component',", + " name: 'Group',", + " displayName: 'Group',", + " usage: {description: 'Rows.'},", + " props: [{name: 'title', type: 'string', description: 'Heading.'}],", + '};', + 'export const docsZh = {', + " description: '一组行。',", + " props: [{name: 'title', type: 'string', description: '标题。'}],", + '};', + 'export const docsDense = {', + " props: [{name: 'title', type: 42, description: 'Heading.'}],", + '};', + '', + ].join('\n'), + ); + + const report = await check([['components/Group.doc.mjs', 'components']]); + + expect(summary(report)).toEqual([ + 'components/Group.doc.mjs docsDense props[0].type wrong_type', + ]); + }); + + it('says a doc that re-exports its value is not followed', async () => { + withTypeScript(); + write( + 'docs/shared.mjs', + `export default ${JSON.stringify({type: 'generic', name: 'shared', title: 'Shared', description: 'x', sections: []})};\n`, + ); + write('docs/again.doc.mjs', "export {default} from './shared.mjs';\n"); + + const report = await check([['docs/again.doc.mjs', 'docs']]); + + expect(report.unchecked).toEqual([ + { + doc: 'docs/again.doc.mjs', + reason: expect.stringContaining('re-exports its doc from ./shared.mjs'), + }, + ]); + }); + + it('reports every unknown field, however many', async () => { + withTypeScript(); + const extra = Array.from({length: 8}, (_, i) => ` extra${i + 1}: ${i},`); + write( + 'components/Badge.doc.mjs', + [ + `export default {...${JSON.stringify(COMPONENT)},`, + ...extra, + '};', + '', + ].join('\n'), + ); + + const report = await check([['components/Badge.doc.mjs', 'components']]); + + expect(summary(report)).toEqual( + Array.from( + {length: 8}, + (_, i) => + `components/Badge.doc.mjs default extra${i + 1} unknown_field`, + ), + ); + expect(report.unchecked).toEqual([]); + }); + + it('passes a component doc that matches ComponentDoc, with a field another member declares', async () => { + withTypeScript(); + write( + 'components/Badge.doc.mjs', + `export default ${JSON.stringify({...COMPONENT, description: 'A count chip.'})};\n`, + ); + + const report = await check([['components/Badge.doc.mjs', 'components']]); + + expect(report.status).toBe('checked'); + expect(report.typescript).toMatch(/^\d+\.\d+/); + expect(report.docs).toBe(1); + expect(report.findings).toEqual([]); + }); + + it('names an unknown field, and the field TypeScript suggests instead', async () => { + withTypeScript(); + write( + 'components/Badge.doc.mjs', + [ + `export default {...${JSON.stringify(COMPONENT)},`, + " keyword: ['count'],", + '};', + '', + ].join('\n'), + ); + + const report = await check([['components/Badge.doc.mjs', 'components']]); + + expect(report.findings).toEqual([ + { + doc: 'components/Badge.doc.mjs', + line: 2, + export: 'default', + type: 'ComponentDoc', + field: 'keyword', + problem: 'unknown_field', + severity: 'warning', + message: + "Object literal may only specify known properties, but 'keyword' does not exist in type 'SingleComponentDoc'. Did you mean to write 'keywords'?", + fix: 'Rename `keyword` to `keywords`.', + }, + ]); + }); + + it('names a value outside its type, deep in the doc, at its line', async () => { + withTypeScript(); + write( + 'components/Badge.doc.mjs', + [ + 'export default {', + " type: 'component',", + " name: 'Badge',", + " displayName: 'Badge',", + ' usage: {', + " description: 'Shows a count.',", + " bestPractices: [{guidance: true, description: 'Do.'}, {guidance: 'yes', description: 'Dont.'}],", + ' },', + " props: [{name: 'label', type: 'string', description: 'Text.'}],", + '};', + '', + ].join('\n'), + ); + + const report = await check([['components/Badge.doc.mjs', 'components']]); + + expect(report.findings).toEqual([ + expect.objectContaining({ + line: 7, + field: 'usage.bestPractices[1].guidance', + problem: 'wrong_type', + fix: 'Change `usage.bestPractices[1].guidance` to a value of type boolean.', + }), + ]); + }); + + it("checks a doc against its root's type, so a topic stamped beside a component source is flagged", async () => { + withTypeScript(); + write( + 'components/Badge.doc.mjs', + `export default ${JSON.stringify({...COMPONENT, type: 'generic'})};\n`, + ); + + const report = await check([['components/Badge.doc.mjs', 'components']]); + + expect(report.findings).toEqual([ + expect.objectContaining({ + field: 'type', + problem: 'wrong_type', + fix: "This doc is in the components root, so it is a component doc: set type: 'component', or move it to the docs root.", + }), + ]); + }); + + it("ignores the author's own annotation and checks the value once, against the root's type", async () => { + withTypeScript(); + write( + 'components/Badge.doc.mjs', + [ + "/** @type {import('@astryxdesign/cli/authoring').ReferenceDoc} */", + `export const docs = ${JSON.stringify({...COMPONENT, keyword: []})};`, + '', + ].join('\n'), + ); + + const report = await check([['components/Badge.doc.mjs', 'components']]); + + expect(summary(report)).toEqual([ + 'components/Badge.doc.mjs docs keyword unknown_field', + ]); + }); + + it('checks translations: a string overlay against ComponentTranslationDoc, a full one against ComponentDoc with no field required', async () => { + withTypeScript(); + write( + 'components/Badge.doc.mjs', + [ + `export default ${JSON.stringify(COMPONENT)};`, + "export const docsDense = {name: 'Badge', description: 'count chip'};", + `export const docsZh = ${JSON.stringify({...COMPONENT, displayName: undefined, keyword: []})};`, + '', + ].join('\n'), + ); + + const report = await check([['components/Badge.doc.mjs', 'components']]); + + expect(report.docs).toBe(3); + expect(summary(report)).toEqual([ + 'components/Badge.doc.mjs docsDense name unknown_field', + 'components/Badge.doc.mjs docsZh keyword unknown_field', + ]); + expect(report.findings[0].type).toBe('ComponentTranslationDoc'); + }); + + it('checks a template, a docs topic by its stamp, a topic overlay, and a theme', async () => { + withTypeScript(); + write( + 'templates/Hero.doc.mjs', + "export default {type: 'block', name: 'Hero', description: 'A hero.', category: 'components/Hero'};\n", + ); + write( + 'docs/setup.doc.mjs', + "export default {type: 'generic', name: 'setup', title: 'Setup', description: 'How.', sections: [{title: 'One', content: [{type: 'prose', txt: 'x'}]}]};\n", + ); + write( + 'docs/setup.doc.zh.mjs', + "export const docsZh = {description: '如何', sections: [{section: 'One', title: '一'}]};\n", + ); + write( + 'themes/ocean/oceanTheme.doc.mjs', + "export default {type: 'theme', name: 'ocean', displayName: 'Ocean', description: 'Blue.', maintained: 'yes'};\n", + ); + + const report = await check([ + ['templates/Hero.doc.mjs', 'templates'], + ['docs/setup.doc.mjs', 'docs'], + ['docs/setup.doc.zh.mjs', 'docs-overlay'], + ['themes/ocean/oceanTheme.doc.mjs', 'themes'], + ]); + + // An integration template's own category is allowed; its missing + // displayName and aspectRatio are not. + // The topic's unknown `txt` hides its missing `text` from TypeScript; + // the check looks again without it, so both are reported. + expect(summary(report)).toEqual([ + 'docs/setup.doc.mjs default sections[0].content[0].text missing_field', + 'docs/setup.doc.mjs default sections[0].content[0].txt unknown_field', + 'docs/setup.doc.zh.mjs docsZh sections[0].content missing_field', + 'templates/Hero.doc.mjs default aspectRatio missing_field', + 'templates/Hero.doc.mjs default displayName missing_field', + 'themes/ocean/oceanTheme.doc.mjs default maintained wrong_type', + ]); + expect(new Set(report.findings.map(f => f.type))).toEqual( + new Set([ + 'ReferenceDoc', + 'ReferenceTranslationDoc', + 'TemplateDoc', + 'ThemeDoc', + ]), + ); + }); + + it('checks nothing with a TypeScript older than 4.9, even when every doc is TypeScript', async () => { + const fake = path.join(pkg, 'node_modules', 'typescript'); + fs.mkdirSync(fake, {recursive: true}); + fs.writeFileSync( + path.join(fake, 'package.json'), + JSON.stringify({name: 'typescript', version: '4.8.4', main: 'index.js'}), + ); + fs.writeFileSync( + path.join(fake, 'index.js'), + `module.exports = {...require(${JSON.stringify(TYPESCRIPT_DIR)}), version: '4.8.4', versionMajorMinor: '4.8'};\n`, + ); + write( + 'components/Badge.doc.ts', + `export default ${JSON.stringify(COMPONENT)};\n`, + ); + + const report = await check([['components/Badge.doc.ts', 'components']]); + + expect(report).toMatchObject({status: 'skipped', docs: 0, findings: []}); + expect(report.reason).toContain('needs TypeScript 4.9 or later'); + }); + + it('names a doc the compiler cannot parse as not checked, never as a pass', async () => { + withTypeScript(); + write( + 'components/Badge.doc.ts', + "export default {type: 'component', name: 'Badge',, displayName: 'Badge'};\n", + ); + + const report = await check([['components/Badge.doc.ts', 'components']]); + + expect(report.docs).toBe(0); + expect(report.findings).toEqual([]); + expect(report.unchecked).toEqual([ + expect.objectContaining({ + doc: 'components/Badge.doc.ts', + reason: expect.stringContaining('cannot parse it'), + }), + ]); + }); + + it('names a file that exports no doc as not checked', async () => { + withTypeScript(); + write('components/Empty.doc.mjs', 'export const other = {};\n'); + + const report = await check([['components/Empty.doc.mjs', 'components']]); + + expect(report.findings).toEqual([]); + expect(report.unchecked).toEqual([ + { + doc: 'components/Empty.doc.mjs', + reason: 'it exports no doc the CLI reads (default or docs)', + }, + ]); + }); +}); From c7c760cfe40b838434b24cd47967105e99be4480 Mon Sep 17 00:00:00 2001 From: Joey Farina Date: Sat, 17 Oct 2026 20:03:58 -0700 Subject: [PATCH 2/3] fix(cli): name an installed TypeScript the doc type check cannot use, and relax translated entries With only a TypeScript that has no JavaScript compiler API installed (TypeScript 7), the check said TypeScript was not installed and asked for it. It now names the installed version and says it cannot read types with it. A translation that carries props is no longer told to add each prop's `type`: props and sub-component entries merge onto the base doc by name, which keeps each prop's type, so a translated entry needs only its name. --- .../doctypes/template/template.doc.mjs | 2 +- .../cli/foundation/discovery/doc-types.mjs | 25 ++++++-- .../foundation/discovery/doc-types.test.mjs | 63 ++++++++++++++++++- 3 files changed, 80 insertions(+), 10 deletions(-) diff --git a/packages/cli/authoring/doctypes/template/template.doc.mjs b/packages/cli/authoring/doctypes/template/template.doc.mjs index cf38551d445aa..d8310fa43f39b 100644 --- a/packages/cli/authoring/doctypes/template/template.doc.mjs +++ b/packages/cli/authoring/doctypes/template/template.doc.mjs @@ -178,7 +178,7 @@ export default { }, { type: 'prose', - text: "`category` uses the shared TemplateCategory taxonomy: 'Group - Variant' strings (e.g. 'Table - Bulk Actions'). Not every value maps to an existing template; unused values are reserved so authors get autocomplete for the full taxonomy.", + text: "For a Core template, `category` uses the shared TemplateCategory taxonomy: 'Group - Variant' strings (e.g. 'Table - Bulk Actions'). Not every value maps to an existing template; unused values are reserved so authors get autocomplete for the full taxonomy. An integration template may use its own value.", }, ], }; diff --git a/packages/cli/foundation/discovery/doc-types.mjs b/packages/cli/foundation/discovery/doc-types.mjs index 3258e5c629f69..b8652d418245e 100644 --- a/packages/cli/foundation/discovery/doc-types.mjs +++ b/packages/cli/foundation/discovery/doc-types.mjs @@ -17,8 +17,9 @@ * - components root: `ComponentDoc` (read as TypeScript reads the union, with * the member the doc's shape names, so messages are about that member); its * `docsZh` and `docsDense` translations against `ComponentTranslationDoc`, - * or, with no field required, `ComponentDoc` when a translation carries - * props, as the CLI overlays them on the base doc; + * or, when a translation carries props, `ComponentDoc` with no field + * required but each prop's and sub-component entry's `name`, as the CLI + * overlays them on the base doc by name; * - hooks (Core): `HookDoc`, translations against `HookTranslationDoc`; * - templates root: `TemplateDoc`; * - docs root: the type the doc's `type` names (`ReferenceDoc` when it names @@ -37,7 +38,10 @@ import * as fs from 'node:fs'; import * as path from 'node:path'; import {fileURLToPath} from 'node:url'; -import {resolveProjectTypeScript} from './project-typescript.mjs'; +import { + resolveProjectTypeScript, + unusableTypeScriptReason, +} from './project-typescript.mjs'; /** * @typedef {'components' | 'hooks' | 'templates' | 'docs' | 'docs-overlay' | 'themes'} DocRoot @@ -261,9 +265,16 @@ function expectedType(ts, root, exportName, node) { } // A translation with props overlays the base doc, which supplies every // field it leaves out: each field it writes must match, none is required. + // Props and sub-component entries merge by `name`, the base doc keeping + // each prop's `type`, so an entry needs only its `name`. if (translation) { const full = componentType(shape); - return {typeExpr: `Partial<${full.typeExpr}>`, typeName: full.typeName}; + const prop = `Partial<${authoring('ComponentPropDoc')}> & {name: string}`; + const entry = `Partial> & {name: string; props?: Array<${prop}>}`; + return { + typeExpr: `Omit, 'props' | 'components'> & {props?: Array<${prop}>; components?: Array<${entry}>}`, + typeName: full.typeName, + }; } return componentType(shape); } @@ -879,10 +890,12 @@ export async function checkDocTypes(files, options) { return {status: 'checked', docs: 0, findings: [], unchecked: []}; } const compiler = resolveProjectTypeScript(resolveFrom); - if (!compiler) { + if (!compiler?.ts) { return { status: 'skipped', - reason: DOC_TYPES_TYPESCRIPT_MISSING_REASON, + reason: compiler + ? unusableTypeScriptReason(compiler.version, 'docs were not checked') + : DOC_TYPES_TYPESCRIPT_MISSING_REASON, docs: 0, findings: [], unchecked: [], diff --git a/packages/cli/foundation/discovery/doc-types.test.mjs b/packages/cli/foundation/discovery/doc-types.test.mjs index ed17541b89f4e..4c220fdb9fafe 100644 --- a/packages/cli/foundation/discovery/doc-types.test.mjs +++ b/packages/cli/foundation/discovery/doc-types.test.mjs @@ -11,6 +11,7 @@ import * as os from 'node:os'; import * as path from 'node:path'; import {createRequire} from 'node:module'; import {afterEach, beforeEach, describe, expect, it} from 'vitest'; +import {unusableTypeScriptReason} from './project-typescript.mjs'; import { checkDocTypes, DOC_TYPES_TYPESCRIPT_MISSING_REASON, @@ -92,6 +93,40 @@ describe('checkDocTypes', () => { }); }); + it('names an installed TypeScript it cannot use, rather than asking to install one', async () => { + write( + 'components/Badge.doc.mjs', + `export default ${JSON.stringify(COMPONENT)};\n`, + ); + // TypeScript 7 has no JavaScript compiler API: one entry exports none, + // another cannot be loaded at all. + for (const stub of [ + {main: 'index.js', code: "module.exports = {version: '7.0.0'};\n"}, + {exports: {'./package.json': './package.json'}}, + ]) { + const dir = path.join(pkg, 'node_modules', 'typescript'); + fs.rmSync(dir, {recursive: true, force: true}); + fs.mkdirSync(dir, {recursive: true}); + const {code, ...fields} = stub; + fs.writeFileSync( + path.join(dir, 'package.json'), + JSON.stringify({name: 'typescript', version: '7.0.0', ...fields}), + ); + if (code) fs.writeFileSync(path.join(dir, 'index.js'), code); + + const report = await check([['components/Badge.doc.mjs', 'components']]); + + expect(report).toEqual({ + status: 'skipped', + reason: unusableTypeScriptReason('7.0.0', 'docs were not checked'), + docs: 0, + findings: [], + unchecked: [], + }); + expect(report.reason).toContain('TypeScript 7.0.0 is installed'); + } + }); + it('says there is nothing to check when the package has no doc files, with or without TypeScript', async () => { const without = await check([]); withTypeScript(); @@ -238,19 +273,41 @@ describe('checkDocTypes', () => { '};', 'export const docsZh = {', " description: '一组行。',", - " props: [{name: 'title', type: 'string', description: '标题。'}],", + " props: [{name: 'title', description: '标题。'}],", '};', 'export const docsDense = {', - " props: [{name: 'title', type: 42, description: 'Heading.'}],", + " props: [{name: 'title', type: 42, description: 'Heading.'}, {description: 'No name.'}],", + '};', + '', + ].join('\n'), + ); + // The overlay merges props, and sub-component entries, by name and keeps + // the base doc's `type`: a translated entry needs only its `name`. + write( + 'components/Family.doc.mjs', + [ + 'export default {', + " type: 'component',", + " name: 'Family',", + " displayName: 'Family',", + " usage: {description: 'Members.'},", + " components: [{name: 'Member', description: 'One.', props: [{name: 'size', type: 'number', description: 'Size.'}]}],", + '};', + 'export const docsZh = {', + " components: [{name: 'Member', props: [{name: 'size', description: '大小。'}]}],", '};', '', ].join('\n'), ); - const report = await check([['components/Group.doc.mjs', 'components']]); + const report = await check([ + ['components/Group.doc.mjs', 'components'], + ['components/Family.doc.mjs', 'components'], + ]); expect(summary(report)).toEqual([ 'components/Group.doc.mjs docsDense props[0].type wrong_type', + 'components/Group.doc.mjs docsDense props[1].name missing_field', ]); }); From 2d52ec0f0223ff5b17630c1f89074023eea67d4a Mon Sep 17 00:00:00 2001 From: Joey Farina Date: Sat, 17 Oct 2026 23:52:14 -0700 Subject: [PATCH 3/3] feat(cli): check integration templates against a new IntegrationTemplateDoc, keep TemplateDoc as it is, and set aside a .doc.ts doc's own annotation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `TemplateDoc` is back to main's, `category` a `TemplateCategory`. A new `IntegrationTemplateDoc` (every `TemplateDoc` field, with the package's own `category` string), exported from `@astryxdesign/cli/authoring` with its own section in `astryx docs authoring`, is what the doc type check holds an integration's templates to; Core's own templates keep `TemplateDoc`. A `.doc.ts` doc's own annotation (`const docs: ReferenceDoc = …`) and type assertion (`{…}`) are set aside the way a JSDoc `@type` is, so the value is checked once, against its root's type. --- .changeset/integration-template-doc.md | 7 ++ .changeset/template-doc-category.md | 7 -- .../template/integration-template.doc.mjs | 49 ++++++++++ .../cli/authoring/doctypes/template/parse.mjs | 9 +- .../doctypes/template/template.doc.mjs | 8 +- .../cli/authoring/doctypes/template/type.ts | 30 +++++-- packages/cli/authoring/index.d.ts | 1 + .../discovery/authoring-self-docs.mjs | 1 + .../cli/foundation/discovery/doc-types.mjs | 89 +++++++++++++------ .../foundation/discovery/doc-types.test.mjs | 86 +++++++++++++++--- 10 files changed, 225 insertions(+), 62 deletions(-) create mode 100644 .changeset/integration-template-doc.md delete mode 100644 .changeset/template-doc-category.md create mode 100644 packages/cli/authoring/doctypes/template/integration-template.doc.mjs diff --git a/.changeset/integration-template-doc.md b/.changeset/integration-template-doc.md new file mode 100644 index 0000000000000..5ca59d97b0bb4 --- /dev/null +++ b/.changeset/integration-template-doc.md @@ -0,0 +1,7 @@ +--- +'@astryxdesign/cli': patch +--- + +[feat] Three new published types describe an integration package's templates: `IntegrationPageTemplateDoc` and `IntegrationBlockTemplateDoc`, each with every field of its `TemplateDoc` member but `category` as the package's own string instead of a `TemplateCategory` value, and their union, `IntegrationTemplateDoc`. `@astryxdesign/cli/authoring` exports `IntegrationTemplateDoc`; `@astryxdesign/cli/doc` and `@astryxdesign/cli/template` export all three. `TemplateDoc` is unchanged. + +@josephfarina diff --git a/.changeset/template-doc-category.md b/.changeset/template-doc-category.md deleted file mode 100644 index 13b21a7d03dbd..0000000000000 --- a/.changeset/template-doc-category.md +++ /dev/null @@ -1,7 +0,0 @@ ---- -'@astryxdesign/cli': patch ---- - -[fix] The published `TemplateDoc` type admits an integration template's own `category`, which the loader always accepted and bare `astryx template` groups by; Core templates keep the `TemplateCategory` values. - -@josephfarina diff --git a/packages/cli/authoring/doctypes/template/integration-template.doc.mjs b/packages/cli/authoring/doctypes/template/integration-template.doc.mjs new file mode 100644 index 0000000000000..91c5083b6e5e3 --- /dev/null +++ b/packages/cli/authoring/doctypes/template/integration-template.doc.mjs @@ -0,0 +1,49 @@ +// Copyright (c) Meta Platforms, Inc. and affiliates. + +/** + * @file SchemaDoc for the `IntegrationTemplateDoc` doc-type — the template + * metadata an integration package ships: TemplateDoc, with the package's own + * `category`. Colocated with the type it describes (`type.ts`). + * @position packages/cli/authoring/doctypes/template — doc-type documentation + */ + +/** @type {import('@astryxdesign/cli/authoring').SchemaDoc} */ +export const doc = { + type: 'schema', + name: 'integration-template-doc', + displayName: 'IntegrationTemplateDoc', + namespace: 'authoring', + description: + "The doc-type for an integration package's template metadata. It has every " + + "TemplateDoc field and rule, except that `category` is the package's own " + + 'string instead of a TemplateCategory value.', + appliesTo: '.doc.mjs', + fields: [ + { + name: 'category', + type: 'string', + description: + "The package's own category. Bare `astryx template` groups the package's templates by it.", + }, + ], + examples: [ + { + label: 'Block template in an integration package', + code: `/** @type {import('@astryxdesign/cli/authoring').IntegrationTemplateDoc} */ +export default { + type: 'block', + name: 'acme-pricing-table', + displayName: 'Pricing Table', + description: 'Plans side by side, with the chosen plan highlighted.', + aspectRatio: 16 / 9, + category: 'Acme - Commerce', +};`, + }, + ], + notes: [ + { + type: 'prose', + text: "Every other field, and the page and block variants, are TemplateDoc's. Core's own templates use TemplateDoc, whose `category` is a TemplateCategory value.", + }, + ], +}; diff --git a/packages/cli/authoring/doctypes/template/parse.mjs b/packages/cli/authoring/doctypes/template/parse.mjs index d7322931fae21..3208d80a26678 100644 --- a/packages/cli/authoring/doctypes/template/parse.mjs +++ b/packages/cli/authoring/doctypes/template/parse.mjs @@ -76,10 +76,11 @@ const blockTemplateSchema = z /** * Templates as they load. Integration templates already published omit - * `displayName` and `aspectRatio`, so the loader accepts those (template - * discovery falls back to an aspect ratio of 1); the published types keep - * asking authors for both. `category` is any string in both: an integration - * template groups itself under its own. + * `displayName` and `aspectRatio` and group themselves under their own + * `category`, so the loader accepts those (template discovery falls back to an + * aspect ratio of 1). The published `TemplateDoc` keeps asking authors for all + * three; `IntegrationTemplateDoc`, for an integration package's templates, + * takes the package's own `category`. * * @typedef {Omit * & {displayName?: string, category?: string}} LoadedPageTemplateDoc diff --git a/packages/cli/authoring/doctypes/template/template.doc.mjs b/packages/cli/authoring/doctypes/template/template.doc.mjs index d8310fa43f39b..10b4c2799a792 100644 --- a/packages/cli/authoring/doctypes/template/template.doc.mjs +++ b/packages/cli/authoring/doctypes/template/template.doc.mjs @@ -78,9 +78,9 @@ export const doc = { }, { name: 'category', - type: 'TemplateCategory | string', + type: 'TemplateCategory', description: - "Functional category. Core templates use a TemplateCategory value following a 'Group - Variant' convention (e.g. 'Dashboard - Analytics', 'Table - Basic', 'Form - Wizard'); the overview groups by the text before ' - '. An integration template may use its own value, and bare `astryx template` groups templates by it.", + "Functional gallery category following a 'Group - Variant' convention (e.g. 'Dashboard - Analytics', 'Table - Basic', 'Form - Wizard'). The overview groups by the text before ' - '.", }, { name: 'isHiddenFromOverview', @@ -162,7 +162,7 @@ export default { notes: [ { type: 'prose', - text: 'When it loads, a template may leave out `displayName` and `aspectRatio`, as integration templates already published do; a block with no `aspectRatio` previews at 1. Write to the type anyway; it is the contract.', + text: 'When it loads, a template may leave out `displayName` and `aspectRatio` and use its own `category`, as integration templates already published do; a block with no `aspectRatio` previews at 1. Write to the type anyway; it is the contract.', }, { type: 'prose', @@ -178,7 +178,7 @@ export default { }, { type: 'prose', - text: "For a Core template, `category` uses the shared TemplateCategory taxonomy: 'Group - Variant' strings (e.g. 'Table - Bulk Actions'). Not every value maps to an existing template; unused values are reserved so authors get autocomplete for the full taxonomy. An integration template may use its own value.", + text: "`category` uses the shared TemplateCategory taxonomy: 'Group - Variant' strings (e.g. 'Table - Bulk Actions'). Not every value maps to an existing template; unused values are reserved so authors get autocomplete for the full taxonomy.", }, ], }; diff --git a/packages/cli/authoring/doctypes/template/type.ts b/packages/cli/authoring/doctypes/template/type.ts index 9de096ae0a097..710f48cb27af0 100644 --- a/packages/cli/authoring/doctypes/template/type.ts +++ b/packages/cli/authoring/doctypes/template/type.ts @@ -56,12 +56,10 @@ export interface BaseTemplateDoc extends AuthoredDocGraphFields { * browsable template galleries like the craft browser. */ scaffold?: boolean; - /** Functional category. Core templates use a `TemplateCategory` value, and - * the docsite Templates overview gallery groups them by the part before - * `" - "` (e.g. `"Dashboard"`). An integration template may use its own - * value instead: bare `astryx template` groups templates by this field. + /** Functional category for the docsite Templates overview gallery. + * Templates are grouped by the part before `" - "` (e.g. `"Dashboard"`). * Independent of CLI discovery, which uses `name`/`description`. */ - category?: TemplateCategory | (string & {}); + category?: TemplateCategory; /** Boolean opt-out for templates that shouldn't appear on the Templates * overview gallery. The template stays available via the CLI and @@ -203,3 +201,25 @@ export type TemplateCategory = | 'Shell - Blank'; export type TemplateDoc = PageTemplateDoc | BlockTemplateDoc; + +/** A page template an integration package ships, with its own `category`. */ +export interface IntegrationPageTemplateDoc extends Omit< + PageTemplateDoc, + 'category' +> { + category?: string; +} + +/** A block template an integration package ships, with its own `category`. */ +export interface IntegrationBlockTemplateDoc extends Omit< + BlockTemplateDoc, + 'category' +> { + category?: string; +} + +/** The doc-type for an integration package's template metadata: TemplateDoc, + * with `category` the package's own string (bare `astryx template` groups + * the package's templates by it) instead of a TemplateCategory value. */ +export type IntegrationTemplateDoc = + IntegrationPageTemplateDoc | IntegrationBlockTemplateDoc; diff --git a/packages/cli/authoring/index.d.ts b/packages/cli/authoring/index.d.ts index 89843a7c83b66..2c8c58afd5eef 100644 --- a/packages/cli/authoring/index.d.ts +++ b/packages/cli/authoring/index.d.ts @@ -28,6 +28,7 @@ export type {HookDoc} from './doctypes/types.js'; // useToast.doc.mjs export type {FunctionDoc} from './doctypes/types.js'; // search.doc.mjs (hook | api) export type {ReferenceDoc} from './doctypes/types.js'; // theming.doc.mjs export type {TemplateDoc} from './doctypes/types.js'; // Foo.doc.mjs +export type {IntegrationTemplateDoc} from './doctypes/types.js'; // an integration's Foo.doc.mjs export type {SchemaDoc} from './doctypes/types.js'; // config.doc.mjs (object shape) export type {CommandDoc} from './doctypes/types.js'; // search.doc.mjs (CLI command) export type {EnumDoc} from './doctypes/types.js'; // error-codes.doc.mjs (vocabulary) diff --git a/packages/cli/foundation/discovery/authoring-self-docs.mjs b/packages/cli/foundation/discovery/authoring-self-docs.mjs index 7e69af012c2ee..a9bb90060dd2c 100644 --- a/packages/cli/foundation/discovery/authoring-self-docs.mjs +++ b/packages/cli/foundation/discovery/authoring-self-docs.mjs @@ -46,6 +46,7 @@ export const AUTHORING_SELF_DOCS = [ 'doctypes/reference/reference.doc.mjs', 'doctypes/schema/schema.doc.mjs', 'doctypes/template/template.doc.mjs', + 'doctypes/template/integration-template.doc.mjs', 'doctypes/theme/theme.doc.mjs', ]; diff --git a/packages/cli/foundation/discovery/doc-types.mjs b/packages/cli/foundation/discovery/doc-types.mjs index b8652d418245e..095f51fbc5988 100644 --- a/packages/cli/foundation/discovery/doc-types.mjs +++ b/packages/cli/foundation/discovery/doc-types.mjs @@ -21,7 +21,8 @@ * required but each prop's and sub-component entry's `name`, as the CLI * overlays them on the base doc by name; * - hooks (Core): `HookDoc`, translations against `HookTranslationDoc`; - * - templates root: `TemplateDoc`; + * - templates root: `IntegrationTemplateDoc` (`TemplateDoc`, whose `category` + * is a `TemplateCategory` value, for Core's own templates); * - docs root: the type the doc's `type` names (`ReferenceDoc` when it names * none), and a `.doc..mjs` overlay against * `ReferenceTranslationDoc`; @@ -124,8 +125,8 @@ const DOCS_ROOT_TYPES = { enum: 'EnumDoc', theme: 'ThemeDoc', component: 'ComponentDoc', - page: 'TemplateDoc', - block: 'TemplateDoc', + page: 'IntegrationTemplateDoc', + block: 'IntegrationTemplateDoc', }; /** @@ -248,9 +249,10 @@ function componentType(shape) { * @param {DocRoot} root * @param {string} exportName * @param {any} node + * @param {boolean} [core] the docs are Core's own * @returns {{typeExpr: string, typeName: string}} */ -function expectedType(ts, root, exportName, node) { +function expectedType(ts, root, exportName, node, core = false) { const shape = shapeOf(ts, node); const translation = TRANSLATIONS.includes(exportName); if (root === 'components') { @@ -294,7 +296,8 @@ function expectedType(ts, root, exportName, node) { return {typeExpr: authoring('HookDoc'), typeName: 'HookDoc'}; } if (root === 'templates') { - return {typeExpr: authoring('TemplateDoc'), typeName: 'TemplateDoc'}; + const typeName = core ? 'TemplateDoc' : 'IntegrationTemplateDoc'; + return {typeExpr: authoring(typeName), typeName}; } if (root === 'docs-overlay') { return { @@ -316,7 +319,10 @@ function expectedType(ts, root, exportName, node) { ? /** @type {Record} */ (DOCS_ROOT_TYPES)[shape.stamp] : undefined; if (named === 'ComponentDoc') return componentType(shape); - const typeName = named ?? 'ReferenceDoc'; + const typeName = + core && named === 'IntegrationTemplateDoc' + ? 'TemplateDoc' + : (named ?? 'ReferenceDoc'); return {typeExpr: authoring(typeName), typeName}; } @@ -406,9 +412,10 @@ function unwrap(ts, expression) { * @param {any} ts * @param {any} sf * @param {DocRoot} root + * @param {boolean} [core] the docs are Core's own * @returns {Target[]} */ -function targetsOf(ts, sf, root) { +function targetsOf(ts, sf, root, core = false) { const exported = moduleExports(ts, sf); /** @type {string[]} */ const names = []; @@ -430,20 +437,33 @@ function targetsOf(ts, sf, root) { /** @type {Array<[number, number]>} */ const blankRanges = []; for (const wrapper of wrappers) { - // ` satisfies X` / ` as X` after the value. - if ( + if (ts.isTypeAssertionExpression?.(wrapper)) { + // `` before the value. + blankRanges.push([ + wrapper.getStart(sf), + wrapper.expression.getStart(sf), + ]); + } else if ( + // ` satisfies X` / ` as X` after the value. !ts.isParenthesizedExpression(wrapper) && + !ts.isNonNullExpression(wrapper) && wrapper.end > wrapper.expression.end ) { - if (!ts.isNonNullExpression(wrapper)) { - blankRanges.push([wrapper.expression.end, wrapper.end]); - } + blankRanges.push([wrapper.expression.end, wrapper.end]); + } + } + // The variable's own annotation: `const docs: ReferenceDoc = …`. + for (const declaration of ts.isVariableStatement(entry.statement) + ? entry.statement.declarationList.declarations + : []) { + if (declaration.initializer === entry.expression && declaration.type) { + blankRanges.push([declaration.name.end, declaration.type.end]); } } targets.push({ exportName, node, - ...expectedType(ts, root, exportName, node), + ...expectedType(ts, root, exportName, node, core), blank: [entry.statement, ...wrappers], blankRanges, }); @@ -880,11 +900,12 @@ function findingsFor(ts, diagnostic, astPath, where) { /** * Check each doc file's values against the published authoring types. * @param {DocFile[]} files - * @param {{packageDir: string, resolveFrom: Array}} options + * @param {{packageDir: string, resolveFrom: Array, core?: boolean}} options + * `core`: the docs are Core's own, so templates are held to TemplateDoc * @returns {Promise} */ export async function checkDocTypes(files, options) { - const {packageDir, resolveFrom} = options; + const {packageDir, resolveFrom, core = false} = options; // With no doc files there is nothing to check, TypeScript or not. if (files.length === 0) { return {status: 'checked', docs: 0, findings: [], unchecked: []}; @@ -956,7 +977,7 @@ export async function checkDocTypes(files, options) { true, isTs ? ts.ScriptKind.TS : ts.ScriptKind.JS, ); - const targets = targetsOf(ts, sf, file.root); + const targets = targetsOf(ts, sf, file.root, core); if (targets.length === 0) { const names = MAIN_EXPORTS[file.root]; const from = reexportSource(ts, sf, [...names, ...TRANSLATIONS]); @@ -1029,6 +1050,7 @@ export async function checkDocTypes(files, options) { /** @type {DocTypeFinding[]} */ const findings = []; + const stamped = new Set(); // docs-root docs whose `type` names no type let checked = [...instrumented.values()].reduce( (sum, entry) => sum + entry.targets.length, 0, @@ -1037,8 +1059,7 @@ export async function checkDocTypes(files, options) { // TypeScript stops at an object's first unknown field, and a value outside // its type hides the rest of its object, so a doc is checked again with each // unknown field blanked and each such value cast to `any`, until nothing new - // is found. The doc's own `type` is never cast: a wrong one means the doc is - // a different kind, and what the other kind would report is a guess. + // is found, the doc's own `type` too where its root fixes its type. /** @type {Entry[]} */ let pending = [...instrumented.values()]; /** @type {any} */ @@ -1115,15 +1136,26 @@ export async function checkDocTypes(files, options) { entry.casts.push(range); changed = true; } - } else if ( - finding.problem === 'wrong_type' && - finding.field !== 'type' - ) { - // A deeper `type` picks the shape of the object that holds it (a - // content block, a prop), so that whole object is set aside. - const range = /(^|\.)type$/.test(finding.field) - ? objectRangeAt(ts, entry.sf, target.node, pos) - : valueRangeAt(ts, entry.sf, target.node, pos); + } else if (finding.problem === 'wrong_type') { + // A deeper `type` picks the shape of the object that holds it, so + // that object is set aside; a docs-root doc's own `type` is not. + const picked = + finding.field === 'type' && + entry.file.root === 'docs' && + !TRANSLATIONS.includes(target.exportName); + if (picked && !stamped.has(doc)) { + stamped.add(doc); + unchecked.push({ + doc, + reason: + 'its `type` names no type the docs root takes, so its other fields were not checked; fix the `type` and run the check again', + }); + } + const range = picked + ? null + : /\.type$/.test(finding.field) + ? objectRangeAt(ts, entry.sf, target.node, pos) + : valueRangeAt(ts, entry.sf, target.node, pos); if (range && !entry.casts.some(([from]) => from === range[0])) { entry.casts.push(range); changed = true; @@ -1169,7 +1201,8 @@ export async function checkDocTypes(files, options) { status: 'checked', typescript: version, docs: checked, - findings, + // A docs-root doc whose `type` names no type reports only that. + findings: findings.filter(f => !stamped.has(f.doc) || f.field === 'type'), unchecked, }; } diff --git a/packages/cli/foundation/discovery/doc-types.test.mjs b/packages/cli/foundation/discovery/doc-types.test.mjs index 4c220fdb9fafe..ebcb8a7f2727e 100644 --- a/packages/cli/foundation/discovery/doc-types.test.mjs +++ b/packages/cli/foundation/discovery/doc-types.test.mjs @@ -53,11 +53,12 @@ function write(rel, text) { /** * @param {Array<[string, import('./doc-types.mjs').DocRoot]>} files + * @param {{core?: boolean}} [options] */ -function check(files) { +function check(files, options = {}) { return checkDocTypes( files.map(([rel, root]) => ({path: path.join(pkg, rel), root})), - {packageDir: pkg, resolveFrom: [pkg]}, + {packageDir: pkg, resolveFrom: [pkg], ...options}, ); } @@ -430,21 +431,40 @@ describe('checkDocTypes', () => { ]); }); - it("checks a doc against its root's type, so a topic stamped beside a component source is flagged", async () => { + it("checks a doc against its root's type, so a topic stamped beside a component source is flagged, with the rest of the doc", async () => { withTypeScript(); + const {displayName, ...rest} = COMPONENT; write( 'components/Badge.doc.mjs', - `export default ${JSON.stringify({...COMPONENT, type: 'generic'})};\n`, + `export default ${JSON.stringify({...rest, type: 'generic', colour: 'red'})};\n`, + ); + // In the docs root the `type` picks the doc's type, so a wrong one leaves + // the rest of it unchecked. + write( + 'docs/setup.doc.mjs', + "export default {type: 'generc', name: 'setup', title: 1, colour: 'red'};\n", ); - const report = await check([['components/Badge.doc.mjs', 'components']]); + const report = await check([ + ['components/Badge.doc.mjs', 'components'], + ['docs/setup.doc.mjs', 'docs'], + ]); - expect(report.findings).toEqual([ - expect.objectContaining({ - field: 'type', - problem: 'wrong_type', - fix: "This doc is in the components root, so it is a component doc: set type: 'component', or move it to the docs root.", - }), + expect(summary(report)).toEqual([ + 'components/Badge.doc.mjs default colour unknown_field', + 'components/Badge.doc.mjs default displayName missing_field', + 'components/Badge.doc.mjs default type wrong_type', + 'docs/setup.doc.mjs default type wrong_type', + ]); + expect(report.findings[2].fix).toBe( + "This doc is in the components root, so it is a component doc: set type: 'component', or move it to the docs root.", + ); + expect(report.unchecked).toEqual([ + { + doc: 'docs/setup.doc.mjs', + reason: + 'its `type` names no type the docs root takes, so its other fields were not checked; fix the `type` and run the check again', + }, ]); }); @@ -466,6 +486,31 @@ describe('checkDocTypes', () => { ]); }); + it("ignores a .doc.ts doc's own annotation and type assertion, as it does a JSDoc one", async () => { + withTypeScript(); + const header = + "import type {ReferenceDoc} from '@astryxdesign/cli/authoring';"; + const doc = JSON.stringify({...COMPONENT, keyword: []}); + write( + 'components/Badge.doc.ts', + `${header}\nexport const docs: ReferenceDoc = ${doc};\n`, + ); + write( + 'components/Chip.doc.ts', + `${header}\nexport default ${doc};\n`, + ); + + const report = await check([ + ['components/Badge.doc.ts', 'components'], + ['components/Chip.doc.ts', 'components'], + ]); + + expect(summary(report)).toEqual([ + 'components/Badge.doc.ts docs keyword unknown_field', + 'components/Chip.doc.ts default keyword unknown_field', + ]); + }); + it('checks translations: a string overlay against ComponentTranslationDoc, a full one against ComponentDoc with no field required', async () => { withTypeScript(); write( @@ -514,8 +559,8 @@ describe('checkDocTypes', () => { ['themes/ocean/oceanTheme.doc.mjs', 'themes'], ]); - // An integration template's own category is allowed; its missing - // displayName and aspectRatio are not. + // An integration template's own category is allowed (IntegrationTemplateDoc); + // its missing displayName and aspectRatio are not. // The topic's unknown `txt` hides its missing `text` from TypeScript; // the check looks again without it, so both are reported. expect(summary(report)).toEqual([ @@ -530,10 +575,23 @@ describe('checkDocTypes', () => { new Set([ 'ReferenceDoc', 'ReferenceTranslationDoc', - 'TemplateDoc', + 'IntegrationTemplateDoc', 'ThemeDoc', ]), ); + + // Core's own templates keep TemplateDoc and its TemplateCategory values. + const core = await check([['templates/Hero.doc.mjs', 'templates']], { + core: true, + }); + expect(summary(core)).toEqual([ + 'templates/Hero.doc.mjs default aspectRatio missing_field', + 'templates/Hero.doc.mjs default category wrong_type', + 'templates/Hero.doc.mjs default displayName missing_field', + ]); + expect(new Set(core.findings.map(f => f.type))).toEqual( + new Set(['TemplateDoc']), + ); }); it('checks nothing with a TypeScript older than 4.9, even when every doc is TypeScript', async () => {