Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/doctor-prop-types.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@astryxdesign/cli': patch
---

[feat] `astryx doctor integration components` also judges each documented prop's type against the code, for props the package declares, a documented field such as `status.type` and a name the package's own index signature takes included, and warns when they differ (`type_mismatch`). A type is judged only where both sides read exactly, with the project's own `typescript`: the same text once whitespace, quotes and union order are set aside, or primitives, literals, arrays and unions of those, and names for them. Every other type (a callback, an object, a generic with arguments, an intersection, `keyof`, a type that does not resolve, a name two index signatures take) is listed as not compared, never reported. The comparison needs TypeScript 4.8 or later: with an older one, no type is judged and the text says so in one `[skip]` line. JSON adds `typesCompared`, `typesNotCompared` and, when the types were not judged, `typesSkipped` to `propDrift`. The findings are warnings, so the exit code does not change.

@josephfarina
2 changes: 1 addition & 1 deletion packages/cli/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -478,7 +478,7 @@ Every response has a `type` discriminant. The full set is below (generated from
| `integration.pack-check` | The packed-package check: name, version, packable, tarball {filename, fileCount, size, unpackedSize} or null, inventory {manifest, roots [{kind, path, expectedFiles, missingFiles, complete}], expectedFiles, packedFiles}, contributions {local, packed}, each null or {themes [{slug, exportName}], components, templates [{id, type, name}], codemods [{version, id}], docs, agentDocsAppend}, and issues [{code, severity, message}]. |
| `integration.validate` | The validation result: validated (false when nothing was validated: with no package.json the issues list carries one no_package error so the command exits 1, and with a package.json but no integration manifest the issues list is empty and proves nothing), the package name and version (both null when validated is false) plus issues, an AstryxIntegrationIssue[] of {code, severity: warning \| error, message}. |
| `integration.template-conflicts` | validated (false when no integration manifest was found, so nothing was inspected), the integration identity, structural issues, and non-blocking Core template-id conflicts as {id, severity: warning, integrationPackage, integrationType, integrationName, coreMatches, message, command}. |
| `integration.component-conflicts` | validated (false when no integration manifest was found, so nothing was inspected), the integration identity, structural issues, and non-blocking conflicts where an integration component name is also owned by Core; each conflict includes the exact package-qualified command. When the manifest loads and declares a components root, propDrift compares documented props with the TypeScript props: status (checked \| skipped), reason, typescript, components, findings (all warnings), unchecked, and propsNotChecked. |
| `integration.component-conflicts` | validated (false when no integration manifest was found, so nothing was inspected), the integration identity, structural issues, and non-blocking conflicts where an integration component name is also owned by Core; each conflict includes the exact package-qualified command. When the manifest loads and declares a components root, propDrift compares documented props with the TypeScript props: status (checked \| skipped), reason, typescript, components, typesCompared, typesSkipped, findings (all warnings), unchecked, typesNotCompared, and propsNotChecked. |
| `integration.doc-conflicts` | validated (false when no integration manifest was found, so nothing was inspected), the integration identity, structural issues, and Core doc overlaps. Each finding includes `severity` (`info` \| `error`) and `relationship` (`replaces` \| `extends` \| `accidental`). |

<!-- END GENERATED: response-types -->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,24 @@ export const doc = {
'declares the CLI range that turns replacement on and warnings otherwise. ' +
"Each component doc's props are compared with the component's TypeScript props, " +
"read with the project's own `typescript`: a documented prop the code lacks, a " +
'prop the package declares that the doc omits, or a wrong `required`. Those ' +
'findings are warnings ' +
'in `propDrift`; without TypeScript it is `skipped` with a reason.',
'prop the package declares that the doc omits, a wrong `required`, or a type ' +
'that does not match. A type is judged only when it reads exactly: the same ' +
'text as the source, once whitespace, quotes and union order are set aside (exactly, if it contains `readonly`), ' +
'or primitives, literals, arrays and unions of those, or a name for one, on ' +
'both sides, a field such as `status.type` included, and for a union of ' +
"props types, the type of any one member. A field the prop's variants give " +
"different types is listed when the doc gives only some variants' types. A " +
'mutable array type matches a readonly one, because callers can pass it; a ' +
'readonly one documented for a mutable array is a mismatch. ' +
'`Name = …` with a union after it ' +
"must match in both parts; one value after it (`Size = 'md'`) is read as a " +
"default. A name only TypeScript's own libraries declare is not resolved, and " +
'`React.X` resolves through React where the prop is declared. Every ' +
'other type is listed as not compared, never reported, and so is a name an ' +
'index signature takes unless exactly one the package declares takes it; ' +
'with a TypeScript older than 4.8 no type is judged. ' +
'Those findings are warnings in `propDrift`; without TypeScript it is ' +
'`skipped` with a reason.',
importPath: '@astryxdesign/cli/api',
signature:
'integrationComponentConflicts(pkg?: string, options?: IntegrationAuthoringOptions): Promise<IntegrationComponentConflictResponse>',
Expand All @@ -42,7 +57,7 @@ export const doc = {
{
type: 'integration.component-conflicts',
description:
'`validated` (false when no integration manifest was found, so nothing was inspected), integration identity, structural and replaces issues, and Core component-name conflicts with package-qualified commands. An active replacement that keeps the Core name it replaces is not a conflict. When the manifest loads and declares a components root, `propDrift` holds the props comparison: `status` (`checked` | `skipped`), `reason` when skipped, `typescript`, `components` compared, `findings` ({component, prop, kind: `not_in_code` | `not_in_doc` | `required_mismatch`, severity: `warning`, message, doc, source, docType?, codeType?, docRequired?, codeRequired?}), `unchecked` ({component, doc, reason}), and `propsNotChecked` when a mapped type builds props from keys the check cannot trace to the package ({component, doc, reason, props}: no warning is drawn for those props).',
'`validated` (false when no integration manifest was found, so nothing was inspected), integration identity, structural and replaces issues, and Core component-name conflicts with package-qualified commands. An active replacement that keeps the Core name it replaces is not a conflict. When the manifest loads and declares a components root, `propDrift` holds the props comparison: `status` (`checked` | `skipped`), `reason` when skipped, `typescript`, `components` compared, `typesCompared` (documented prop types judged), `typesSkipped` (why none was, with a TypeScript older than 4.8), `findings` ({component, prop, kind: `not_in_code` | `not_in_doc` | `required_mismatch` | `type_mismatch`, severity: `warning`, message, doc, source, docType?, codeType?, docRequired?, codeRequired?}), `unchecked` ({component, doc, reason}), `typesNotCompared` ({component, doc, reason, props}: every other documented prop type, grouped by why it was not judged: not a form the check judges exactly (a doc type that does not parse, or a prop a mapped type builds from keys the check cannot trace, included), a field the prop\'s variants give different types, a standard prop such as `className`, or a prop declared outside the package), and `propsNotChecked` when a mapped type builds props from keys the check cannot trace to the package ({component, doc, reason, props}: no warning is drawn for those props).',
},
],
examples: [
Expand Down
51 changes: 51 additions & 0 deletions packages/cli/assets/docs/tree/check-the-props.doc.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,59 @@ docType: boolean
'`required` disagrees with the code',
'Set `required: true` exactly when every caller must pass the prop. One your `defaultProps` fills counts as optional, as it is to JSX callers, and so does one only some members of a union require, since a caller can leave it out: say in its description which variants need it. An entry that names several props (`value / onChange`) makes the same claim about each: give them separate entries when they differ',
],
[
'`type_mismatch`',
"The documented `type` differs from the code's, where both read exactly",
'Copy the type from the source, or fix the code. The next section says which types are judged',
],
],
},
],
},
{
id: 'write-types-the-check-can-judge',
title: 'Write types the check can judge',
content: [
{
type: 'prose',
text: 'For a prop your package declares, the check also compares the documented `type`, but only where both sides read exactly. It never guesses, so a type it cannot judge is listed, not reported. It needs TypeScript 4.8 or later: with an older one it prints `[skip] Prop types not judged` and judges none.',
},
{
type: 'code',
lang: 'text',
code: `severity: [warn]
component: AcmeCarousel
prop: interval
kind: type_mismatch
message: "interval" is documented as string, but the code declares number.
doc: components/AcmeCarousel.doc.mjs
source: components/AcmeCarousel.tsx
docType: string
codeType: number`,
},
{
type: 'list',
style: 'unordered',
items: [
"Write the type as the source writes it, such as `ReactNode[]` or `(index: number) => void`. Parameter names, member order, quotes and whitespace may differ. Text that writes `readonly` must match exactly, parameter names included; only whitespace may differ. `React.X` names React's own `X` where the prop is declared, so it matches a bare `X` only where that name is React's: `React.KeyboardEvent` is not the DOM's `KeyboardEvent`.",
"Name a type as the file that declares the prop names it, so an import renamed there (`Item as BaseItem`) is written as that file writes it. A field (`cfg.level`) takes its prop's file, even when the field's own type is declared in another file.",
'A type written another way is judged only when both sides are built from exact parts: primitives such as `string` and `number`, string, number and boolean literals, string enum members, arrays and unions of these, and type names that stand for them. Union members may come in any order, and a member another covers adds nothing, as TypeScript passes it there: `undefined` beside `void`, or `string[]` beside `(string | number)[]`.',
'A mutable array type matches a readonly one, because callers can pass it. A readonly one documented for a mutable array is a mismatch.',
"When a prop's variants give a field different types, as `{value: string} | {value: number}` does, document every variant's type (`string | number`). The types of only some variants are listed: the prop's variants give this field different types. With more than six variant types, any other type is listed too.",
"To name an alias and show its values, write `AcmeSize = 'sm' | 'md'` or `AcmeSize ('sm' | 'md')`. Both parts must match.",
"An optional prop's `undefined` is set aside on both sides. With `exactOptionalPropertyTypes`, the doc shows `undefined` exactly when the code declares it.",
'Anything else, such as an object, a tuple, a generic with arguments, `keyof`, an intersection, or a mapped or conditional type written another way, is not compared.',
],
},
{
type: 'prose',
text: "The check prints how many documented types it judged, then lists the rest by reason: not a form the check judges exactly, a field the prop's variants give different types, a standard prop, or a prop declared outside the package. With `--json`, they are in `data.propDrift.typesNotCompared`.",
},
{
type: 'code',
lang: 'text',
code: '[info] Documented prop types judged: 2; not compared: 0. A type is judged when the doc writes it the same as the source (exactly, if it contains `readonly`), or when both are primitives, literals, arrays and unions of those, or names for them.',
},
],
},
{
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/assets/docs/tree/checks.doc.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export const docs = {
'`npx astryx doctor integration components`',
"No component name clashes with a Core component, each `replaces` names one Core component, and each component doc's props match the component's TypeScript props",
'Core is not installed (`core_not_found`). In a package that declares the CLI range that turns replacement on, a `replaces` target is missing (`missing_component_replacement_target`) or invalid (`invalid_component_replacement`), or two components replace one Core component (`ambiguous_component_replacement`)',
'A name clashes with Core, a component sets `replaces` in a package without that range (`inactive_component_replacement`, plus any missing, invalid, or duplicate `replaces` reported as a warning), or a documented prop differs from the code: it is missing there (`not_in_code`), the doc omits a prop the package declares (`not_in_doc`), or its `required` disagrees (`required_mismatch`). It reads props with your installed `typescript`; without it, it prints `[skip] Props not compared`',
'A name clashes with Core, a component sets `replaces` in a package without that range (`inactive_component_replacement`, plus any missing, invalid, or duplicate `replaces` reported as a warning), or a documented prop differs from the code: it is missing there (`not_in_code`), the doc omits a prop the package declares (`not_in_doc`), or its `required` (`required_mismatch`) or type (`type_mismatch`) disagrees. It reads props with your installed `typescript`; without it, it prints `[skip] Props not compared`',
],
[
'`npx astryx doctor integration templates`',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,10 @@ export const doc = {
'with a warning for each such component, naming the range to declare. ' +
"Then it compares each component doc's props with the component's TypeScript " +
"props, using the project's own `typescript`, and warns about a documented prop " +
'the code lacks, a prop the package declares that the doc omits, and a wrong ' +
'`required`. Without TypeScript it prints one ' +
'the code lacks, a prop the package declares that the doc omits, a wrong ' +
'`required`, and a type that does not match where both sides read exactly ' +
'(it lists every other documented type as not compared, with the reason). ' +
'Without TypeScript it prints one ' +
'line saying the comparison was skipped.',
fn: 'integrationComponentConflicts',
args: [
Expand All @@ -35,7 +37,10 @@ export const doc = {
},
],
examples: [
{label: 'Check the local integration', cli: 'astryx doctor integration components'},
{
label: 'Check the local integration',
cli: 'astryx doctor integration components',
},
{
label: 'Check an installed integration',
cli: 'astryx doctor integration components @acme/widgets --json',
Expand Down
Loading
Loading