[api-extractor] Add support for type alias types and type parameters

[rbuckton]

Leaving this as a draft PR for now as I'm just curious what you think about this approach. This adds support for TypeParameterDeclaration lists and the type for a TypeAliasDeclaration, which currently aren't included in the api-extractor output. This does not add support in api-documenter yet, though type parameters are supported by docfx and Type Aliases could also be supported in docfx via a custom theme.

[octogonz]

Leaving this as a draft PR for now as I'm just curious what you think about this approach.

Looks awesome! :-) Thanks for implementing this!

[rbuckton]

I found a few small issues I need to address before this might possibly be merged.

[rbuckton]

It should be fixed now.

[rbuckton]

I'm not certain if TypeParameters should be treated like Parameters (not themselves an ApiItem), or like a type declaration (which is an ApiItem). I wrote them as an ApiItem, in the event you need to perform a lookup when emitting yaml for DocFX, but I'm realizing that DocFX doesn't treat them as types.

[octogonz]

I'm pretty sure that @param and @typeParam are the same kind of thing, and should be presented the same way in the docs. TSDoc declaration references cannot refer to them, so currently it doesn't seem to make sense to model them as ApiItems. I'm open minded about this, though. :-)

[rbuckton]

I'll rework them back to be handled more like parameters are this weekend.

[octogonz]

Consider the following class:

class GenericNumber<T> {
  zeroValue: T; 
  add: (x: T, y: T) => T;
}

In the website docs, can you think of any cases where we would ever want T to be a hyperlink? Or will its meaning always be pretty obvious from the context (as I am assuming)?

If it was a hyperlink, should it simply take you to the page for the class declaration that introduced it? Would it be important to have an anchor to the table row for that particular type parameter? My feeling is that for a well-defined API nobody would really click these hyperlinks, they would just click on the class node itself because they know that's where the docs will be. But maybe there's some case I'm not thinking of.

[rbuckton]

I'm more concerned about someone using a nontrivial name for a type parameter that also exists as a type to which lookups would be incorrectly pointed.

[octogonz]

I'm more concerned about someone using a nontrivial name for a type parameter that also exists as a type to which lookups would be incorrectly pointed.

I think you're referring to _linkToUidIfPossible() in API Documenter, that creates hyperlinks by matching symbol names. This was a temporary workaround. The hyperlinking is intended to be based on ExcerptTokenKind.Reference which is an .api.json field that will be calculated using the compiler's symbols, and be serialized as an unambiguous TSDoc declaration reference. This is a somewhat important feature that unfortunately didn't make the cut for API Extractor 7. There's a community PR #1005 that was working on implementing it. We really should help get it solved.

That would eliminate any possibility of a type parameter being misinterpreted as a type, as far as I understand what you're saying.

[rbuckton]

I was thinking of rewriting _linkToUidIfPossible() too. I have a custom version I'm using that works well. It basically generates a yaml reference, mapping the excerpt tokens to a "spec.typeScript" array.

[rbuckton]

There's a community PR #1005 that was working on implementing it. We really should help get it solved.

I can put together a Draft PR for my approach in the [kAdaptType] method which pretty much does what you're looking for. You can see an example of how it works here (notice the parameter type is Iterable<Cancelable>, with Cancelable properly linked). It uses the "spec.languageName" feature in DocFX, which is documented here.

[rbuckton]

I created #1326 which includes an implementation of deeply linked types that should work with DocFX.

[rbuckton]

I noticed that the compiler accepts interface IIdentifier<> { } as a valid interface declaration. Does this form have any special meaning?

No special meaning, it probably should be an error. function f<>() {} is an error.

[octogonz]

Cool, somehow we forgot to export this

[octogonz]

I've changed this to minor. We're aiming to keep the versions the same between the packages.

(@iclanton maybe the best way would be to enable Rush lockstep versioning for api-extractor, api-extractor-model, and api-documenter.)

[octogonz]

@rbuckton Did you consider updating MarkdownDocumenter? If you don't have time, I'm okay with merging this as-is and someone else from the community will likely come along and finish that piece.

[rbuckton]

I didn't look at MarkdownEmitter at all, but could consider it in a separate PR later if I have the time.

[octogonz]

This does not add support in api-documenter yet, though type parameters are supported by docfx and Type Aliases could also be supported in docfx via a custom theme.

Just curious, why did you introduce aliasTypeTokenRange if you aren't using it? (The Markdown generator could render it, although arguably that information is already displayed in the type signature.)

[octogonz]

I find the naming ...TypeAlias.aliasType... to be somewhat uninformative. It seems to muddy the waters of whether the "alias" is the thing to the left or the right of the =.

Could we rename this to originalTypeExcerpt? Seems more straightforward to distinguish type TheAlias = TheOriginalType.

[rbuckton]

