Skip to content

Commit 4b24028

Browse files
committed
[code-infra] Fix duplicate JSDoc in proptypes generation for merged declarations
The `getSymbolDocumentation` function was introduced in mui#47685 to handle JSDoc merging across multiple TypeScript declarations. However, it unconditionally joined all unique JSDoc comments when a symbol had multiple declarations, which produced duplicated descriptions in the generated propTypes for interface declaration merging and module augmentation scenarios. TypeScript treats JSDoc differently depending on how declarations combine: - Intersection types (type C = A & B): all constituent JSDoc are merged - Interface extends (interface Z extends X, Y): first parent's JSDoc wins - Interface override (interface W extends X { prop }): the override's JSDoc wins - Declaration merging / module augmentation: last declaration's JSDoc wins The previous implementation treated all multi-declaration cases as intersections, concatenating distinct comments. This caused issues in downstream repos (e.g. mui-x) where module augmentation is used to extend interface props — the original and augmented JSDoc would both appear in the generated propTypes output. The fix now uses `parentType.isIntersection()` to distinguish intersection types (where merging is correct) from declaration merging (where the last declaration should win), matching TypeScript's own behavior. Added test cases for all four JSDoc resolution scenarios: - jsdoc-intersection: verifies both descriptions are merged - jsdoc-interface-extends: verifies first parent's description wins - jsdoc-interface-override: verifies override description wins - declaration-merging: verifies last (augmented) description wins
1 parent 9763c96 commit 4b24028

10 files changed

Lines changed: 243 additions & 27 deletions

File tree

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

Lines changed: 19 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ function getSymbolFileNames(symbol: ts.Symbol): Set<string> {
3131
function getSymbolDocumentation({
3232
symbol,
3333
project,
34+
parentType,
3435
}: {
3536
symbol: ts.Symbol | undefined;
3637
project: TypeScriptProject;
@@ -42,23 +43,9 @@ function getSymbolDocumentation({
4243

4344
const decl = symbol.getDeclarations();
4445
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
47-
// There is an open issue for it in: https://github.com/microsoft/TypeScript/issues/30901
48-
//
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)
54-
//
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
61-
const declarationsWithComments = decl
46+
// Replicates how TypeScript merges JSDoc comments across declarations.
47+
// See https://github.com/microsoft/TypeScript/issues/30901
48+
const commentedDeclarations = decl
6249
.map((d) => {
6350
const jsDocNodes = ts.getJSDocCommentsAndTags(d).filter((node) => ts.isJSDoc(node));
6451
const comment =
@@ -67,19 +54,24 @@ function getSymbolDocumentation({
6754
: undefined;
6855
return { declaration: d, comment };
6956
})
70-
.filter((item) => item.comment !== undefined);
57+
.filter(
58+
(item): item is { declaration: ts.Declaration; comment: string } =>
59+
item.comment !== undefined,
60+
);
61+
62+
if (commentedDeclarations.length > 0) {
63+
if (commentedDeclarations.length === 1) {
64+
return commentedDeclarations[0].comment;
65+
}
7166

72-
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;
67+
// Intersection types: merge unique JSDoc (matches TS hover behavior)
68+
if (parentType && parentType.isIntersection()) {
69+
const uniqueComments = [...new Set(commentedDeclarations.map((d) => d.comment))];
70+
return uniqueComments.join('\n');
7771
}
7872

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');
73+
// Declaration merging / module augmentation: last declaration wins
74+
return commentedDeclarations[commentedDeclarations.length - 1].comment;
8375
}
8476
}
8577

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: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
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 of the component.
19+
* @default 'base'
20+
* The label from extra props.
21+
* @default 'extra'
22+
*/
23+
label: PropTypes.string.isRequired,
24+
/**
25+
* The size of the component.
26+
*/
27+
size: PropTypes.oneOf(['large', 'medium', 'small']),
28+
};
29+
30+
export default Component;

0 commit comments

Comments
 (0)