Skip to content

feat(cli): doctor integration components judges documented prop types against the code - #7667

Merged
josephfarina merged 2 commits into
mainfrom
josephfarina/doctor-prop-types
Oct 19, 2026
Merged

josephfarina merged 2 commits into
mainfrom
josephfarina/doctor-prop-types

Conversation

@josephfarina

@josephfarina josephfarina commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

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 required flags 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 in astryx 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:

  • the doc writes the type as the source writes it (or as the compiler prints it), once whitespace, quotes, union order and an optional prop's undefined are set aside; or
  • both sides hold only primitives (string, 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 | string is string), undefined is set aside on an optional prop (which takes it anyway), and null and undefined are 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, any or unknown, 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. A className the 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 tone gained a value its doc doesn't list, and meta is an object type:

severity:  [warn]
component: AcmeButton
prop:      tone
kind:      type_mismatch
message:   "tone" is documented as 'neutral' | 'accent', but the code declares AcmeTone ('neutral' | 'accent' | 'danger').
doc:       components/AcmeButton.doc.mjs
source:    components/AcmeButton.tsx
docType:   'neutral' | 'accent'
codeType:  AcmeTone

[info] Documented prop types judged: 4; not compared: 1. A type is judged when the doc writes it as the source does, or when both are primitives, literals, arrays and unions of those, or names for them.

component: AcmeButton
doc:       components/AcmeButton.doc.mjs
reason:    not a form the check judges exactly
props:     meta

The same doc writes size as 'md' | 'sm' for the code's 'sm' | 'md', and onPress as () => void exactly 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, PowerSearch statusVariant, Text as, and Icon color. They join core-prop-docs.known-drift.json.

Current specification

Implements the type comparison of architecture:cli-surface INV33, "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: typesNotCompared is that list. #7663 and #7676 implement the rest of INV33.

