Repository navigation
feat(cli): doctor integration components judges documented prop types against the code - #7667
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
PR Analysis Report
|
There was a problem hiding this comment.
Approval waits on a maintainer: docs/specs/AST-039/spec.md on main is still a draft without FR12, so a maintainer needs to approve the type-comparison clause of the FR12 proposed in #7658, or name the current record that governs this check. Independently, two matching cases need a fix and a test:
- A doc written as
Alias = …orAlias (…)passes even when the spelled-out values are stale, becausetypeMatchesaccepts either part (prop-drift.mjs,docTypeVariants). - An optional prop documented with an explicit
| undefinedis reported as a mismatch when the code writes the type through an alias, becausedocTypeMatchesdropsundefinedonly from the code's side.
Please also keep the deleted required_mismatch assertion on docRequired and codeRequired in prop-drift.test.mjs.
[Automated review]
| * @param {CodeProp} code | ||
| */ | ||
| function typeMatches(ts, checker, docType, code) { | ||
| return docTypeVariants(docType).some(variant => |
There was a problem hiding this comment.
some() lets the bare alias name satisfy Name = expansion and Name (expansion) on its own, so a stale expansion is never reported. With type LinkVariant = 'link' | 'token' | 'button', a doc of LinkVariant = 'link' | 'token' produces no finding, while 'link' | 'token' alone produces one. When both parts are present, consider requiring the expansion to match, and add a test with a stale expansion.
| types.filter( | ||
| t => | ||
| !(t.flags & ts.TypeFlags.Never) && | ||
| !(ctx.optional && t.flags & ts.TypeFlags.Undefined), |
There was a problem hiding this comment.
This drops undefined from the code's members on an optional prop, but the doc's own undefined member is kept and then finds no partner in matchUnion. A correct doc such as 'neutral' | 'info' | undefined for tone?: Tone is reported as type_mismatch; the text fast path hides this only when the declared text normalizes equal. Dropping a top-level documented undefined on an optional prop before matching would follow the rule in this file's header.
| expect( | ||
| report.findings.find(f => f.kind === 'required_mismatch'), | ||
| ).toMatchObject({ | ||
| docRequired: false, |
There was a problem hiding this comment.
This was the only assertion on docRequired and codeRequired; with it gone, dropping either field from the required_mismatch finding would pass. Consider keeping it beside the new type_mismatch assertion.
62fdc47 to
d04b441
Compare
2021473 to
59ea262
Compare
There was a problem hiding this comment.
Approval waits on a maintainer: docs/specs/AST-039/spec.md on main is still a draft without FR12, so a maintainer needs to approve the type-comparison clause of the FR12 proposed in #7658, or name the current record that governs this check. The earlier stale-expansion, | undefined and test-assertion issues are fixed, but the matcher still reports some correct docs, and each case below needs a fix and a test:
- A callback with an optional parameter is reported whenever its doc is spelled differently from the source, because the function-type branch in
type-match.mjsreads an optional parameter's type as includingundefined. This causes four of the five type warnings on Lab (onReactandonSubmitonCommentsandCommentThread). - A doc that spells out an intersection alias (
A & B, orAB = A & B) is reported, which breaks the published rule that alias spelling does not count. - For an optional method prop,
codeTypereadsundefined.
[Automated review]
| at(ctx, params[i]), | ||
| param.type, | ||
| constituents( | ||
| checker.getTypeOfSymbolAtLocation(params[i], ctx.location), |
There was a problem hiding this comment.
For an optional parameter, this returns T | undefined, but a doc's x?: T contributes only T, so matchUnion leaves undefined unmatched and a correct doc is reported. For example, onPick?: Handler with type Handler = (value: string, event?: Event) => void, documented as (value: string, event?: Event) => void, gives type_mismatch. The same thing produces the four warnings on Lab's Comments and CommentThread (onReact, onSubmit). Consider dropping undefined from an optional parameter's constituents, as the type-literal branch does for optional properties, and adding a test with an aliased callback that has an optional parameter.
|
|
||
| // Indexed access, conditional, mapped, template literal and the like. | ||
| const docText = normalizeTypeText(node.getText()).join('|'); | ||
| return docText === normalizeTypeText(printed(ctx, code)).join('|'); |
There was a problem hiding this comment.
printed() prints an aliased type by its alias name, so a doc that spells out an intersection alias never matches here. value?: AB with type AB = A & B, documented as A & B or as AB = A & B, is reported as type_mismatch ("…but the code declares AB (A & B)"). Also comparing against the alias-expanded print (TypeFormatFlags.InTypeAlias), or matching intersection members structurally, would keep the published rule that alias spelling does not count.
| typeof entry.type === 'string' && | ||
| !typeMatches(ts, checker, entry.type, code) | ||
| ) { | ||
| const codeType = (code.declared[0] ?? code.renderings()[0][0] ?? '') |
There was a problem hiding this comment.
For a prop declared as an optional method (onPick?(value: string): void), declared is empty and the first printed member is undefined. As a result, codeType is "undefined", and the message reads "the code declares undefined ((value: string) => void)". Skipping undefined on an optional prop when picking this fallback, as the expanded list below already does, would fix it.
d04b441 to
8b14433
Compare
59ea262 to
87d060f
Compare
There was a problem hiding this comment.
Approval waits on a maintainer: docs/specs/AST-039/spec.md on main is still a draft without FR12, so a maintainer needs to approve the type-comparison clause of the FR12 proposed in #7658, or name the current record that governs this check. The earlier optional-parameter, intersection-alias and codeType issues are fixed, but each case below still needs a fix and a test:
[ok] Documented props match the codeprints for a component whose prop type the same output lists under "Props not compared", because an unresolved prop type now adds anuncheckedentry while the component still counts as compared.- A correct doc that writes a generic alias with type arguments (
MaybePromise<void>,Nullable<string>,Partial<Parts>) is reported once its text differs from the source, becausetype-match.mjsresolves only names written without type arguments. keyof (A & B)documented exactly as the source writes it is read as theAlias (…)notation and reported, and an intersection documented for a flat object is reported even though object splitting shouldn't count.
[Automated review]
| blocks.push( | ||
| text( | ||
| `[ok] Documented prop names and required flags match the code in ${report.components} component(s) (TypeScript ${report.typescript}).`, | ||
| `[ok] Documented props match the code in ${report.components} component(s) (TypeScript ${report.typescript}).`, |
There was a problem hiding this comment.
This [ok] now also prints when unchecked lists one of the compared components: a prop whose type doesn't resolve adds a per-prop unchecked entry in prop-drift.mjs, but its component still counts in components. A fixture with icon?: MissingIcon from an uninstalled package prints [info] Props not compared for 1 documented component(s): and then [ok] Documented props match the code in 1 component(s). Consider printing the [ok] only when no compared component has an unchecked entry (or saying how many prop types weren't compared), and adding a CLI test.
| if (args.length === 0) { | ||
| const named = resolveName(ctx, name); | ||
| if (named && named.length === 1) { | ||
| if (named[0] === code) return true; | ||
| // An alias of a structure: compare the structure. | ||
| return sameStructure(ctx, named[0], code, depth + 1); | ||
| } | ||
| } | ||
| return false; |
There was a problem hiding this comment.
Only a name without type arguments is resolved to its structure here, and the union shortcut in matchUnion has the same condition, so a correct doc that writes a generic alias with arguments is reported once the text fast path misses. For example, onSubmit?: SubmitHandler, with type SubmitHandler = (content: string, files?: File[]) => MaybePromise<void> and type MaybePromise<T> = T | Promise<T>, documented as (content: string, files?: File[]) => MaybePromise<void>, gives type_mismatch; so do Nullable<string> for string | null and Partial<Parts> for an all-optional Parts. Consider matching such a name against the code type before it's split into members (its aliasSymbol and aliasTypeArguments), and adding tests.
| const named = | ||
| /^\s*([A-Za-z_$][\w$.]*)\s*=(?!>)\s*([\s\S]+)$/.exec(docType) ?? | ||
| /^\s*([A-Za-z_$][\w$.]*)\s*\(([\s\S]+)\)\s*$/.exec(docType); | ||
| return named ? [named[1], named[2]] : [docType]; |
There was a problem hiding this comment.
keyof (A & B) matches the second pattern, so it's split into keyof and A & B, and the bare keyof can never match: a doc written exactly as the source declares it is reported. Trying the whole text first, and splitting only when it doesn't match, would keep the stale-expansion rule.
| if (ts.isIntersectionTypeNode(node)) { | ||
| const remaining = | ||
| code.flags & TypeFlags.Intersection ? [...code.types] : [code]; | ||
| for (const part of node.types) { | ||
| const index = remaining.findIndex(each => | ||
| matchOne(ctx, part, each, depth + 1), | ||
| ); | ||
| if (index === -1) return false; | ||
| remaining.splice(index, 1); | ||
| } | ||
| return remaining.length === 0; | ||
| } |
There was a problem hiding this comment.
When the code's type is a single object ({a: string; b: number}, or an alias of one), a doc that writes it as {a: string} & {b: number} is reported, because each documented part has to match a whole member of the code's intersection. The PR description says how an object is split doesn't count; comparing the documented parts' combined members with the code's props would cover this direction too.
8b14433 to
5ddd619
Compare
87d060f to
ddc6822
Compare
There was a problem hiding this comment.
Approval waits on a maintainer: docs/specs/AST-039/spec.md on main is still a draft without FR12, so a maintainer needs to approve the type-comparison clause of the FR12 proposed in #7658, or name the current record that governs this check. The issues raised on the previous revision are fixed, but each case below still needs a fix and a test:
- A correct doc that writes an array of a union, such as
(IconName | string)[]for a type that isstring[], or(number | null)[]in a project without strict null checks, is reported, because literal absorption and thenullrule intype-match.mjsapply only at the top level. - With TypeScript 6, a tsconfig that sets neither
strictnorstrictNullCheckshas strict null checks, butprop-drift.mjsreads it as non-strict, so a doc that leaves out| nullpasses;ts.getStrictOptionValuegives the compiler's own default. - A named type followed by its default (
Size = 'md') is reported as a stale alias expansion, and a documented field under ananyorunknownprop is neither judged nor listed as not compared.
[Automated review]
| } | ||
| const array = (/** @type {any} */ element) => { | ||
| const parts = docAtoms(ctx, element, depth + 1); | ||
| return parts ? [`[${[...new Set(parts)].sort().join('|')}]`] : null; |
There was a problem hiding this comment.
An array's element is built from its raw parts here, while parts() absorbs literals into their primitive, and drops null without strict null checks, only at the top level. The compiler has already reduced the code side's element (Array<IconName | string> is string[]), so a correct doc of (IconName | string)[] for type IconList = Array<IconName | string> is reported ("…but the code declares IconList (string[])"), and so is (number | null)[] for Array<number | null> with strict: false. Reducing the element atoms the same way on both sides before building […] (absorption always; null and undefined only without strict null checks), here and in codeAtoms, would fix it.
There was a problem hiding this comment.
Fixed at 1090014: literal absorption and the null rule apply at every depth. foundation/discovery/prop-drift.test.mjs › "reads a union the way TypeScript reduces it, and a default after = as no alias" pins (IconName | string)[] for string[], and "sets aside null and undefined when the project has no strict null checks" pins (number | null)[] for MaybeNum[].
| const nullable = !( | ||
| compilerOptions.strictNullChecks ?? | ||
| compilerOptions.strict ?? | ||
| false | ||
| ); |
There was a problem hiding this comment.
TypeScript 6 turns strict null checks on when neither strict nor strictNullChecks is set (ts.getStrictOptionValue({}, 'strictNullChecks') is true in 6.0.3), so for such a tsconfig this reads the project as non-strict while the compiler keeps null. A doc of number for count?: number | null, or string | null for label?: string, then passes, though strict: true reports both. Using ts.getStrictOptionValue?.(compilerOptions, 'strictNullChecks'), with this expression as the fallback, would follow the installed compiler.
There was a problem hiding this comment.
Fixed at 1090014: strict null checks come from ts.getStrictOptionValue. foundation/discovery/prop-drift.test.mjs › "reads strict null checks as the TypeScript version defaults them" pins a tsconfig with neither option.
| : (/^\s*([A-Za-z_$][\w$.]*)\s*=(?!>)\s*([\s\S]+)$/.exec(docType) ?? | ||
| /^\s*([A-Za-z_$][\w$.]*)\s*\(([\s\S]+)\)\s*$/.exec(docType)); | ||
| // `boolean = false` gives a default, not an alias's expansion. | ||
| const named = notation && !KEYWORD_TYPES.has(notation[1]) ? notation : null; |
There was a problem hiding this comment.
Only a keyword before = is read as a default. Size = 'md', the same convention with a named type, is read as an alias whose expansion is 'md' and reported ("…but the code declares Size ('sm' | 'md' | 'lg')"). When the right side is a single member of the named type, the notation can't be judged exactly, so listing it as not compared would match the rest of this design.
There was a problem hiding this comment.
Fixed at 1090014: one value after the name, a literal or a bare name (Size = 'md', Size = Size.Md), reads as a default and only the name is judged; a union or another type form after it is still judged as an expansion. foundation/discovery/prop-drift.test.mjs › "reads a union the way TypeScript reduces it, and a default after = as no alias" pins Size = 'md', and "lists a name two index signatures take, and reads a field or a default for one union member" pins SizeE = SizeE.Md.
| const reached = code | ||
| ? fieldTypes(ts, checker, code, entry.path) | ||
| : null; | ||
| if (reached === 'any') continue; |
There was a problem hiding this comment.
A documented field under a prop typed any or unknown (meta.kind for meta?: any) is skipped here without being judged or added to notCompared, so the [info] count leaves it out, while a top-level any prop is listed. Adding entry.written to notCompared here, for an own prop with a string type, would keep the published rule that every other type is listed.
There was a problem hiding this comment.
Fixed at 1090014: a field under an any or unknown prop is listed as not compared. foundation/discovery/prop-drift.test.mjs › "lists every other form of type as not compared, never as a mismatch" pins data.id and extra.kind.
e881162 to
e7ce3a1
Compare
There was a problem hiding this comment.
This is ready to approve: the type comparison follows architecture:cli-surface INV33, warning only where both sides can be judged exactly and listing every other documented type as not compared, and the generic-instantiation gap from the previous review is fixed and covered by the new instantiation tests. Nothing else needs to change in this PR. It should land after the four pull requests it is stacked on and no later than the release of integration check, so the patch bump stays valid.
[Automated review]
894c2ab to
53a42e3
Compare
e9a8db8 to
509579d
Compare
There was a problem hiding this comment.
Changes requested: the type comparison follows architecture:cli-surface INV33, and its tests and the Core run check out, but one reason label is wrong. A documented prop that the package's own mapped type builds from keys the check can't trace (for example {[K in `on${'Open' | 'Close'}`]?: () => void}) is listed in typesNotCompared as “declared outside the package, not compared”, while the same props left out of the doc are reported in propsNotChecked as keys the check can't trace to the package's source. Please give those props their own reason (or “not a form the check judges exactly”) at the three outside() call sites, name it in the typesNotCompared docs, and add a test; the FunctionDoc's text-match sentence could also carry the “(exactly, if it contains readonly)” qualifier the [info] line already has.
[Automated review]
| const outside = name => | ||
| PROPS_NOT_DOCUMENTED.includes(name) | ||
| ? 'standard prop, not compared' | ||
| : 'declared outside the package, not compared'; |
There was a problem hiding this comment.
A prop with code.untraced (one the package's own mapped type builds from keys the check can't trace) also lands here, so its documented type is listed as declared outside the package, not compared although the check doesn't know where it is declared. With type Handlers = {[K in `on${'Open' | 'Close'}`]?: () => void} in the component's own file, documented onOpen and onClose get this reason, while the same props left out of the doc go to propsNotChecked as keys the check can't trace to the package's source. A separate reason for untraced props (or NOT_EXACT) at the three call sites, named in the typesNotCompared docs and pinned by a test, would keep the two in step.
There was a problem hiding this comment.
Fixed in 0907e58. The three call sites now give not a form the check judges exactly for a prop the package's own mapped type builds from keys the check can't trace, so onOpen from {[K in `on${'Open' | 'Close'}`]?: () => void} is listed under that reason, in step with propsNotChecked. A documented type that does not parse is listed there too now, never matched, and so is prose in parentheses after a type (number (or a CSS length)), which was judged on number alone. The typesNotCompared docs name both cases, and the mapped-type test covers them; it failed before. With a project TypeScript older than 4.8, no type is judged: the text prints [skip] Prop types not judged with the reason, and JSON adds typesSkipped, which is documented and tested with a stubbed 4.7.4.
| '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, ' + |
There was a problem hiding this comment.
For text that writes readonly, quotes and union order are not set aside: judgeOne compares it as written, whitespace aside (type-match.mjs lines 147–152), so (v: readonly ("a" | "b")[]) => void documented for (v: readonly ('a' | 'b')[]) => void is listed, not judged. Adding the [info] line's “(exactly, if it contains readonly)” here would keep the API reference accurate.
There was a problem hiding this comment.
Done in 0907e58. The API reference now says the text match is "exact, if it contains readonly", as the [info] line does.
53a42e3 to
2900034
Compare
509579d to
0907e58
Compare
There was a problem hiding this comment.
This is ready to approve: the type comparison follows architecture:cli-surface INV33, judging a documented type only where both sides read exactly and listing every other one, and the reason-label and API-reference points from the previous review are fixed. Nothing else needs to change in this PR; it should land after the four pull requests it is stacked on and no later than the release of integration check, whose rebase should treat typesSkipped as a comparison that did not run under --strict. Optionally, the Changeset could also mention typesSkipped and the TypeScript 4.8 floor.
[Automated review]
2900034 to
592bc25
Compare
0907e58 to
c700d87
Compare
There was a problem hiding this comment.
Changes requested: the type comparison follows architecture:cli-surface INV33 and its tests pass, but a documented name that only another package's index signature takes is still judged and can warn. For example, with props that extend Core's BaseProps, a doc that lists data-variant as 'solid' | 'outline' gets type_mismatch against Core's string | undefined, while xstyle from the same type is listed as not compared, and the props guide says only props the package declares are compared. Please list such names as declared outside the package (or document that they are judged), with a test.
[Automated review]
| skip(NOT_EXACT, propName); | ||
| } | ||
| if (index && typeof entry.type === 'string') { | ||
| const verdict = judgeType(entry.type, [index.type], { |
There was a problem hiding this comment.
This judges a name that only another package's index signature takes, with no ownership test. With props BaseProps & {label?: string} where BaseProps comes from Core and carries [key: `data-${string}`]: string | undefined, a doc that lists data-variant as 'solid' | 'outline' draws type_mismatch against string | undefined, while xstyle from the same BaseProps is listed as standard prop, not compared; the props guide says only props the package declares are compared. Deciding ownership from the signature's declaration file, as named props do through code.own, and listing the others with outside(propName) would keep the two in step. A test with an inherited data-* signature would pin it.
There was a problem hiding this comment.
Fixed in 01c59c2. A name only index signatures take is judged only when exactly one signature takes it and the package's own source declares that signature. One that only another package's declared signature takes is listed with outside(propName), as declared outside the package, like xstyle from the same BaseProps. One a mapped type's signature takes (Record, or a signature passed through Omit) has no declaration to say whose it is, so it is listed as not a form the check judges exactly. The className test adds data-variant against a dependency's inherited [key: `data-${string}`] and an own Record<aria-${string}, string>. It failed before. On Core this lists one name that was judged before: CollapsibleGroup's data-testid, which reaches Core's own signature through Omit.
| const reached = code | ||
| ? fieldTypes(ts, checker, code, entry.path) | ||
| : null; | ||
| if (code && !code.own && typeof entry.type === 'string') { |
There was a problem hiding this comment.
Optional: this lists a field of another package's prop before checking that the field exists, so a documented tone.missing is reported as not_in_code and also counted under declared outside the package, not compared, while a missing field of the package's own prop is only reported. Listing it after the reached check would keep not compared to fields that exist.
There was a problem hiding this comment.
Done in 01c59c2. A field of another package's prop is listed only once the field is found, so tone.missing is reported as not_in_code and not counted as not compared. The className test adds it.
592bc25 to
4f18fe9
Compare
c700d87 to
01c59c2
Compare
There was a problem hiding this comment.
Changes requested: the type comparison follows architecture:cli-surface INV33 and the index-signature issue from the previous review is fixed, but the matcher treats two different string enums with the same values as the same type. With enum Color {Red = 'red', Blue = 'blue'} and enum Hue {Red = 'red', Blue = 'blue'}, a doc that types a Color prop as Hue (or a Color[] prop as Hue[]) counts as a judged match, although tsc rejects Hue.Red there (TS2322). Please key a string enum member by its enum as well as its value in codeAtoms (type-match.mjs), and add a test with two such enums.
[Automated review]
| if (t.flags & F.EnumLiteral) { | ||
| // A string enum member is exact; a numeric enum also takes numbers. | ||
| if (!(t.flags & F.StringLiteral)) return null; | ||
| out.push(`enum:${t.value}`); |
There was a problem hiding this comment.
This keys a string enum member by its value alone, so two different enums with the same values reduce to the same atoms. With enum Color {Red = 'red', Blue = 'blue'} and enum Hue {Red = 'red', Blue = 'blue'}, a doc of Hue for color?: Color (or Hue[] for Color[]) counts as a judged match, while tsc rejects Hue.Red there (TS2322). Keying the atom by the member as well, for example enum:${checker.getFullyQualifiedName(t.symbol)}, keeps C.Color, a re-exported alias and Color.Red | Color.Blue matching, since they resolve to the same members, and leaves the string absorption in reduced working, since the atom still starts with enum:. A test with two such enums, direct and in an array, would pin it.
There was a problem hiding this comment.
Fixed in 57f468e. A string enum member's part now names its enum as well as its value, so Hue for color?: Color and Hue[] for Color[] are mismatches, as tsc rejects Hue.Red there (TS2322). C.Color, a re-exported or renamed import, and Color.Red | Color.Blue still match, since they resolve to the same enum. A string-literal union of the same values is decided by what tsc accepts. 'red' | 'blue' for a Color prop is a mismatch (tsc rejects 'red' there, TS2345), and so is Color for a 'red' | 'blue' prop, since Color does not take the 'red' the prop does. One case is listed instead of judged: two different enums of one name, such as Theme.Color and a top-level Color. tsc relates those by their members, by rules its versions differ on (member names on 4.8 and 5.0, names and values on 6.0). The primitives test adds the two enums, direct and in an array, and both literal-union directions, and the not-compared test adds the same-named pair. They failed before.
4f18fe9 to
bba990c
Compare
01c59c2 to
57f468e
Compare
There was a problem hiding this comment.
This is ready to approve: the type comparison follows architecture:cli-surface INV33, judging a documented type only where both sides read exactly and listing every other one, and the string-enum issue from the previous review is fixed and covered by tests. Nothing else needs to change in this PR; it should land after the four pull requests it is stacked on and no later than the release of integration check, whose rebase should treat typesSkipped as a comparison that did not run under --strict.
[Automated review]
There was a problem hiding this comment.
This is ready to approve: the type comparison follows architecture:cli-surface INV33, judging a documented type only where both sides read exactly and listing every other one, and the INV33 test suites and the Core comparison pass. Nothing needs to change before it lands after the four pull requests it is stacked on, no later than the release of integration check, whose rebase should treat typesSkipped as a comparison that did not run under --strict. One optional tidy-up: an a / b entry whose one type does not split still lists a missing field (such as status.b) as not compared beside its not_in_code warning; listing a field only once it is found would match the rule the code states for other fields.
[Automated review]
| if ( | ||
| entry.type === undefined && | ||
| !typesSkipped && | ||
| typeof raw.type === 'string' && | ||
| (code || read.accepts(propName).length > 0) | ||
| ) { | ||
| // `a / b` with one type that does not split across the names. | ||
| skip( | ||
| code && !code.own ? outside(propName, code) : NOT_EXACT, | ||
| entry.written, | ||
| ); | ||
| } |
There was a problem hiding this comment.
Optional: this lists both names of an a / b entry whose one type does not split before the field is looked up. With status?: {a: number}, a doc entry status.a / status.b typed number reports status.b as not_in_code and also counts it under not a form the check judges exactly, while line 1682 says a field is listed only once it is found. Listing a field-path name here only when fieldTypes reaches it would keep not compared to fields that exist.
There was a problem hiding this comment.
Done in 9e2b7c0. A field-path name of an a / b entry whose type doesn't split is now listed only once fieldTypes reaches it, so status.b is reported as not_in_code and nothing else. The index-signature test adds the pair; it failed before. The not-compared list had the same NUL-joined sort key the findings had in #7663, so it now sorts by component, then reason, pinned by the mapped-type test.
There was a problem hiding this comment.
This is ready to approve: the type comparison follows architecture:cli-surface INV33, judging a documented type only where both sides read exactly and listing every other one, and its test suites, the Core comparison and checks on TypeScript 4.8, 5.0 and 6.0 pass on this revision. The a / b counting issue from the previous review is fixed. Nothing else needs to change; it lands after the four pull requests it is stacked on and no later than the release of integration check.
[Automated review]
There was a problem hiding this comment.
Changes requested: the type comparison follows architecture:cli-surface INV33 and its test suites pass, but it applies two match rules that the props guide and the integrationComponentConflicts reference do not state, which INV33 (“the loose comparison the command documents”) and AST-017 FR14 require. A mutable array documented where the code takes a readonly one (string[] for readonly string[]), and one member's type documented for a field of a union-typed prop (sel.value: string for {value: string} | {value: number}), are judged matches, while the guide calls any difference a type_mismatch. Please add a clause for each rule to the guide's “Write types the check can judge” list and to the API reference, or report the readonly case; nothing else needs to change.
[Automated review]
| 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.', | ||
| '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.', |
There was a problem hiding this comment.
This list doesn't say that a mutable array documented where the code takes a readonly one is a match: string[] for ids?: readonly string[], or string[][] for ReadonlyArray<ReadonlyArray<string>>, is judged with no warning (type-match.mjs line 184), while line 96 calls any difference a type_mismatch and line 128 says text that writes readonly must match exactly. INV33 holds the warning to the comparison the command documents, and AST-017 FR14 requires automatic behavior to be documented, so a clause here would close it, for example: “A mutable array documented where the code takes a readonly one matches, as TypeScript passes it; a readonly array documented for a mutable one does not.” Reporting the case instead would also work.
There was a problem hiding this comment.
Fixed in 691a46c: the guide's "Write types the check can judge" list and the API reference now say that a mutable array type matches a readonly one, because callers can pass it, and that a readonly one documented for a mutable array is a mismatch.
| '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. `Name = …` with a union after it ' + |
There was a problem hiding this comment.
The one-member rule is stated here only for a union of props types, but a field of a union-typed prop uses it too: with sel?: {kind: 'a'; value: string} | {kind: 'b'; value: number}, sel.value documented as string is a judged match (prop-drift.mjs line 1707 hands the field's per-member types to judgeMembers). Saying “for a prop or field whose type differs between the members of a union, any one member's type matches”, here and in the props guide, beside the readonly-array rule, would document what the comparison does.
There was a problem hiding this comment.
Changed rather than documented, in 691a46c: a field the prop's variants give different types takes every variant's type. So sel.value: string for {value: string} | {value: number} is now listed as not compared, with the reason "the prop's variants give this field different types", as are the types of any other proper subset of the variants, and string | number is judged. The guide and the API reference say so, and the index-signature test pins both; it failed before. A prop that differs between the members of a union of props types keeps the one-member rule, as the reference says.
There was a problem hiding this comment.
This is ready to approve: the type comparison follows architecture:cli-surface INV33, judging a documented type only where both sides read exactly and listing every other one, and the two match rules the previous review found undocumented are now documented, one of them now listing the type instead of matching it. It should land after the four pull requests it is stacked on and no later than integration check. One optional tidy-up: the typesNotCompared reason list in the integrationComponentConflicts reference and in the PropDriftTypesNotCompared JSDoc still names three reasons, so adding “a field the prop's variants give different types” would match what the check now reports.
[Automated review]
| 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 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).', |
There was a problem hiding this comment.
Optional: this list of typesNotCompared reasons names three, but the check can now also report the prop's variants give this field different types (a field documented with only some of its variants' types), which the props guide already lists. Adding it here, and to the PropDriftTypesNotCompared.reason JSDoc in foundation/discovery/prop-drift.mjs, would keep the reference's value list complete.
There was a problem hiding this comment.
Done in 5864b9a: the reference's list of typesNotCompared reasons and the PropDriftTypesNotCompared.reason JSDoc now name "the prop's variants give this field different types" too.
There was a problem hiding this comment.
Changes requested: the type comparison follows architecture:cli-surface INV33 and its tests pass, but the judge it wires in reads React.X as whatever bare X names where the prop is declared, a rule the props guide and the API reference don't state. As a result, a handler the source types with the DOM's KeyboardEvent and the doc types (e: React.KeyboardEvent) => void counts as a judged match although tsc rejects it (TS2322), and a correct React.Key doc draws type_mismatch when the component file declares its own Key. Please resolve React.X through React where the prop is declared (in #7684's judge), and add a test here for both cases.
[Automated review]
| 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.', |
There was a problem hiding this comment.
The judge also sets a React. qualifier aside, which this list doesn't say, and reads React.X as whatever X names where the prop is declared (type-text.mjs line 194, type-match.mjs lines 451–453). So onKey?: (e: KeyboardEvent) => void, the DOM's event with no React import, documented as (e: React.KeyboardEvent) => void counts as a judged match although tsc rejects that handler, and a correct React.Key doc draws type_mismatch when the file declares its own Key. Resolving React.X through React in #7684's judge, with a test here for both cases, would fix it.
There was a problem hiding this comment.
Fixed in 60fa1df's judge, with the test here at f1607d1. React.X now resolves through React where the prop is declared (an imported namespace or React's global one), and the text match is refused when one side writes React.X and the other a bare X that names another type there. So the DOM's KeyboardEvent handler documented as (e: React.KeyboardEvent) => void is listed as not compared, and React.Key matches string | number | bigint where the file declares its own Key. Both sides writing React.X, or a bare X imported from React, still match. The test covers both cases, with React imported and global; it failed before. The props guide and the API reference now state the rule. On Core, three handlers whose docs write the DOM's MouseEvent for React.MouseEvent move from judged to not compared; no finding changes.
There was a problem hiding this comment.
Changes requested: the type comparison follows architecture:cli-surface INV33 and React. names now resolve correctly, but documented fields, index-signature names and props declared in another file are still text-matched against the code's type printed outside the scope where the doc's names resolve. As a result, when a component file imports Item as BaseItem or Size as BaseSize and declares its own Item or Size, wrong docs such as cfg.item: Item or size-a: Size count as judged matches with no warning, although tsc rejects them (TS2741, TS2322). Please print those types where the prop is declared (in fieldTypeText, for the index type, and for the renderings) and add a test for an imported type that the file names its own way; with that change the existing tests and Core's results stay the same.
[Automated review]
| .map(t => | ||
| checker.typeToString( | ||
| t, | ||
| undefined, |
There was a problem hiding this comment.
This prints the field's type with no enclosing declaration, so it reads as the type's own name: a field typed BaseItem (from import type {Item as BaseItem}) prints as Item. The doc's names resolve at code.location, so where the file declares its own Item, a doc of cfg.item: Item text-matches and counts as a judged match with no warning, although tsc rejects it (TS2741). Printing at code.location (passed from the call at line 1802) lists it instead, and the existing tests still pass.
There was a problem hiding this comment.
Fixed in a99b029: a field's type is now printed at code.location, where the doc's names resolve, so a field typed BaseItem prints as BaseItem there and cfg.item: Item (the file's own Item, an object type) is listed as not compared. The new test pins it; it failed before.
| // An index signature takes `undefined` only when its type does. | ||
| optional: takesUndefined(ts, index.type), | ||
| ...reading, | ||
| written: [[checker.typeToString(index.type)]], |
There was a problem hiding this comment.
The index type is printed with no location too. With import type {Size as BaseSize} and a local type Size = 'xs' | 'xl', size-a: Size against [key: `size-${string}`]: BaseSize text-matches Size and draws no warning, although Size resolves here to 'xs' | 'xl' and tsc rejects it (TS2322). Printing at index.declaration, here and for the message's codeType below, reports it as type_mismatch.
There was a problem hiding this comment.
Fixed in a99b029: the index type is printed at the index signature's declaration, so size-a: Size against BaseSize draws type_mismatch where Size is the file's own literal union, as tsc rejects it. The new test pins it; it failed before.
| printed.push(checker.typeToString(part, sourceFile, flags)); | ||
| expanded.push( | ||
| checker.typeToString( | ||
| part, | ||
| sourceFile, | ||
| flags | ts.TypeFormatFlags.InTypeAlias, |
There was a problem hiding this comment.
These renderings are printed in the component's file, while the doc's names resolve at the prop's declaration (location, line 552). For a prop declared in another file of the package that imports Item as BaseItem and declares its own Item, item: Item and onPick: (item: Item) => void match the printed Item and count as judged, although tsc rejects both. Printing at ownDeclarations[0] ?? sourceFile keeps the two in one scope, and Core's results do not change.
There was a problem hiding this comment.
Fixed in a99b029: the renderings are printed at the prop's declaration (ownDeclarations[0], else the component's file), so in another file that imports Item as BaseItem and declares its own Item, item: Item, onPick: (item: Item) => void and a mapped entry: Item are listed as not compared, while docs that name the type as that file does still match. The test covers each; the existing tests and Core's results are unchanged.
There was a problem hiding this comment.
This is ready to approve: astryx doctor integration components now judges documented prop types as architecture:cli-surface INV33 requires, warning only where both sides read exactly and listing every other type as not compared, and a type that a file imports under another name (such as Item as BaseItem) is now judged where the prop is declared, with a test that covers it. Nothing else needs to change in this pull request; merge it after #7681, #7663, #7676 and #7684, and no later than the release that ships integration check.
[Automated review]
… against the code
`astryx doctor integration components` now also judges each documented
prop's type against the code, for props the package declares, a documented
field (`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 as the
source once whitespace, quotes and union order are set aside (the text of
the prop's own declaration, of the generic type's prop it instantiates
when the result prints in the component's file as the declared type, as
when a generic component passes its own type parameters on, or of one a
mapping copies unchanged,
such as `Pick`; a prop a mapping rewrites, such as `{[K in keyof T]:
T[K][]}` or `Required<T>`, or another type instantiates, such as
`Box<string>` or `ListProps<T[]>`, is judged by its type), or
primitives, literals, string enum members (each only its own enum's, as
TypeScript takes no other enum's member of the same value; two enums of
one name, which TypeScript relates by their members, are listed), arrays
and unions of those, and names for them, on both sides. `Name = …` and `Name (…)` must match in both parts, and a doc
may describe one member of a union whose members give a prop different
types; a field the prop's variants give different types is listed when
the doc gives only some variants' types (`string` for
`{value: string} | {value: number}`), as those are neither the field's type
nor wrong, and judged when it gives them all. Every other type (a callback, an object, a generic with arguments,
an intersection, `keyof`, a type that does not resolve, a doc type that
does not parse, prose in parentheses after a type, a prop a mapped type
builds from keys the check can't trace) is listed as not compared, never
reported. A prop declared outside
the package is not judged either, and is listed with its reason: a
standard prop (`className`, `style`, …) or another package's prop (a
field of one once the field is found), a name only another package's
index signature takes included; a `className`
the package declares itself is judged like any other prop. A name a
mapped type's index (`Record`) takes is listed, as nothing says whose it
is.
`React.X` in a doc resolves through `React` where the prop is declared,
so it is never read as a bare name that means another type there.
The code's type is printed where the prop (or its index signature) is
declared, where the doc's names resolve, so a type a file imports under
another name is never read as the file's own type of the same name.
With a project TypeScript older than 4.8 no type is judged: the text
prints `[skip] Prop types not judged` with the reason, and JSON adds
`typesSkipped`. A field of a name only an index
signature takes, and an `a / b` entry whose one type does not split
across the names (a field once it is found), are listed too, by component
and then reason. With `exactOptionalPropertyTypes`, an
optional prop takes `undefined` only when its type says so, and is judged
that way. A name an index signature takes, and a field a
`Record` takes, take `undefined` only when the index's type does. A
readonly array documented for a mutable array is a mismatch, while a
mutable array documented for a readonly one matches, as TypeScript
passes it. Text that writes `readonly` matches only as written, the
same text with `readonly` kept on both sides; otherwise it is judged by
its parts, so `readonly` that sits elsewhere (another depth, a callback's
parameter, an object's field) is listed as not compared. JSON adds `typesCompared` and
`typesNotCompared` ({component, doc, reason, props}) to `propDrift`; the
text says how many types were judged and lists the rest with the reason.
The type judge itself (type-match.mjs) comes from the PR below.
The props-table guide adds `type_mismatch` to its findings, and a section on writing a `type` the check can judge: as the source writes it (exactly, if it writes `readonly`), or built from exact parts; a mutable array type for a readonly one, which matches because callers can pass it; every variant's type for a field the prop's variants type differently; `React.X` as React's own `X`; a type named as the file that declares the prop names it; a union member another covers; an alias with its values; `undefined` on an optional prop; and what is listed as not compared, by reason.
User need
A component doc's prop table says what type each prop takes, and people and agents write code from it. When a union gains or loses a value and the doc doesn't change with it, the doc is wrong, and prop names and
requiredflags can't show it.Problem and solution fit
This PR is stacked on #7684, which reads two written types as one and holds the judge this PR wires in (
type-match.mjs), on #7676, which runs the prop comparison inastryx doctor integration components. It adds the type comparison.For a prop the package declares, a documented field such as
status.type, and a name an index signature takes, the command now judges the documented type against the code's and warns (type_mismatch) when they differ. It judges only where both sides can be judged exactly:undefinedare set aside; orstring,number,boolean,null, …), string, number and boolean literals, string enum members, arrays of these and unions of these, or a name that resolves to one of these. The two sides are then compared as TypeScript reduces each union, at any depth: a literal beside its primitive is absorbed by it (IconName | stringisstring),undefinedis set aside on an optional prop (which takes it anyway), andnullandundefinedare set aside when the project has no strict null checks, where the compiler drops them.Every other type is listed as not compared, never reported: a callback, an object, a tuple, a generic with arguments,
keyof, an intersection, a mapped or conditional type,anyorunknown, a numeric enum, and a type that doesn't resolve. A documented prop that the package doesn't declare is listed too, with the reason: a standard prop (className,style,xstyle,data-testid,ref,key), or a prop declared by another package. AclassNamethe package declares itself is judged like any other prop. A warning check that flags correct docs teaches people to ignore it, so this one judges only what it can judge exactly, and counts everything else.Here an Acme package's
tonegained a value its doc doesn't list, andmetais an object type:The same doc writes
sizeas'md' | 'sm'for the code's'sm' | 'md', andonPressas() => voidexactly as the source does; neither is a finding.On Core it finds the 4 docs whose union leaves out a value the code takes: FieldStatus
variant, PowerSearchstatusVariant, Textas, and Iconcolor. They joincore-prop-docs.known-drift.json.Current specification
Implements the type comparison of
architecture:cli-surfaceINV33, "Documented component props agree with the source" (AST-039 FR12, which now points to it; DEC-5), including its requirement that a documented type the check cannot judge is listed as unchecked rather than reported as a match or a mismatch:typesNotComparedis that list. #7663 and #7676 implement the rest of INV33.Public delta and caller burden
type_mismatch, indata.propDrift.findings, carryingdocType(as the doc writes it) andcodeType(as the source writes it). It is a warning, so exit codes are unchanged.data.propDriftgainstypesCompared(how many documented types were judged) andtypesNotCompared({component, doc, reason, props}). The second holds every other documented type, grouped by why it wasn't judged: 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.Design and alternatives
Tonein a doc resolves to the code's'neutral' | 'info'.Name = …andName (…)judge both parts, so stale spelled-out values are reported (Items = string[]fornumber[]too). With one value after it, a literal or a bare name (Size = 'md',Size = Size.Md), or after a type keyword (boolean = false), that part may be the prop's default, so only the name is judged. Text that is a type by itself, such askeyof (A & B), is never read as that notation.sel.valuein{value: string} | {value: number}) takes every variant's type. A doc that gives their union is judged; one that gives the types of only some variants is listed ("the prop's variants give this field different types"), as those are neither the field's type nor wrong; one that matches no set of variants is a mismatch. Past six different variant types, the sets are not tried, and such a field is listed, never reported.{size?: 'sm'} & {size?: 'sm' | 'md'}takes only'sm'), so such a prop is judged by the type the compiler gives it, never by one declaration's text.undefinedon a required prop counts. A requiredisOpen: booleandocumented asboolean | undefined, or a requiredlabel: string | undefineddocumented asstring, is a mismatch, as TypeScript rejects or acceptsundefinedthere.'red' | 'blue'documented for an enumColoris a mismatch, because TypeScript rejects'red'there. A numeric enum, which also takes plain numbers, isn't compared.Recordtakes, are judged against the index's value type, which is what TypeScript checks them against:aria-busy: booleanagainststring | number | undefinedis a mismatch. TypeScript holds a name two different signatures take to both, so such a name is listed as not compared; one signature seen through several union members counts once. A union member that lacks a named prop but takes its name through an index adds the index's type, as one more member a doc may describe.ScrollBehavior) is not resolved, as the code may never mean it; the type is listed as not compared unless the doc writes it as the source does.nullandundefinedat every depth, so the check does too. Whether they are on comes from the project's TypeScript (getStrictOptionValue): TypeScript 6 turns them on when the tsconfig sets neitherstrictnorstrictNullChecks.anyorunknownhas no type to judge, and is listed as not compared.not comparedcounts every documented type the check didn't judge. A standard prop the package declares itself, such as its ownclassName?: string, is judged.Partialof an all-optional type, generic aliases with arguments), each fix grew it, and each review found the next case. Listing those forms ends that class of false finding.Evidence
foundation/discovery/prop-drift.test.mjs:Partial<…>,keyof,string & {}, tuples, records, a type that doesn't resolve, andany;Recordtakes, and names index signatures take;IconName | stringand(IconName | string)[], a string enum or a number literal beside its primitive,1_000), defaults after=(boolean = false,Size = 'md',SizeE = SizeE.Md), a project without strict null checks ((number | null)[]too), and strict null checks as the TypeScript version defaults them;anyandunknownlisted as not compared;Items = string[](judged, a mismatch), and a single index signature seen through two union members (judged once);undefinedon required props, both ways;| undefinedon an optional prop; a doc that describes one member of a discriminated union;classNamethe package declares, judged (a wrong type reported), and a standard prop, another package's prop and a field of one, listed with their reasons;React.Xread through React: the DOM'sKeyboardEventdocumented asReact.KeyboardEventis not a match, andReact.Keyis React's even where the file declares its ownKey, with React imported or global;exactOptionalPropertyTypes,| undefinedon an optional prop judged as TypeScript reads it; a field of an index-only name and an unsplita / btype, listed;| undefinedon a name an index signature takes and on a field aRecordtakes, held to the index's type; a readonly array documented for a mutable one reported, and a mutable array for a readonly one matched;exactOptionalPropertyTypes, a field whose code declares| undefined, held to it, and an alias that includesundefinedshown with it;readonlythat sits elsewhere than the code's, in a nested array, a callback's parameter, an object's field or another union member, listed as not compared; the same readonly text as the code's, whitespace aside, matched.doctor-integration.test.mjs: the types line, and a test that compares the JSON with the text field by field,type_mismatch,typesComparedandtypesNotComparedincluded.Since the first automated review
Alias = …andAlias (…)match only when both parts match, so stale spelled-out values are reported.undefinedis set aside on both sides.required_mismatchassertion ondocRequiredandcodeRequiredis back in the test.After two independent audits and the reviews that followed
The first audit sorted every type finding on Core and on a second real package by hand: 7 of 38 were false, and fixtures found more patterns of the same kind (optional callback parameters, intersections, callback unions written without parentheses,
Partialof an all-optional type, index signatures, tuples). Each was fixed in turn, and the next review found generic aliases with arguments (MaybePromise<void>,Nullable<string>) andkeyof (A & B)read asName (…). The class kept growing, so the comparison now judges only the exact forms above and lists every other type as not compared:IconName | string), two index signatures that take one name, a project without strict null checks,boolean = falseread as an alias, and a field aRecordtakes. A correct doc of 19 props, covering each judged form, passes on TypeScript 4.8, 5.0 and 6.[ok]about names and required flags stays true.After the next review and a third audit
The rule above is the test for each: when the check can't prove its reading exact, the type goes to "not compared", never to a mismatch. Each of these got a test that failed first:
(IconName | string)[]forstring[];(number | null)[]without strict null checks): literal absorption and the null rule now apply at every depth.strictnorstrictNullCheckshas strict null checks on; they were read as off.Size = 'md'andSizeE = SizeE.Mdwere read as stale alias expansions. One type after the name reads as a default now.anyorunknownprop was neither judged nor listed. It is listed as not compared now.ScrollBehavior), was judged against the DOM's version; it is listed as not compared now.Items = string[]passed fornumber[], as any single type after=read as a default; only a literal or a bare name reads as a default now.data-sectiondoc went unjudged; one signature counts once now.codeTypeshows that type rather than the first declaration's text (the fix sits in feat(cli): compare documented component props with the TypeScript source #7663's message helper, the test here).integration.component-conflictsrow in the response-types reference, which the README's JSON table comes from, namestypesComparedandtypesNotCompared; a test checks that it names everypropDriftfield.uncheckedfield's description no longer mentions per-prop entries, which go totypesNotCompared.undefinedon a required prop was always set aside, soboolean | undefinedpassed for a requiredboolean; it is set aside only on an optional prop now.After the round that followed
className,style,xstyle,data-testid,ref,keyand other props declared outside the package were neither judged nor listed. A doc that typed its ownclassName?: stringasnumbergot no warning andnot compared: 0, although the docs and the types line say every unjudged type is listed. These types are listed now undertypesNotCompared, each group with its reason ("standard prop, not compared" or "declared outside the package, not compared"). AclassNamethe package declares itself is judged like any other prop. On Core, 105 more documented types are judged, with no new finding.comparePropTables, and finding each doc's tables lives in feat(cli): doctor integration components compares documented props with the code #7676. Neither move changes this PR's behavior.After the latest review
slotHeader.labelunder[key: `slot${string}`]), and ana / bentry whose one type doesn't split across the names (min / maxtypednumber), were neither judged nor listed, so the types line could saynot compared: 0. Both are listed as not compared now.exactOptionalPropertyTypes, a doc that added| undefinedto an optional prop was judged a match, though TypeScript rejectsprop={undefined}there. The comparison now reads the option. With it set, an optional prop takesundefinedonly when its type says so, and the compiler's mark for an absent prop isn't counted as a type the prop takes.After the review that came next
| undefinedon a name an index signature takes, or on a field aRecordtakes, was judged a match, though TypeScript rejectsundefinedthere unless the index's value type includes it.undefinedis set aside for those only when the index's type takes it now.data-testid: stringunder an index typedstring | undefinedstill matches, so Core's findings are unchanged.readonlyarray documented for a mutable array prop was judged a match, though TypeScript rejects passing a readonly array there. Exact arrays now carry readonly-ness. A readonly array documented for a mutable one is a mismatch; a mutable array documented for a readonly one matches, as TypeScript passes it; and a nested mix is listed as not compared. A text match holds only when the doc writesreadonlyno more than the code. The text normalization is unchanged, so the matcher stays small. Core and Lab judge the same types with the same findings, and the audit's 165 fixtures are unchanged.After the most recent review
exactOptionalPropertyTypes, a field whose code declares| undefined(status?: {tone?: 'a' | 'b' | undefined}) was judged a match for a doc of'a' | 'b', while the same declaration as a top-level prop was reported. The field's text dropped the declaredundefinedalong with the compiler's absent-prop marker. A field keeps the declared one now, as a top-level prop does, and the message shows it.After the round after that
exactOptionalPropertyTypes, a field an index signature takes (labels.close: stringforlabels?: Record<string, string | undefined>) was reported, while a name an index signature takes at the top level matched. The field path had stopped setting the index'sundefinedaside. Both set it aside now, and a field the code declares optional still keeps its declaredundefined.codeTypeleft out theundefinedthe code declares, so a union prop whose members each write| undefinedcould show the same text for both sides. The type and its expansion keep it now.readonlywritten at another depth ((readonly string[])[]forreadonly string[][], and the reverse) passed the text match, thoughtscrejects both. The text match now holds an array only where each depth the doc writesreadonlyis readonly in the code too. Otherwise the structural comparison decides, and a nested mix is listed as not compared. Callbacks and other forms keep the text match, so Lab's callbacks that take readonly arrays are still judged.type-text.mjsand its tests) moved, unchanged, to feat(cli): read two written TypeScript types as one when they differ only in how they are written #7684, stacked between feat(cli): doctor integration components compares documented props with the code #7676 and this PR. This PR's diff no longer holds those files.After the latest audit
exactOptionalPropertyTypes, a prop typed by an alias that includesundefined(t6?: MaybeTone) was flagged correctly against a doc withoutundefined, but its message expanded the alias without theundefinedthat was the whole difference. The message now keeps anundefinedthe code takes itself, written or through an alias.| undefinedshowed it twice, once in the type and once missing from its expansion ('a' | 'b' | undefined ('a' | 'b')). An expansion that differs only by an optional prop'sundefinedisn't shown now.neveras a whole type, a string literal with;in it) is no longer judged a match.readonlysits somewhere other than the code's, in a callback's parameter ((values: string[]) => voidfor(values: readonly string[]) => void), another object field, or another union member, all of whichtscrejects. Text that writesreadonlynow matches only as written: the same text, whitespace aside, withreadonlykept on both sides. Any other type that writesreadonlygoes to the structural comparison, which judges arrays as before and lists the rest. On this repository, 5 of Core's types and 7 of Lab's move from judged to listed: a mutable array documented for a readonly array of callbacks or objects, and docs that spell the same readonly type another way, such as another parameter name. Core judges 1,822 types and Lab 243. Core's 4 findings and their messages are unchanged.After the round that followed
readonlyto the structural comparison, which listed correct docs whose text is the code's own. Text that writesreadonlythe same way on both sides, whitespace aside, is a text match again, so 27 of those 39 types are judged again. The readonly test adds a callback and an object field written as the code writes them; both were listed before.[info]line and the matcher's header said a type is judged when the doc writes it as the source does. They now say "the same as the source (exactly, if it containsreadonly)", which is what the matcher does.IconColorin its declared order, andText.asno longer repeats a type whose members are already written out.Authoring docs
typescriptto feat(cli): read two written TypeScript types as one when they differ only in how they are written #7684's reading, which now parses type text with it. On Core and Lab the same types are judged, with the same findings and messages.type_mismatchto its findings, plus a section on writing atypethe check can judge. That section covers writing it as the source does (exactly, if it writesreadonly), types built from exact parts, an alias with its values,undefinedon an optional prop with and withoutexactOptionalPropertyTypes, and what is listed as not compared, by reason. Each rule was checked against a real run.After the approval
Partial<Record<'a' | 'b', T>>, has no declaration, so it was listed as declared outside the package. The props comparison under this PR now counts such a prop as the package's own (feat(cli): doctor integration components compares documented props with the code #7676), so its type is judged. A test pins atype_mismatchfor one, and keepsdeclared outside the packagefor a prop another package's type builds.After the review of mapped props
props: Many<Base>({[K in keyof Base]: Base[K][]}), a doc copyingBase'ssize: 'sm' | 'md'counted as a match althoughtscrejectssize="sm";Nullable<Base>andRequired<…>over a| undefinedproperty slipped through the same way, and a mismatch showed the source's text ascodeType.PickandOmitdo. A prop a mapping rewrites, or a concrete type instantiates (Box<string>, whosevalue: Tis now astring), is judged by its type, and a message shows that type:('sm' | 'md')[]. The test coversMany,Nullable,Required,Pick, a generic component's alias and a concretely instantiated interface, and failed before.viewprop, documented asScheduleView<Options>while the component takesScheduleView<ScheduleViewOptions>, is now listed as not compared instead of matching on the generic's text.After the review of generic instantiations
function MultiList<T>(props: ListProps<T[]>), a doc copyingvalue?: ToronChange?: (v: T) => voidfromListPropscounted as a match althoughtscrejects both, andListProps<T | null>documented asTpassed without itsnull.Itemthe file namesBItemorNS.Itemstays apart from a type parameter namedItem. Otherwise the prop is judged by its type or listed as not compared, and a message shows the instantiated type, such as(v: T | null) => void. The test addsListProps<T[]>,ListProps<T | null>,type MultiProps<T> = ListProps<T[]>with a correctT[]doc (judged, no finding), andItemList<Item>instantiated with an aliased import, a namespace member (both listed) and the file's ownItem(judged), and failed before. Core and Lab output is byte-identical to the last round.After the review of untraced and unparsed types
{[K in `on${'Open' | 'Close'}`]?: () => void}) was listed intypesNotComparedas "declared outside the package". It is listed as "not a form the check judges exactly" now, in step withpropsNotChecked.number (or a CSS length), is listed too: it was judged onnumberalone, a falsetype_mismatch.[skip] Prop types not judged: TypeScript 4.7.4 is older than 4.8, the oldest the type comparison reads., and JSON addstypesSkipped. The names andrequiredcomparison still runs. A test with a stubbed TypeScript 4.7.4 covers it.typesSkipped, says which types land under "not a form the check judges exactly", and now carries the[info]line's "(exactly, if it containsreadonly)". The README row and the props guide say so too.After the approval, round 21
typesSkippedwhen it can't run. This PR is rebased on the bases' new heads; nothing else in it changed.After the round-21 review
data-variantas'solid' | 'outline'drewtype_mismatchagainst another package's[key: `data-${string}`]: string | undefined, whilexstylefrom the same type was listed. Such a name is now judged only when exactly one signature takes it and the package's own source declares that signature:Record, or a signature passed throughOmit) has no declaration to say whose it is. It is listed as not a form the check judges exactly.CollapsibleGroup'sdata-testid, which reaches Core's own signature throughOmit.not_in_codeand nothing else.data-variantagainst a dependency's signature, an ownRecordand a missing field. It failed before. To make room, the two alias tests are now one.After the round-22 review
Colorprop asHue(orColor[]asHue[]) matched, thoughtscrejectsHue.Redthere (TS2322). A string enum member's part now names its enum as well as its value, so both are mismatches.tscaccepts.'red' | 'blue'for aColorprop is a mismatch, astscrejects'red'there (TS2345).Colorfor a'red' | 'blue'prop is one too:tscpassesColor.Redthere, butColordoes not take the'red'the prop does.tscrelates them by their members, comparing member names on TypeScript 4.8 and 5.0 and names and values on 6.0, so no one answer is exact. An independent audit found that the first version of this change flaggedTheme.Colordocumented as a top-levelColorwith the same members.a / bentry go into the index-signature test, and an alias's member order goes into the| undefinedtest.Rebased onto current main
After the latest review
a / bentry whose one type doesn't split listed a missing field, such asstatus.b, as not compared beside itsnot_in_codewarning. A field there is now listed only once it is found, as other fields are, so a missing one is reported once. The index-signature test adds the pair and failed before.HandledCellbesideHandled; it failed before. The enum test's expected order follows the new finding order (shadebeforeshades).Restacked
After the review of documented match rules
integrationComponentConflictsreference now say so, with the reverse, which is a mismatch.sel.value: stringfor{value: string} | {value: number}, counted as a match, though the field takesstring | number. It is now listed as not compared, with the reason "the prop's variants give this field different types", as is a doc that gives the types of only some variants (string | numberof three), and a doc that gives every variant's type is judged. The guide and the reference say so. The index-signature test documentssel.valuewith one variant's type,tri.vwith two of three,sev.vwith two of seven, andlabels.closewith both; it failed before. A prop that differs between the members of a union of props types keeps the rule above: a doc may describe one member.type-match.mjs) moved unchanged to feat(cli): read two written TypeScript types as one when they differ only in how they are written #7684, which now tests it directly, and this PR carries feat(cli): compare documented component props with the TypeScript source #7663's test of props from other packages and the exempt list, unchanged. Core and Lab give the same results.After the latest approval
typesNotComparedreasons, and thePropDriftTypesNotComparedJSDoc, now name "the prop's variants give this field different types" too.After the review of
React.namesReact.XthroughReactwhere the prop is declared, and never reads it as a bareXthat names another type there. A new test here covers both reported cases: a handler typed with the DOM'sKeyboardEventdocumented asReact.KeyboardEventis listed as not compared instead of matched, andReact.Keydocumented forstring | number | bigintmatches where the file declares its ownKey, which a doc that writes the bareKeywould not; React imported and React's global namespace both work. It failed before. The props guide and the API reference state the rule, and the guide now also says that a union member another covers adds nothing, as feat(cli): read two written TypeScript types as one when they differ only in how they are written #7684's judge reads it.MouseEventfor the code'sReact.MouseEvent(Item, ListItem and SideNavItemonClick) move from judged to not compared. No finding changes.After the review of names a file imports under another name
Item as BaseItemorSize as BaseSizeand declares its ownItemorSize, wrong docs such ascfg.item: Itemorsize-a: Sizewere judged matches, thoughtscrejects them.Size, a literal union) or listed (Item, an object type), while docs that name the type as the declaring file does still match. A new test covers a field, an index name, and props declared in another file (direct, a callback taking the type, and mapped); it failed before. The other tests and Core's results are unchanged. The props guide now says to name a type as the file that declares the prop names it, a field taking its prop's file.Merge order
The order is required:
doctor integration components, stacked on feat(cli): compare documented component props with the TypeScript source #7663), then feat(cli): read two written TypeScript types as one when they differ only in how they are written #7684 (reading two written types as one, stacked on feat(cli): doctor integration components compares documented props with the code #7676), then feat(cli): doctor integration components judges documented prop types against the code #7667 (prop types, stacked on feat(cli): read two written TypeScript types as one when they differ only in how they are written #7684).validate, stacked on feat(cli): check each authored doc against its published authoring type #7664).integration check) last, after a rebase that adds these PRs' fields and codes to its failures list. Merged earlier, it breaks main.This PR must ship in the same release as
integration check. If it slips past that release, it can ship only in a minor, because its findings failintegration check --strict.Overlap with open PRs
astryx componentprints #7623, fix(cli): init writes agent docs every agent reads, at the project root, never in the home folder #7497, spec(AST-039, cli-surface): integration docs are found by their documented names and checked in one command #7658, feat(cli): a component doc can name a differently named export of the file beside it #7659, fix(cli): integration verify reads exports with the reader discovery uses #7660, feat(docsite): put the integration guide's pages under CLI Integrations in the sidebar #7661, feat(cli): integration family members are components under their own names #7674, or the other PRs in this set.Size
1,813 changed lines.
Scope
Testing
The CI test jobs run only for pull requests into main, so this PR's tests run in CI once it is retargeted after #7684 merges. On main 9b8b723 with #7681, #7663, #7676, #7684 and this branch: copyright, sync-exports, the published
./apitype check,readme:check, the four CLI typechecks (strict, json-api, authoring-contract, template-docs),check:changesets, the lint job's changeset coverage check (run on the merge commit GitHub builds),check:cli-structure,check:repo, eslint on the changed files, prettier on the new files, and the fullpnpm vitest run --project nodesuite: 10,415 tests pass. The 22 failures are in the npm pack, packed real-world package, theme resolution, CSS graph and browser accessibility tests, which fail the same way on main 9b8b723 on the same machine (they need a package registry and a browser it doesn't reach), plus a packed-consumer test that passes on its own in a full checkout. Earlier in review, with all nine PRs merged into main, the fourdoctor integrationsubcommands were run in text and JSON on 64 package shapes, against main: templates output is byte-identical everywhere;validateandcomponentsonly add their new section and JSON field;docsdiffers only where the doc-links PR fixes a link warning.integration checkexits 1 exactly when one of the four would, each of its sections is byte-identical to that subcommand's output, and onlyintegration check's three list tests fail until its planned rebase.