Skip to content

Commit 13ce5e8

Browse files
committed
[docs-infra] Fix duplicate JSDoc in proptypes generation
`getSymbolDocumentation` (introduced in mui#47685) concatenated JSDoc from all declarations when a symbol had multiple (intersection, declaration merging, module augmentation). This produced duplicated descriptions in generated propTypes. Fix: use "last declaration wins" for all multi-declaration cases, consistent with `squashPropTypeDefinitions`. Added tests for intersection, interface extends, interface override, and declaration merging scenarios. This change has no effect on this repo and affects around 7 api/proptypes generation in X repo.
1 parent 36d3f89 commit 13ce5e8

10 files changed

Lines changed: 244 additions & 29 deletions

File tree

packages-internal/scripts/typescript-to-proptypes/src/getPropTypesFromFile.ts

Lines changed: 22 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -34,30 +34,28 @@ function getSymbolDocumentation({
3434
}: {
3535
symbol: ts.Symbol | undefined;
3636
project: TypeScriptProject;
37-
parentType?: ts.Type;
3837
}): string | undefined {
3938
if (symbol === undefined) {
4039
return undefined;
4140
}
4241

4342
const decl = symbol.getDeclarations();
4443
if (decl && decl.length > 0) {
45-
// This behavior tries to replicate how TypeScript itself merges JSDoc comments
46-
// It is a complex logic that changes based on the kind of declarations
44+
// This behavior tries to replicate how TypeScript itself merges JSDoc comments.
45+
// It is a complex logic that changes based on the kind of declarations.
4746
// There is an open issue for it in: https://github.com/microsoft/TypeScript/issues/30901
4847
//
49-
// For intersection types (A & B), the symbol may have multiple declarations.
50-
// We need to handle three cases:
51-
// 1. Intersection (type C = A & B): merge JSDoc from all declarations (deduplicated)
52-
// 2. Interface extends (interface Z extends X, Y): use the (only) declaration's JSDoc
53-
// 3. Interface override (interface W extends X { prop }): use the override's JSDoc (which is the only declaration)
48+
// The symbol may have multiple declarations in several scenarios:
49+
// 1. Intersection (type C = A & B): one declaration per constituent type
50+
// 2. Interface extends (interface Z extends X, Y): single declaration from the original interface
51+
// 3. Interface override (interface W extends X { prop }): single declaration from the overriding interface
52+
// 4. Declaration merging / module augmentation: one declaration per interface opening
5453
//
55-
// Note: TypeScript gives us:
56-
// - Multiple declarations for intersection types (one from each constituent type)
57-
// - Single declaration for interface extends (from the original interface)
58-
// - Single declaration for interface override (from the overriding interface)
59-
60-
// Get JSDoc comments paired with their declarations
54+
// In all cases we use the last declaration that has JSDoc. This is consistent with
55+
// squashPropTypeDefinitions and avoids duplicated descriptions when multiple
56+
// declarations carry different JSDoc (e.g. module augmentation overriding
57+
// the original, or intersection types where the last constituent's JSDoc
58+
// is the most specific).
6159
const declarationsWithComments = decl
6260
.map((d) => {
6361
const jsDocNodes = ts.getJSDocCommentsAndTags(d).filter((node) => ts.isJSDoc(node));
@@ -67,19 +65,17 @@ function getSymbolDocumentation({
6765
: undefined;
6866
return { declaration: d, comment };
6967
})
70-
.filter((item) => item.comment !== undefined);
68+
.filter(
69+
(item): item is { declaration: ts.Declaration; comment: string } =>
70+
item.comment !== undefined,
71+
);
7172

7273
if (declarationsWithComments.length > 0) {
73-
// If there's only one declaration with a comment, use it
74-
// This handles both interface extends and interface override cases
75-
if (declarationsWithComments.length === 1) {
76-
return declarationsWithComments[0].comment;
77-
}
78-
79-
// Multiple declarations with comments - this is the intersection case (type C = A & B)
80-
// Merge JSDoc comments, deduplicating identical ones
81-
const uniqueComments = [...new Set(declarationsWithComments.map((d) => d.comment))];
82-
return uniqueComments.join('\n');
74+
// Use the last declaration that has JSDoc. This handles:
75+
// - Declaration merging / module augmentation: augmentation wins
76+
// - Intersection types: last constituent wins (consistent with squashPropTypeDefinitions)
77+
// - Interface extends / override: override wins (single declaration)
78+
return declarationsWithComments[declarationsWithComments.length - 1].comment;
8379
}
8480
}
8581

@@ -428,18 +424,16 @@ function checkSymbol({
428424
symbol,
429425
location,
430426
typeStack,
431-
parentType,
432427
}: {
433428
project: PropTypesProject;
434429
symbol: ts.Symbol;
435430
location: ts.Node;
436431
typeStack: readonly number[];
437-
parentType?: ts.Type;
438432
}): PropTypeDefinition {
439433
const declarations = symbol.getDeclarations();
440434
const declaration = declarations && declarations[0];
441435
const symbolFilenames = getSymbolFileNames(symbol);
442-
const jsDoc = getSymbolDocumentation({ symbol, project, parentType });
436+
const jsDoc = getSymbolDocumentation({ symbol, project });
443437

444438
// TypeChecker keeps the name for
445439
// { a: React.ElementType, b: React.ReactElement | boolean }
@@ -619,7 +613,6 @@ function generatePropTypesFromNode(
619613
project: params.project,
620614
location: parsedComponent.location,
621615
typeStack: [(componentType as any).id],
622-
parentType: componentType,
623616
}),
624617
);
625618

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
import * as React from 'react';
2+
import type { ComponentProps } from './types';
3+
4+
export default function Component(props: ComponentProps) {
5+
const { name, onItemClick } = props;
6+
7+
return <button onClick={(e) => onItemClick?.(e.nativeEvent)}>{name}</button>;
8+
}
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
import * as React from 'react';
2+
import PropTypes from 'prop-types';
3+
function Component(props) {
4+
const { name, onItemClick } = props;
5+
return <button onClick={(e) => onItemClick?.(e.nativeEvent)}>{name}</button>;
6+
}
7+
8+
Component.propTypes = {
9+
/**
10+
* A normal prop.
11+
*/
12+
name: PropTypes.string.isRequired,
13+
/**
14+
* Augmented description of the callback.
15+
* @param {MouseEvent | React.MouseEvent} event The event source (augmented).
16+
*/
17+
onItemClick: PropTypes.func,
18+
};
19+
20+
export default Component;
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
export interface ComponentProps {
2+
/**
3+
* Original description of the callback.
4+
* @param {MouseEvent} event The event source.
5+
*/
6+
onItemClick?(event: MouseEvent): void;
7+
/**
8+
* A normal prop.
9+
*/
10+
name: string;
11+
}
12+
13+
// Module augmentation / declaration merging
14+
export interface ComponentProps {
15+
/**
16+
* Augmented description of the callback.
17+
* @param {MouseEvent | React.MouseEvent} event The event source (augmented).
18+
*/
19+
onItemClick?(event: MouseEvent | React.MouseEvent): void;
20+
}
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
import * as React from 'react';
2+
3+
interface BaseProps {
4+
/**
5+
* The label from base.
6+
* @default 'base'
7+
*/
8+
label: string;
9+
/**
10+
* If true, the component is disabled.
11+
*/
12+
disabled?: boolean;
13+
}
14+
15+
interface ExtraProps {
16+
/**
17+
* The label from extra.
18+
* @default 'extra'
19+
*/
20+
label: string;
21+
/**
22+
* The size of the component.
23+
*/
24+
size?: 'small' | 'medium' | 'large';
25+
}
26+
27+
interface CombinedProps extends BaseProps, ExtraProps {}
28+
29+
export default function Component(props: CombinedProps) {
30+
const { label, disabled, size } = props;
31+
return (
32+
<button disabled={disabled} data-size={size}>
33+
{label}
34+
</button>
35+
);
36+
}
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
import * as React from 'react';
2+
import PropTypes from 'prop-types';
3+
function Component(props) {
4+
const { label, disabled, size } = props;
5+
return (
6+
<button disabled={disabled} data-size={size}>
7+
{label}
8+
</button>
9+
);
10+
}
11+
12+
Component.propTypes = {
13+
/**
14+
* If true, the component is disabled.
15+
*/
16+
disabled: PropTypes.bool,
17+
/**
18+
* The label from base.
19+
* @default 'base'
20+
*/
21+
label: PropTypes.string.isRequired,
22+
/**
23+
* The size of the component.
24+
*/
25+
size: PropTypes.oneOf(['large', 'medium', 'small']),
26+
};
27+
28+
export default Component;
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
import * as React from 'react';
2+
3+
interface BaseProps {
4+
/**
5+
* The label from base.
6+
* @default 'base'
7+
*/
8+
label: string;
9+
/**
10+
* If true, the component is disabled.
11+
*/
12+
disabled?: boolean;
13+
}
14+
15+
interface OverrideProps extends BaseProps {
16+
/**
17+
* The overridden label description.
18+
* @default 'override'
19+
*/
20+
label: string;
21+
}
22+
23+
export default function Component(props: OverrideProps) {
24+
const { label, disabled } = props;
25+
return <button disabled={disabled}>{label}</button>;
26+
}
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
import * as React from 'react';
2+
import PropTypes from 'prop-types';
3+
function Component(props) {
4+
const { label, disabled } = props;
5+
return <button disabled={disabled}>{label}</button>;
6+
}
7+
8+
Component.propTypes = {
9+
/**
10+
* If true, the component is disabled.
11+
*/
12+
disabled: PropTypes.bool,
13+
/**
14+
* The overridden label description.
15+
* @default 'override'
16+
*/
17+
label: PropTypes.string.isRequired,
18+
};
19+
20+
export default Component;
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
import * as React from 'react';
2+
3+
type BaseProps = {
4+
/**
5+
* The label of the component.
6+
* @default 'base'
7+
*/
8+
label: string;
9+
/**
10+
* If true, the component is disabled.
11+
*/
12+
disabled?: boolean;
13+
};
14+
15+
type ExtraProps = {
16+
/**
17+
* The label from extra props.
18+
* @default 'extra'
19+
*/
20+
label: string;
21+
/**
22+
* The size of the component.
23+
*/
24+
size?: 'small' | 'medium' | 'large';
25+
};
26+
27+
type CombinedProps = BaseProps & ExtraProps;
28+
29+
export default function Component(props: CombinedProps) {
30+
const { label, disabled, size } = props;
31+
return (
32+
<button disabled={disabled} data-size={size}>
33+
{label}
34+
</button>
35+
);
36+
}
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
import * as React from 'react';
2+
import PropTypes from 'prop-types';
3+
function Component(props) {
4+
const { label, disabled, size } = props;
5+
return (
6+
<button disabled={disabled} data-size={size}>
7+
{label}
8+
</button>
9+
);
10+
}
11+
12+
Component.propTypes = {
13+
/**
14+
* If true, the component is disabled.
15+
*/
16+
disabled: PropTypes.bool,
17+
/**
18+
* The label from extra props.
19+
* @default 'extra'
20+
*/
21+
label: PropTypes.string.isRequired,
22+
/**
23+
* The size of the component.
24+
*/
25+
size: PropTypes.oneOf(['large', 'medium', 'small']),
26+
};
27+
28+
export default Component;

0 commit comments

Comments
 (0)