That doesn't align with the fact that type aliases can have type arguments, since it's not an "original" type, per se. If anything I would just rename it to typeExcerpt (dropping the "alias" bit).

[rbuckton]

It also mostly aligns with the fact that an ApiVariable as a variableTypeExcerpt. If I used that naming scheme it would have been TypeAlias.typeAliasType. Of the three (aliasTypeExcerpt, typeExcerpt or typeAliasTypeExcerpt), which would you prefer?

[octogonz]

But might we later introduce an excerpt corresponding to the Boxed<T> part (to the left of the =)? That thing is also a "type", at least to a layperson. It would be nice to have a more specific jargon. I can't think of anything very good, though. (alias "bodyExcerpt", alias "definitionExcerpt", ...)

If you can't think of anything better, type is better than aliasType.

[rbuckton]

An ApiTypeAlias is also an ApiTypeParameterListMixin, so for type Boxed<T> = { box: T; };, the <T> will be found in ApiTypeAlias#typeParameters and the { box: T; } will be found in ApiTypeAlias#typeExcerpt (or whatever its called). I think the whole of the declaration is covered.

[octogonz]

Of the three (aliasTypeExcerpt, typeExcerpt or typeAliasTypeExcerpt), which would you prefer?

Definitely typeExcerpt of those three. Throwing in more copies of the words "alias" and/or "type" is definitely not clarifying anything heheh.

For type A = B; I'm somewhat comfortable telling people that "A is the alias, and B is the type."

[rbuckton]

Just curious, why did you introduce aliasTypeTokenRange if you aren't using it? (The Markdown generator could render it, although arguably that information is already displayed in the type signature.)

I plan to use it in a later PR, but there's nothing stopping other tooling beyond api-documenter from using the data from api-extractor.

The main reason its not used is that DocFX doesn't understand type aliases without a custom template. I'm using a custom template for https://esfx.js.org/esfx that can understand them, however, and plan to put up a PR in due time that enables their emit in YamlDocumenter for anyone who wants to customize their DocFX template for them.

[octogonz]

BTW I found it interesting that the ; is part of the excerpt.

[rbuckton]

I find it interesting that a number of excerpts include an empty " " token at the end.

[rbuckton]

Just curious, why did you introduce aliasTypeTokenRange if you aren't using it? (The Markdown generator could render it, although arguably that information is already displayed in the type signature.)

I plan to use it in a later PR, but there's nothing stopping other tooling beyond api-documenter from using the data from api-extractor.

Actually, I already had this work in a stash. I can just add it to this PR.

[octogonz]

@rbuckton I finished reviewing your PR. Overall it looks good. There's one remaining thing I want to investigate myself (whether we can easily use metadata.schemaVersion to handle the backwards compatibility logic in ApiTypeAlias.onDeserializeInto()).

But otherwise let me know when you're done with any further changes you want to make, and then we can merge it. Thanks again for implementing this feature -- it's very cool!

[rbuckton]

I should have an update with the name change for aliasTypeExcerpt et al, plus the yaml emit for typealias and variable in a few minutes (which are currently ignored by the default DocFX template anyways).

[rbuckton]

On second thought, I'll postpone the typealias/variable YAML emit for a later PR and will just push up the name change in a moment.

[rbuckton]

Should be good to go once the build finishes.

[octogonz]

There's one remaining thing I want to investigate myself (whether we can easily use metadata.schemaVersion to handle the backwards compatibility logic in ApiTypeAlias.onDeserializeInto()).

I'll tackle this as a separate PR. It needs to add a context parameter to every deserialization function, and there are a bunch of them.

[rbuckton]

There's one remaining thing I want to investigate myself (whether we can easily use metadata.schemaVersion to handle the backwards compatibility logic in ApiTypeAlias.onDeserializeInto()).
I'll tackle this as a separate PR. It needs to add a context parameter to every deserialization function, and there are a bunch of them.

Either that, or handle the schema translation in ApiPackage.loadFromJsonFile. In the past I've done something similar where I maintained outdated versions of a json schema alongside the latest version. I used the schemas both to validate the input and to detect the schema version, then applied a json to json transformation to upgrade from version to version.

[octogonz]

Either that, or handle the schema translation in ApiPackage.loadFromJsonFile. In the past I've done something similar where I maintained outdated versions of a json schema alongside the latest version. I used the schemas both to validate the input and to detect the schema version, then applied a json to json transformation to upgrade from version to version.

It's tempting to simply report an error if the schema version is different, i.e. provide no backwards compatibility at all, and see if anyone complains about it.

If we allow loading of old .api.json files, the rendered website is going to be wrong (e.g. the type parameters will be simply missing for certain packages, but displayed for others). This could cause weird breaks for a custom tool someone wrote that that somehow depends on the new feature. It could possibly be a more troublesome experience for users than forcing them to upgrade and rebuild. Better to discuss it as a separate PR, though.