[docs-infra] Fix duplicate JSDoc in proptypes generation for merged declarations#48296
Conversation
06ecde2 to
1801414
Compare
…eclarations 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
1801414 to
4b24028
Compare
Bundle size
Deploy previewhttps://deploy-preview-48296--material-ui.netlify.app/ Check out the code infra dashboard for more information about this PR. |
|
|
||
| const decl = symbol.getDeclarations(); | ||
| if (decl && decl.length > 0) { | ||
| // This behavior tries to replicate how TypeScript itself merges JSDoc comments |
There was a problem hiding this comment.
Is this comment not valid anymore?
| /** | ||
| * Augmented description of the callback. | ||
| * @param {MouseEvent | React.MouseEvent} event The event source (augmented). | ||
| */ | ||
| onItemClick: PropTypes.func, |
There was a problem hiding this comment.
I suppose this is "somewhat correct"?
Like, the real ts implementation resolves to different overloads (because it is an interface with functions), for natives objects it still merges the docs.
I've added more examples to the ts playground
But since proptypes will likely only care for the most complete, I guess in this case it makes sense to pick last?
There was a problem hiding this comment.
Most complete will need further refinement. 1st definition could also technically be more complete. But in this case (and in the relevant example in Charts), there is an augmented type that is more complete that comes later in the definition. So I went with that way of just picking the last. Devs would have to make sure that later definitions are more complete.
There was a problem hiding this comment.
We can refine this further as need arises. But this change doesnt affect anything in core and in X only 3-4 files are affected.
The
getSymbolDocumentationfunction was introduced in #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:
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:
The change doesn't affect anythin on this repo but does create some changes in X when run where the above 4 cases are encountered.