Public delta and caller burden

  • A new finding kind, type_mismatch, in data.propDrift.findings, carrying docType (as the doc writes it) and codeType (as the source writes it). It is a warning, so exit codes are unchanged.
  • data.propDrift gains typesCompared (how many documented types were judged) and typesNotCompared ({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.
  • The text prints, after the findings, how many documented types were judged and how many were not. Then it prints one record per component and reason, naming the props not compared.
  • No new command, flag, option or environment variable.

Design and alternatives

  • Exact or not at all. A type is judged when its text matches the source's, or when both sides are built from parts the check can compare exactly. Everything else is listed, so a reader sees what wasn't checked instead of a guess.
  • Names resolve where the prop is declared. Tone in a doc resolves to the code's 'neutral' | 'info'. Name = … and Name (…) judge both parts, so stale spelled-out values are reported (Items = string[] for number[] 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 as keyof (A & B), is never read as that notation.
  • A union of props types whose members give a prop different types (a discriminated union): any member's type, or their union, matches, since a doc may describe one member. It is a mismatch only against every one.
  • A field the prop's variants give different types (sel.value in {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.
  • Readonly arrays. A mutable array type matches a readonly one, because callers can pass it; a readonly one documented for a mutable array is a mismatch. Declarations an intersection joins narrow each other ({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.
  • undefined on a required prop counts. A required isOpen: boolean documented as boolean | undefined, or a required label: string | undefined documented as string, is a mismatch, as TypeScript rejects or accepts undefined there.
  • Enums. A string enum is judged: 'red' | 'blue' documented for an enum Color is a mismatch, because TypeScript rejects 'red' there. A numeric enum, which also takes plain numbers, isn't compared.
  • Index signatures. A documented name one index signature takes, and a documented field a Record takes, are judged against the index's value type, which is what TypeScript checks them against: aria-busy: boolean against string | number | undefined is 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.
  • Names the code can mean. A name only TypeScript's own libraries declare (the DOM's 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.
  • Null checks as the project has them. Without strict null checks the compiler drops null and undefined at 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 neither strict nor strictNullChecks.
  • A field under any or unknown has no type to judge, and is listed as not compared.
  • Only props the package declares are judged, and every other one is counted. The type of an inherited DOM prop isn't the package's to document, so it isn't judged. It is still listed with its reason, so not compared counts every documented type the check didn't judge. A standard prop the package declares itself, such as its own className?: string, is judged.
  • Rejected: a structural matcher for callbacks, objects, intersections and generics. An earlier version of this PR had one. It reported correct docs (optional callback parameters, intersections, Partial of 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

  • The text reading's tests, and the type judge with a direct test of its own, are in feat(cli): read two written TypeScript types as one when they differ only in how they are written #7684.
  • foundation/discovery/prop-drift.test.mjs:
    • a wrong type, with its alias expanded;
    • primitives, literals, arrays and names judged in any order, with a wrong primitive, a wrong array element, and an enum written as its values reported;
    • every other form listed as not compared, never reported: callbacks (one written as the source writes it is judged a match), objects, intersections, Partial<…>, keyof, string & {}, tuples, records, a type that doesn't resolve, and any;
    • a documented field's type, a field a Record takes, and names index signatures take;
    • unions reduced as TypeScript reduces them, at any depth (IconName | string and (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;
    • a name two index signatures take, in one member or in two union members, listed as not compared; a field documented with one variant's type, two of three, or two of seven, listed, and one documented with every variant's type, judged; fields under any and unknown listed as not compared;
    • a field in an array of a union, a name one member indexes and the other names, a lib-only name the code never uses (not compared), Items = string[] (judged, a mismatch), and a single index signature seen through two union members (judged once);
    • a prop two intersected types declare, judged by its narrowed type and shown with it; undefined on required props, both ways;
    • an alias spelled out beside its name, and a stale one; | undefined on an optional prop; a doc that describes one member of a discriminated union;
    • a className the package declares, judged (a wrong type reported), and a standard prop, another package's prop and a field of one, listed with their reasons;
    • props from other packages and the exempt list never asked for (feat(cli): compare documented component props with the TypeScript source #7663's test, carried here to keep that PR under 2,000 lines);
    • a type a file imports under another name, printed as that file names it, for a field, an index name and a mapped prop declared in another file;
    • React.X read through React: the DOM's KeyboardEvent documented as React.KeyboardEvent is not a match, and React.Key is React's even where the file declares its own Key, with React imported or global;
    • with exactOptionalPropertyTypes, | undefined on an optional prop judged as TypeScript reads it; a field of an index-only name and an unsplit a / b type, listed;
    • | undefined on a name an index signature takes and on a field a Record takes, held to the index's type; a readonly array documented for a mutable one reported, and a mutable array for a readonly one matched;
    • with exactOptionalPropertyTypes, a field whose code declares | undefined, held to it, and an alias that includes undefined shown with it;
    • readonly that 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, typesCompared and typesNotCompared included.
  • Core: 4 type mismatches, each read by hand and each a real difference, now in the known list. 1,822 documented types are judged. 121 are listed: 94 not an exact form, 24 standard props and 3 from another package. The comparison takes about 6 s.

Since the first automated review

  • Alias = … and Alias (…) match only when both parts match, so stale spelled-out values are reported.
  • An optional prop's undefined is set aside on both sides.
  • The required_mismatch assertion on docRequired and codeRequired is 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, Partial of 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>) and keyof (A & B) read as Name (…). The class kept growing, so the comparison now judges only the exact forms above and lists every other type as not compared:

  • The comparison shrank from about 1,300 lines to 660, and Core's type findings from 30 to the 4 real ones above.
  • A second audit found no false type finding on Core or on the second package, and the fixtures it wrote to make a correct doc fail found five patterns, each fixed here with a test that failed first: a name with a literal union beside its primitive (IconName | string), two index signatures that take one name, a project without strict null checks, boolean = false read as an alias, and a field a Record takes. A correct doc of 19 props, covering each judged form, passes on TypeScript 4.8, 5.0 and 6.
  • A prop whose type doesn't resolve is listed under types not compared, so the [ok] about names and required flags stays true.
  • A name an index signature takes is judged against the index's value type, and a documented field's type is judged too.
  • A long expansion in a message is cut at 400 characters, not dropped.

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:

  • An array of a union was reported ((IconName | string)[] for string[]; (number | null)[] without strict null checks): literal absorption and the null rule now apply at every depth.
  • With TypeScript 6, a tsconfig that sets neither strict nor strictNullChecks has strict null checks on; they were read as off.
  • Size = 'md' and SizeE = SizeE.Md were read as stale alias expansions. One type after the name reads as a default now.
  • A field under an any or unknown prop was neither judged nor listed. It is listed as not compared now.
  • A name two index signatures take was judged against one of them; it is listed as not compared now. A field documented for one member of a union prop was reported; the one-member rule covers fields now.
  • A field in an array of a union was reported for one member's type; the one-member rule reaches into arrays now.
  • A name one union member declares and another takes through an index signature was judged against the named member only; the index's type joins as another member now.
  • A doc naming a type only TypeScript's libraries declare, which the code never uses (ScrollBehavior), was judged against the DOM's version; it is listed as not compared now.
  • Items = string[] passed for number[], as any single type after = read as a default; only a literal or a bare name reads as a default now.
  • An index signature intersected with a union counted once per member, so a correct data-section doc went unjudged; one signature counts once now.
  • A prop two intersected types declare passed on either declaration's text, although TypeScript narrows it; it is judged by its narrowed type now, and its codeType shows 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).
  • The integration.component-conflicts row in the response-types reference, which the README's JSON table comes from, names typesCompared and typesNotCompared; a test checks that it names every propDrift field.
  • The types line names the text match too, which it judges in any form, and the unchecked field's description no longer mentions per-prop entries, which go to typesNotCompared.
  • undefined on a required prop was always set aside, so boolean | undefined passed for a required boolean; it is set aside only on an optional prop now.
  • The audit's re-sweeps found no false type finding on Core (4 real ones) or on the second package (none).

After the round that followed

  • The types of className, style, xstyle, data-testid, ref, key and other props declared outside the package were neither judged nor listed. A doc that typed its own className?: string as number got no warning and not compared: 0, although the docs and the types line say every unjudged type is listed. These types are listed now under typesNotCompared, each group with its reason ("standard prop, not compared" or "declared outside the package, not compared"). A className the package declares itself is judged like any other prop. On Core, 105 more documented types are judged, with no new finding.
  • Rebased on feat(cli): compare documented component props with the TypeScript source #7663's split. The comparison this PR extends is now 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

  • A documented field of a name only an index signature takes (slotHeader.label under [key: `slot${string}`]), and an a / b entry whose one type doesn't split across the names (min / max typed number), were neither judged nor listed, so the types line could say not compared: 0. Both are listed as not compared now.
  • With exactOptionalPropertyTypes, a doc that added | undefined to an optional prop was judged a match, though TypeScript rejects prop={undefined} there. The comparison now reads the option. With it set, an optional prop takes undefined only when its type says so, and the compiler's mark for an absent prop isn't counted as a type the prop takes.
  • Rebased on feat(cli): compare documented component props with the TypeScript source #7663, which now sits on feat(cli): read a package's TypeScript project the way tsc reads it #7681.

After the review that came next

  • A documented | undefined on a name an index signature takes, or on a field a Record takes, was judged a match, though TypeScript rejects undefined there unless the index's value type includes it. undefined is set aside for those only when the index's type takes it now. data-testid: string under an index typed string | undefined still matches, so Core's findings are unchanged.
  • A readonly array 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 writes readonly no 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

  • With 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 declared undefined along 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.
  • Rebased on feat(cli): compare documented component props with the TypeScript source #7663's import rule. A prop whose type comes from a missing import now leaves its whole component unchecked there, so the test of types never compared writes its unresolved type as a name declared nowhere. Core and Lab judge the same types, with the same findings and the same not-compared lists.

After the round after that

  • With exactOptionalPropertyTypes, a field an index signature takes (labels.close: string for labels?: 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's undefined aside. Both set it aside now, and a field the code declares optional still keeps its declared undefined.
  • With that option, the message and codeType left out the undefined the code declares, so a union prop whose members each write | undefined could show the same text for both sides. The type and its expansion keep it now.
  • readonly written at another depth ((readonly string[])[] for readonly string[][], and the reverse) passed the text match, though tsc rejects both. The text match now holds an array only where each depth the doc writes readonly is 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.
  • Each fix has a test that failed first. Core and Lab are unchanged: the same 4 findings, the same 1,827 and 250 types judged, and the same not-compared lists.
  • These fixes took this PR past 2,000 lines, so the text reading (type-text.mjs and 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

  • With exactOptionalPropertyTypes, a prop typed by an alias that includes undefined (t6?: MaybeTone) was flagged correctly against a doc without undefined, but its message expanded the alias without the undefined that was the whole difference. The message now keeps an undefined the code takes itself, written or through an alias.
  • Without that option, an optional prop that writes | undefined showed 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's undefined isn't shown now.
  • With feat(cli): read two written TypeScript types as one when they differ only in how they are written #7684's fixes, a type the text match read as another (an array of callbacks against a callback returning an array, never as a whole type, a string literal with ; in it) is no longer judged a match.
  • The text match still accepted a doc whose readonly sits somewhere other than the code's, in a callback's parameter ((values: string[]) => void for (values: readonly string[]) => void), another object field, or another union member, all of which tsc rejects. Text that writes readonly now matches only as written: the same text, whitespace aside, with readonly kept on both sides. Any other type that writes readonly goes 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.
  • Each has a test that failed first.

After the round that followed

  • The previous revision left every type that writes readonly to the structural comparison, which listed correct docs whose text is the code's own. Text that writes readonly the 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.
  • The [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 contains readonly)", which is what the matcher does.
  • A type message names an alias's members in the order the source declares them (from feat(cli): compare documented component props with the TypeScript source #7663), so it no longer moves with what the compiler read first. A test pins it for a mismatch like Core's. Core's messages now list IconColor in its declared order, and Text.as no longer repeats a type whose members are already written out.

Authoring docs

After the approval

  • The optional follow-up is taken. A prop that the package's own mapped type builds, such as one from 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 a type_mismatch for one, and keeps declared outside the package for a prop another package's type builds.

After the review of mapped props

  • A prop that a mapping in the package rewrites still carried the declaration of the property it was copied from, so it was judged against that text. With props: Many<Base> ({[K in keyof Base]: Base[K][]}), a doc copying Base's size: 'sm' | 'md' counted as a match although tsc rejects size="sm"; Nullable<Base> and Required<…> over a | undefined property slipped through the same way, and a mismatch showed the source's text as codeType.
  • Now a declaration's text counts only for the prop's own symbol, for the generic type's prop it instantiates with type parameters (as a generic component does), or where a mapping copies its type unchanged, as Pick and Omit do. A prop a mapping rewrites, or a concrete type instantiates (Box<string>, whose value: T is now a string), is judged by its type, and a message shows that type: ('sm' | 'md')[]. The test covers Many, Nullable, Required, Pick, a generic component's alias and a concretely instantiated interface, and failed before.
  • On Core and Lab the findings are unchanged. Core judges the same 1,822 types. Lab judges 242, one fewer: Schedule's view prop, documented as ScheduleView<Options> while the component takes ScheduleView<ScheduleViewOptions>, is now listed as not compared instead of matching on the generic's text.

After the review of generic instantiations

  • The text of a generic's prop counted wherever its instantiation still named a type parameter. A component's own type built from its parameter slipped through: with function MultiList<T>(props: ListProps<T[]>), a doc copying value?: T or onChange?: (v: T) => void from ListProps counted as a match although tsc rejects both, and ListProps<T | null> documented as T passed without its null.
  • The declaration's text now counts only when the instantiated type prints exactly as the declared one in the component's file, as when a generic component passes its own type parameters on. Printed there, an imported Item the file names BItem or NS.Item stays apart from a type parameter named Item. 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 adds ListProps<T[]>, ListProps<T | null>, type MultiProps<T> = ListProps<T[]> with a correct T[] doc (judged, no finding), and ItemList<Item> instantiated with an aliased import, a namespace member (both listed) and the file's own Item (judged), and failed before. Core and Lab output is byte-identical to the last round.

After the review of untraced and unparsed types

  • A documented prop built by the package's own mapped type from keys the check can't trace ({[K in `on${'Open' | 'Close'}`]?: () => void}) was listed in typesNotCompared as "declared outside the package". It is listed as "not a form the check judges exactly" now, in step with propsNotChecked.
  • A documented type that does not parse never matches now. feat(cli): read two written TypeScript types as one when they differ only in how they are written #7684 marks such text, and it is listed as not compared. Prose in parentheses after a type, number (or a CSS length), is listed too: it was judged on number alone, a false type_mismatch.
  • With a project TypeScript older than 4.8, no type is judged: the text prints [skip] Prop types not judged: TypeScript 4.7.4 is older than 4.8, the oldest the type comparison reads., and JSON adds typesSkipped. The names and required comparison still runs. A test with a stubbed TypeScript 4.7.4 covers it.
  • The API reference names typesSkipped, says which types land under "not a form the check judges exactly", and now carries the [info] line's "(exactly, if it contains readonly)". The README row and the props guide say so too.
  • Each change has a test that failed before. Core and Lab output is byte-identical.

After the approval, round 21

  • The changeset now says the type comparison needs TypeScript 4.8 or later, and that JSON adds typesSkipped when it can't run. This PR is rebased on the bases' new heads; nothing else in it changed.

After the round-21 review

  • A documented name that only index signatures take was judged against any of them. So a doc listing data-variant as 'solid' | 'outline' drew type_mismatch against another package's [key: `data-${string}`]: string | undefined, while xstyle from 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:
    • One that only another package's declared signature takes is listed as declared outside the package.
    • One a mapped type's signature takes (Record, or a signature passed through Omit) has no declaration to say whose it is. It is listed as not a form the check judges exactly.
    • On Core this lists one name that was judged before: CollapsibleGroup's data-testid, which reaches Core's own signature through Omit.
  • A field of another package's prop is listed as not compared only once the field is found. A missing one is reported as not_in_code and nothing else.
  • The className test adds data-variant against a dependency's signature, an own Record and a missing field. It failed before. To make room, the two alias tests are now one.

After the round-22 review

  • Two string enums with the same values read as one type, so a doc typing a Color prop as Hue (or Color[] as Hue[]) matched, though tsc rejects Hue.Red there (TS2322). A string enum member's part now names its enum as well as its value, so both are mismatches.
  • A string-literal union of the same values is decided the same way, by what tsc accepts. 'red' | 'blue' for a Color prop is a mismatch, as tsc rejects 'red' there (TS2345). Color for a 'red' | 'blue' prop is one too: tsc passes Color.Red there, but Color does not take the 'red' the prop does.
  • Two different enums of one name are listed as not compared. tsc relates 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 flagged Theme.Color documented as a top-level Color with the same members.
  • The primitives test adds the enum and literal cases, and the not-compared test adds the same-named pair; both failed before. To make room, two tests are folded into others: a field only an index takes and an a / b entry go into the index-signature test, and an alias's member order goes into the | undefined test.

Rebased onto current main

  • Rebased onto main 9b8b723 with the PRs below it. Nothing in this PR changed.

After the latest review

  • An a / b entry whose one type doesn't split listed a missing field, such as status.b, as not compared beside its not_in_code warning. 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.
  • The not-compared list was sorted by a NUL-joined key, which collation reads as one run of text, like the findings feat(cli): compare documented component props with the TypeScript source #7663 fixed. It now sorts by component, then reason. The mapped-type test pins it with a HandledCell beside Handled; it failed before. The enum test's expected order follows the new finding order (shade before shades).

Restacked

  • Rebased onto the PRs below it. Nothing in this PR changed.

After the review of documented match rules

  • Two match rules weren't documented, though INV33 holds the warning to the comparison the command documents. A mutable array type documented where the code takes a readonly one matches, because callers can pass it. The props guide's "Write types the check can judge" list and the integrationComponentConflicts reference now say so, with the reverse, which is a mismatch.
  • A field of a union-typed prop documented with one variant's type, such as sel.value: string for {value: string} | {value: number}, counted as a match, though the field takes string | 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 | number of three), and a doc that gives every variant's type is judged. The guide and the reference say so. The index-signature test documents sel.value with one variant's type, tri.v with two of three, sev.v with two of seven, and labels.close with 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.
  • To keep this PR under 2,000 lines, the type judge (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

  • Rebased onto the PRs below it. The reference's list of typesNotCompared reasons, and the PropDriftTypesNotCompared JSDoc, now name "the prop's variants give this field different types" too.

After the review of React. names

  • The judge in feat(cli): read two written TypeScript types as one when they differ only in how they are written #7684 now resolves React.X through React where the prop is declared, and never reads it as a bare X that names another type there. A new test here covers both reported cases: a handler typed with the DOM's KeyboardEvent documented as React.KeyboardEvent is listed as not compared instead of matched, and React.Key documented for string | number | bigint matches where the file declares its own Key, which a doc that writes the bare Key would 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.
  • On Core, three handlers whose docs write the DOM's MouseEvent for the code's React.MouseEvent (Item, ListItem and SideNavItem onClick) move from judged to not compared. No finding changes.

After the review of names a file imports under another name

  • A documented field, a name an index signature takes, and a prop declared in another file were text-matched against the code's type as printed outside the scope where the doc's names resolve. So where a 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 were judged matches, though tsc rejects them.
  • Those types are now printed where the prop, or its index signature, is declared. The wrong docs are reported (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:

  1. feat(cli): read a package's TypeScript project the way tsc reads it #7681 (reading the package's TypeScript project), then feat(cli): compare documented component props with the TypeScript source #7663 (the prop comparison, stacked on feat(cli): read a package's TypeScript project the way tsc reads it #7681), then feat(cli): doctor integration components compares documented props with the code #7676 (props in 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).
  2. feat(cli): check each authored doc against its published authoring type #7664 (the doc type check), after feat(cli): read a package's TypeScript project the way tsc reads it #7681: once feat(cli): read a package's TypeScript project the way tsc reads it #7681 merges, feat(cli): check each authored doc against its published authoring type #7664 is rebased onto main, which takes out the 65 lines the two share and brings it to 1,960, and reviewed again. Then feat(cli): doctor integration validate checks each doc against its published authoring type #7673 (doc types in validate, stacked on feat(cli): check each authored doc against its published authoring type #7664).
  3. fix(cli): doc links to a component, page, or block resolve #7668 (doc links to components and templates) any time before feat(cli): integration check runs every source check for a package's build, with --strict #7665.
  4. feat(cli): integration check runs every source check for a package's build, with --strict #7665 (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 fail integration check --strict.

Overlap with open PRs

Size

1,813 changed lines.

Scope

  • One capability is added.
  • Unrelated fixes, cleanup, and policy changes were removed or split.
  • Consumer docs, focused tests, and migration evidence ship with the feature.
  • Public text and artifacts contain no internal Meta context.

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 ./api type 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 full pnpm vitest run --project node suite: 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 four doctor integration subcommands were run in text and JSON on 64 package shapes, against main: templates output is byte-identical everywhere; validate and components only add their new section and JSON field; docs differs only where the doc-links PR fixes a link warning. integration check exits 1 exactly when one of the four would, each of its sections is byte-identical to that subcommand's output, and only integration check's three list tests fail until its planned rebase.

@vercel

vercel Bot commented Oct 17, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
astryx Ready Ready Preview Oct 19, 2026 11:22pm UTC

Request Review

@astracat-bot

astracat-bot Bot commented Oct 17, 2026 •

Copy link
Copy Markdown

AI review status for this pull request.

Review status Updated
🔄 In progress (for maintainers only) Oct 19, 2026, 11:23 PM UTC

@github-actions

github-actions Bot commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

PR Analysis Report

Previews ready: Storybook and Sandbox were built with this PR's docsite. CI evidence will be added when available.

View Storybook for this PR · View Sandbox for this PR

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 = … or Alias (…) passes even when the spelled-out values are stale, because typeMatches accepts either part (prop-drift.mjs, docTypeVariants).
  • An optional prop documented with an explicit | undefined is reported as a mismatch when the code writes the type through an alias, because docTypeMatches drops undefined only 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 =>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.mjs reads an optional parameter's type as including undefined. This causes four of the five type warnings on Lab (onReact and onSubmit on Comments and CommentThread).
  • A doc that spells out an intersection alias (A & B, or AB = A & B) is reported, which breaks the published rule that alias spelling does not count.
  • For an optional method prop, codeType reads undefined.

[Automated review]

at(ctx, params[i]),
param.type,
constituents(
checker.getTypeOfSymbolAtLocation(params[i], ctx.location),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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('|');

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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] ?? '')

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 code prints for a component whose prop type the same output lists under "Props not compared", because an unresolved prop type now adds an unchecked entry 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, because type-match.mjs resolves only names written without type arguments.
  • keyof (A & B) documented exactly as the source writes it is read as the Alias (…) 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}).`,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +598 to +606
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;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +450 to +453
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];

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +497 to +508
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;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@josephfarina
josephfarina force-pushed the josephfarina/doctor-prop-drift branch from 8b14433 to 5ddd619 Compare October 18, 2026 02:16
@josephfarina
josephfarina changed the base branch from josephfarina/doctor-prop-drift to josephfarina/doctor-prop-drift-components October 18, 2026 02:16
@josephfarina
josephfarina force-pushed the josephfarina/doctor-prop-types branch from 87d060f to ddc6822 Compare October 18, 2026 02:17
@josephfarina josephfarina changed the title feat(cli): doctor integration components compares documented prop types with the code feat(cli): doctor integration components judges documented prop types against the code Oct 18, 2026

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 is string[], or (number | null)[] in a project without strict null checks, is reported, because literal absorption and the null rule in type-match.mjs apply only at the top level.
  • With TypeScript 6, a tsconfig that sets neither strict nor strictNullChecks has strict null checks, but prop-drift.mjs reads it as non-strict, so a doc that leaves out | null passes; ts.getStrictOptionValue gives 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 an any or unknown prop 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;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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[].

Comment on lines +1044 to +1048
const nullable = !(
compilerOptions.strictNullChecks ??
compilerOptions.strict ??
false
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@josephfarina
josephfarina force-pushed the josephfarina/doctor-prop-drift-components branch from e881162 to e7ce3a1 Compare October 18, 2026 04:10

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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]

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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]

Comment on lines +146 to +149
const outside = name =>
PROPS_NOT_DOCUMENTED.includes(name)
? 'standard prop, not compared'
: 'declared outside the package, not compared';

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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, ' +

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 0907e58. The API reference now says the text match is "exact, if it contains readonly", as the [info] line does.

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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]

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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], {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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') {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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}`);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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]

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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]

Comment on lines +1659 to +1670
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,
);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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]

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.',

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 ' +

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).',

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.',

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)]],

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +561 to +566
printed.push(checker.typeToString(part, sourceFile, flags));
expanded.push(
checker.typeToString(
part,
sourceFile,
flags | ts.TypeFormatFlags.InTypeAlias,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

This branch was successfully deployed

1 active deployment
Preview — a566ea32 Deployed Oct 19, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Meta Open Source bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant