RFC for automatically generated typescript API documentation for every plugins public services, types, and functionality

[stacey-gammon]

RFC

[rudolf]

My biggest concern is that we end up in a situation where we cannot upgrade typescript because our custom docs system breaks and we don't have the capacity to test pre-releases and fix the docs system ahead of time. Typescript performance is currently a huge bottleneck for (almost) all developers across Kibana, not being able to upgrade to a newer version that brings performance optimisations could be very expensive in terms of developer productivity. (In my opinion more expensive than not having docs, but this is hard to quantify).

So on top of the risk of relying on the ts-morph package, there is a risk that maintaining our custom docs system requires more effort than we imagine. ts-morph increases this risk because it's a more low-level and more general-purpose library than api-extractor

[stacey-gammon]

I'll note your concern in the RFC, I agree it's an important one.

The author does seem to regularly keep up with typescript upgrades, but there is still risk that he will slow down, or certain upgrades will require a large effort on our part.

In my opinion more expensive than not having docs, but this is hard to quantify

We always have the option of making this call in the future. Each time we prepare for a typescript upgrade, we'll need to decide between:

1. Support the new version in our ts-morph implementation
2. Switch to an api-extractor implementation
3. Remove our API docs

If it's easy to upgrade ts-morph, we make the changes and move on. If it's not, we assess how useful we think the docs are compared to the effort it would take to keep them. If the effort outweighs their usefulness, we can remove them, perhaps temporarily, until there is more bandwidth to fix the issues and bring back support.

If you're worried about devs starting to rely on docs that we might need to stop supporting in order to move faster with typescript, I think that would indicate a high level of importance, and mean it's worth investing the effort into keeping them around.

[rudolf]

+1 This is to workaround limitations in the default api-documenter implementation.

[pgayvallet]

Implementing our own typedef parser using AST (even if using ts-morph which is a great library) is monumental work, and, imho, we should be very cautious before deciding to do so.

This is why I think that the first section of this RFC should be an inventory of the limitations of the current api-extractor lib we are using, and the upsides that our own custom tool could provide, both on output format, and features.

I'm not sure how generics will look. I suggest an iterative approach

For development, definitely. For the initial specification, we may want to have a more precise initial vision of what we want inmho.

Generics and complex / composite types are massively used in Kibana's codebase, and I think that we should at least have a specification of how we expect to represent them in the documentation. Ideally, some examples of generated doc (not necessarily using the POC, I'm not asking for working code here, but a spec of what we want) would be useful for the RFC.

Some examples coming to mind (there must be a lot more):

composition

type Foo = Bar | Dolly;

extension

interface Bar extends Foo {
  hello: string;
}

implementation

export class Foo implements Bar

generics + extension generics

export class ObjectType<P extends Props = any> extends Type<ObjectResultType<P>> {

exposing 3rd party types

type Foo {
   component: React.ComponentType
}

Also, the RFC only speaks about plugins. What about packages? Do we plan to also generate documentation for packages, and be able to cross-link from plugins to packages or packages to packages? Not sure about plugins, but I know that core is using types coming from our packages in its public API.

[stacey-gammon]

@pgayvallet , I'll pull the ts-morph vs api-extractor to the top, make sure all the issues are listed.

For development, definitely. For the initial specification, we may want to have a more precise initial vision of what we want inmho.

Well, to be fair, we already have a rudimentary API doc system merged, and as far as I know, there is no vision written up for that, so I don't think we should block this on an exhaustive list.

Here are some examples of how the things you mentioned above appear. This is using my PR, I have tests set up so I can add in any crazy types I want and we can see how they show up. There are some issues with links rendering correctly in all situations.

Generics

Compound types

Sometimes this is accurate, sometimes it's not. See dsherret/ts-morph#923.

Working:

No links:

generics + extension generics

There is an issue with the "extends / implements" part rendering links. No worse than what we have currently, however.

Third party types

No links, but no links with current system as well. I doubt this would work out of the box with api-extractor as well. Where is it going to link to? I don't think we want to host API docs for every external package we use.

How to move forward?

The question I would like to focus on is:

Are we comfortable merging a ts-morph approach in the short term?

The answer to that does not in any way exclude exploring a more robust api-extractor implementation. It's about choosing an improved API docs implementation in the short term. We can consider the entire thing experimental, but I think there is a lot of value in having it merged.

[kobelb]

Will this potentially cause confusion with TypeScript's unknown top-type? Would a TypeScript any fall under this TypeKind also?

[stacey-gammon]

Oh good thought. yes, any falls under here.

Hmmm... I could add any and unknown as TypeKinds, and come up with a new name for "don't know what this is". "Uncategorized"?

[kobelb]

Hmmm... I could add any and unknown as TypeKinds, and come up with a new name for "don't know what this is". "Uncategorized"?

"Uncategorized" seems reasonable. We could also make TypeKind nullable/undefined. I think either is fine.

[kobelb]

IMO, we should treat "required" as the default and "optional" as the exception. It feels a lot more common for parameters, etc. to be required rather than optional.

[stacey-gammon]

++ (cc @snide @glitteringkatie )

[kobelb]

If we were to show a badge for "optional" as opposed to "required", it seems like it'd solve this issue, right?

[stacey-gammon]

Yea, I proposed that to @snide but if I remember correctly, he wanted us to use required for consistency with API docs. I believe this is something we can iterate on, not set in stone.

[kobelb]

It seems like overkill to me...

[stacey-gammon]

@elasticmachine merge upstream