[api-extractor] Support docs/trimming for members of an interface-like "type" alias

[bdwain]

Summary

export interface MyInterface {
  /** this is supported */
  str: string;
}

export type MyType = {
  /** this is not supported */
  str: string;
};

Can support for type aliases be added? They are basically equivalent, but only interface fields can be documented. https://www.typescriptlang.org/docs/handbook/2/everyday-types.html#differences-between-type-aliases-and-interfaces

Details

looking at the generated AST, it looks like interfaces have a members array of PropertySignatures, each with a docComment. When the equivalent is done with type aliases, it only has an array of exceptTokens

Standard questions

Please answer these questions to help us investigate your issue more quickly:

Question	Answer
@microsoft/api-extractor version?	7.18.17
Operating system?	Mac
API Extractor scenario?	docs
Would you consider contributing a PR?	yes, if i can figure out how (may need assistance)
TypeScript compiler version?	4.4.4
Node.js version (node -v)?	14.16.0

[petejodo]

Didn't want to comment on the pull request but I'm interested in this feature as well. The documentation being generated for react components using type for defining props is quite lackluster

[octogonz]

They are basically equivalent

The TypeScript compiler owners view language features in terms of type algebra, in other words compiler syntax and semantics. For example, when comparing these alternatives, their docs will focus on technical details such as inheritance or assignability or this behavior:

function doSomething(x: number): string;
const doSomething = (x: number) => string;

interface IApple { title: string }
type IApple = { title: string };

API documentation has a somewhat different perspective, based around how humans discuss code and the different engineering cultures behind that. For the large corporate code bases that inspired API Extractor, historically we have tended to prefer standard coding stereotypes over flexible free-form expressions. For example, "a function" is a friendlier concept than "a constant variable whose data type is a lambda." And "an interface" is a friendlier concept than "a type signature describing a non-nested object without any alternatives." Building your API from standardized bricks makes it easier for beginners to learn, easier to organize the docs, and easier to talk about when providing support.

This is why API Extractor's generated documentation groups API pages into classes, enums, interfaces, etc. Declarations that don't fit into one of these stereotypical forms are allowed, but they get rendered as an unstructured blob of type algebra. However the TypeScript language has grown a lot over the years, so it makes sense to broaden the set of recognized patterns. (My point is that we should do that by introducing more stereotypes, not dispensing with them entirely; I don't know how to make a friendly documentation index for totally arbitrary type algebra.🙃 )

Can support for type aliases be added?

In your example, the type alias has the same shape as an interface. If there are technical reasons why type is preferable, then I agree that it's reasonable for API Extractor to handle that similarly.

But keep in mind that type has a very flexible structure. Here's a few examples:

export type Example1 = {
  /** Docs for string A */
  a: string;
} | {
  /** Docs for number A */
  a: number;
};

export type Example2 = {
  /** Docs for A */
  a: string;
} & (
  {
    /** Docs for B */
    b: string;
  }
  |
  {
    /** Docs for C */
    c: string;
  }
);

export type Example3 = {
  /** Docs for A */
  a: string;
} | true | undefined;

/** @param a - Docs for A */
export type Example4 = (a: string) => {
  /** Docs for B */
  b: string;
};

These expressions deviate from your original example in different ways. A developer might consider them to be small incremental changes, but at some point API Extractor will suddenly stop modeling them correctly and the member documentation will disappear from the website.

In order to accept your PR #3002, we need to solve three design problems:

1. Decide on a reasonable constraint for which patterns will be supported for member documentation. Whatever the constraint is, it needs to be simple and intuitive for developers to learn. (Maybe we could conservatively ONLY accept your original form?)
2. Decide how the website should present supported patterns versus unsupported patterns.
3. Add test cases to show that your implementation correctly handles the supported and unsupported patterns. I didn't have time to try Example1 with your branch, but my guess is that it won't be handled correctly.

I don't expect this to be a lot of coding, just considering a bigger set of input cases and how they should be handled.

[bdwain]

Thanks @octogonz. Those are good points about why type aliases are more difficult and we need to be careful.

For the record, I think my current solution handles examples 2-4 reasonably well (I don't know how I would display it any differently) but it definitely should change for example 1.

The tl;dr is that since this just adds property tables to what was otherwise there already, so as long as those property tables are correct, I don't think there's much downside to adding them. The only case I see where the property table is incorrect is in example 1, and that's because there are multiple property definitions for the same property name. Since they conflict, it just takes the first one. For the others though, you can see the definition the same as without my change, you just get more readable per-member documentation for the members as you see them in the code snippet.

So i guess to answer your questions.

1. I'm not sure of a great way to display conflicting property definitions, so if it's practical, I think that would be a good constraint for what is supported. If there are conflicts, no property table is generated. Otherwise, it is.
2. Supported patterns should display the property table, while unsupported should not (which is the current behavior. It's just that all type aliases are unsupported).
3. I can do this if you agree with 1 and 2.

thoughts?

[petejodo]

We use types for react components and in general for the more functional aspects of our codebase and interfaces for the more object-oriented parts. From my understanding that's the useful divide for when to use one over the other. We chose to use api-extractor because it isn't coupled to a specific framework/library like react-docgen. Not sure if this is any help. The documentation we're generating right now with api-extractor isn't great because the codebase is mostly functional. I'm going to look closely at the documentation and come up with some, hopefully, constructive criticism that may be helpful for informing a design on this stuff

[bdwain]

Hi @octogonz any thought on my comment above?

[octogonz]

So i guess to answer your questions.

1. I'm not sure of a great way to display conflicting property definitions, so if it's practical, I think that would be a good constraint for what is supported. If there are conflicts, no property table is generated. Otherwise, it is.

2. Supported patterns should display the property table, while unsupported should not (which is the current behavior. It's just that all type aliases are unsupported).

@bdwain 🤔 Generally we've tried to avoid this sort of heuristic. For example, certain API Documenter targets will create a separate web page with its own URL for each interface property.

Suppose I had a declaration like this:

/** `http://example.com/api/widgets.t` */
type T = { 
  /** `http://example.com/api/widgets.t.a` */
  a: string;
  /** `http://example.com/api/widgets.t.b` */
  b: string;
} | {
  /** `http://example.com/api/widgets.t.c` */
  c: string;
}

...and then someone comes along and adds one more property:

/** `http://example.com/api/widgets.t` */
type T = { 
  a: string;
  b: string;
} | {
  b: number; // <-- oops
  c: string;
}

It would be counterintuitive that such a "small incremental change to what was already there" might suddenly cause 3 web pages to silently disappear from the internet. And all those /** */ doc comments are no longer appearing on our website.

I bet we could solve this problem, for example by inventing separate property nomenclature like T.0.b vs T.1.b. However in the interests of unblocking people more quickly, maybe we should first do a PR that solves the restricted case of type T = { }, and then come back and tackle the | /& expressions as a second PR?

[octogonz]

https://github.com/microsoft/rushstack/issues/4699 requests something similar for enum-like types.

A design proposal for both features is written up here: https://github.com/microsoft/rushstack/issues/4699#issuecomment-2108802652