[api-extractor] Type members #3289
Conversation
chadhietala
requested review from
iclanton,
octogonz,
apostolisms,
sachinjoseph and
D4N14L
as code owners
|
@octogonz any chance you can review this? |
|
Note: PR #3337 has renamed our GitHub |
chadhietala
force-pushed
the
type-members
branch
from
abfc14c to
bab6501
Compare
There was a problem hiding this comment.
A few more additional comments:
- I think we'll need to bump the
.api.jsonversion inDeserializerContext.ts. - Can we update the comment at
to reflect that
ApiPropertySignatureitems are no longer unique toApiInterfacecontainers? I think there are a few additional API items that need to have comments updated as well (e.g.ApiMethodSignature,ApiConstructSignature, etc).
| @@ -256,7 +257,8 @@ export class MarkdownDocumenter { | |||
| this._writeEnumTables(output, apiItem as ApiEnum); | |||
| break; | |||
| case ApiItemKind.Interface: | |||
| this._writeInterfaceTables(output, apiItem as ApiInterface); | |||
| case ApiItemKind.TypeAlias: | |||
| this._writeItemContainerTables(output, apiItem as ApiItemContainerMixin); | |||
There was a problem hiding this comment.
I had some initial confusion here because this method is called writeItemContainerTables, and yet only ApiInterface and ApiTypeAlias items use it (not other item containers like ApiClass or ApiEnum). Maybe we could rename this method to writeInterfaceOrTypeAliasTypes to align with the writePackageOrNamespaceTables below?
| */ | ||
| private _writeInterfaceTables(output: DocSection, apiClass: ApiInterface): void { | ||
| private _writeItemContainerTables(output: DocSection, apiClass: ApiItemContainerMixin): void { |
There was a problem hiding this comment.
Update the second parameter to be apiInterfaceOrTypeAlias: ApiInterface|ApiTypeAlias? (I know that the parameter name being incorrect is preexisting).
| @@ -934,6 +934,13 @@ export class ApiModelGenerator { | |||
|
|
|||
| parentApiItem.addMember(apiTypeAlias); | |||
| } | |||
|
|
|||
| if ( | |||
There was a problem hiding this comment.
I'm not 100% confident about this, but don't we only want to process child declarations if apiTypeAlias === undefined? (i.e. move this chunk into the if block above)
|
|
||
| if ( | ||
| ts.isTypeAliasDeclaration(astDeclaration.declaration) && | ||
| ts.isTypeLiteralNode(astDeclaration.declaration.type) |
There was a problem hiding this comment.
It's not clear to me from line 940 which type aliases will have their children processed and which will not. I think a comment providing some supported vs not supported examples could go a long way here.
| /** | ||
| * @public | ||
| */ | ||
| export type MyUnionType = |
There was a problem hiding this comment.
This type alias will not have its members documented, correct? If that's the case, can we say so in its doc comment to make it clear to readers what this type alias is testing?
| @@ -0,0 +1,9 @@ | |||
| /** | |||
There was a problem hiding this comment.
I think the api-extractor-scenarios/apiItemKinds test project could be a better fit for this test case. In the past, I've used the api-extractor-scenarios project to hold different test cases that don't require a standalone project set up. We could create a new file called typeAliases.ts there, and add some supported vs unsupported type alias examples. I think the api-extractor-test-01 test project tests something very specific (despite its ambiguous name). WDYT?
| { | ||
| "packageName": "@microsoft/api-extractor-model", | ||
| "comment": "Support documenting members of type aliases", | ||
| "type": "patch" |
There was a problem hiding this comment.
Doesn't this add new functionality and thus this should be minor (same for the other two changelog files)? Can we also clarify in the change comment that this only supports documenting members of a very simple shape of type alias?
Summary
This supersedes #3003 but only does so for simple type aliases (no type algebra)
Details
See #3002
How it was tested
I extended the example projects to confirm that only simple type aliases get expanded out