[api-extractor] Option to include un-exported types in doc model output
Summary
I'm using these tools to document a TypeScript library, and I noticed that when you don't export a type from your index.ts, that type does not get included in API Documenter's output, and it won't be hyperlinked in any documentation.
It apparently doesn't get included in the "doc model" file from running api-extractor run.
Is there any way to change this behavior so that types may be documented without being exported? As it stands, I'm considering introducing a preprocessing step to replace ^type with export type temporarily in the TypeScript files themselves.
I realize that it may be the philosophy of this library that types should be exported if they're going to be documented, but I would rather not export these types because I don't want them to be part of the contract for my library. Nonetheless it would be useful to have them hyperlinked and documented.
Repro steps
Create e.g.
index.ts:
export type Foo = {
numberField: number
bar: Bar
}
type Bar = {
stringField: string
}
Expected result:
User is able to navigate to "Bar" in the API documenter output so that they may see its definition.
Actual result:
User cannot navigate to "Bar" and no documentation for it is generated.
Details
Standard questions
Please answer these questions to help us investigate your issue more quickly:
| Question | Answer |
|---|---|
@microsoft/api-extractor version? |
7.28.2 |
| Operating system? | Mac |
| API Extractor scenario? | docs (.api.json) |
| Would you consider contributing a PR? | Yes |
| TypeScript compiler version? | 4.2.4 |
Node.js version (node -v)? |
v12.22.7 |
The example you provided seems like a pretty straightforward case of API Extractor's ae-forgotten-export warning. You mention:
I realize that it may be the philosophy of this library that types should be exported if they're going to be documented, but I would rather not export these types because I don't want them to be part of the contract for my library.
But Bar is part of your API contract per the exported Foo type. If a user has something like this in their code:
const foo: Foo = {
numberField: 1,
bar,
};
const bar = {
stringField: 'Hello',
};
If you change the shape of Bar, this might break the user's code. So it's not clear to me how not exporting Bar prevents it from being part of your API contract. It seems like it's really just preventing them from explicitly typing bar (which also seems unfortunate and frustrating to users). Given this, is there a particular reason you're not exporting Bar?
I'd also recommend taking a look at https://github.com/microsoft/rushstack/issues/3430 - which concerns a specific scenario where we concluded that the ae-forgotten-export warning didn't make sense and also contains a lengthy discussion about the warning in general.
Thanks for the response @zelliott! I think you're missing two cases where exporting a type name can undesirably increase the contract surface area of a library.
-
Type renames. In your example, if
baris annotated asbar: Bar, that prevents us from renamingBarinternally (without changing its structure). -
Structure changes. In reality our types are fairly nested. Consider for example:
interface Response {
user: User
}
interface User {
fullName: FullName
}
interface FullName {
firstName: string
lastName: string
}
if we were to change it to:
interface Response {
user: User
}
interface User {
fullName: {
firstName: string
lastName: string
}
}
then if we had exported FullName this would be a breaking change, while if we hadn't, this would not be a breaking change.
Thanks for sharing those two cases, that helps me better understand your motivation.
Regardless of whether or not this is "good API design" (@octogonz is always well-spoken in this area), there's been a lot of previous discussion regarding "getting around" the api-forgotten-export rule (see https://github.com/microsoft/rushstack/issues/1235 for some past proposals). Today, you can disable the ae-forgotten-export warning in your api-extractor.json, but this still doesn't cause these "forgotten exports" to appear in either the API report or API doc model.
It might be helpful to hear a bit more about how you'd expect to be able to configure things. For example:
- Would you want to disable the warning on a per-project or per-declaration basis?
- Would you want forgotten exports to be included in the API report and API doc model files on a per-project or per-declaration basis?
- Would you want it so that disabling the warning for a declaration automatically causes that declaration to be included in the API report and API doc model files?
Hey Zack! To answer your questions 1/2, I am looking to disable this behavior on a per-project basis (per-declaration would work, but per-project is what would satisfy my use case).
Going back to my original rationale: here's an example of some generated documentation where QueryDatabaseParameters is exported but the Path and Body parameters types are not exported.
As a user viewing this documentation, there's no action I can take to see what e.g. QueryDatabasePathParameters is, except by going into the source code (or using IntelliSense).
Sorry if I'm repeating myself here, I'm just trying to make clear what my problem is from a use-case oriented perspective.
Thank you for hearing me out!
If we did add a project-wide configuration to disable the ae-forgotten-export warning and allow forgotten exports to be included in the API report and API doc model files, are there any instances in your project where you'd want the opposite behavior?
That is, I imagine the ae-forgotten-export warning is probably still useful for you in some scenarios, so disabling it entirely might lead to future API design bugs. Consider the following scenario:
interface SomeClassConfig {}
export class SomeClass {
constructor(config: SomeClassConfig) {}
}
In the scenario above, you might have forgotten to export SomeClassConfig. However, with ae-forgotten-export disabled for your entire project, you wouldn't be warned of this.
If we instead supported getting around the forgotten export rule on a per-declaration basis, then you could disable it for cases like the QueryDatabaseParameters type union, but keep it enabled for cases like the SomeClassConfig scenario. Or maybe you think the benefit of the ae-forgotten-export rule is small, and not worth the extra dev-cost needed to configure the warning on a per-declaration basis.
It's also useful to think through how a documentation site like that generated by api-documenter might surface documentation on QueryDatabasePathParameters and QueryDatabaseBodyParameters. Would the site embed their documentation within the documentation page for QueryDatabaseParameters? That might not be ideal, because then if you had multiple API items that referenced these types, their documentation would be duplicated across different pages. Maybe instead the documentation page for QueryDatabaseParameters would link out to dedicated documentation pages for QueryDatabasePathParameters and QueryDatabaseBodyParameters? If we go with that approach:
- How does the
QueryDatabaseParameterspage link to the documentation pages forQueryDatabasePathParametersandQueryDatabaseBodyParameters? Do we render the type signature with links? - How do we communicate on the documentation pages for
QueryDatabasePathParametersandQueryDatabaseBodyParametersthat these types are unexported?
If we did add a project-wide configuration to disable the
ae-forgotten-exportwarning and allow forgotten exports to be included in the API report and API doc model files, are there any instances in your project where you'd want the opposite behavior?
I don't believe so! Given the philosophy we've taken with our library, I don't think that warning is very useful to us.
If we instead supported getting around the forgotten export rule on a per-declaration basis, then you could disable it for cases like the
QueryDatabaseParameterstype union, but keep it enabled for cases like theSomeClassConfigscenario. Or maybe you think the benefit of theae-forgotten-exportrule is small, and not worth the extra dev-cost needed to configure the warning on a per-declaration basis.
Yep -- that last sentence.
It's also useful to think through how a documentation site like that generated by api-documenter might surface documentation on
QueryDatabasePathParametersandQueryDatabaseBodyParameters. Would the site embed their documentation within the documentation page forQueryDatabaseParameters? That might not be ideal, because then if you had multiple API items that referenced these types, their documentation would be duplicated across different pages. Maybe instead the documentation page forQueryDatabaseParameterswould link out to dedicated documentation pages forQueryDatabasePathParametersandQueryDatabaseBodyParameters? If we go with that approach:
Ideally: would link out to dedicated documentation pages, just like if it had been exported.
- How does the
QueryDatabaseParameterspage link to the documentation pages forQueryDatabasePathParametersandQueryDatabaseBodyParameters? Do we render the type signature with links?
Type signature with links, exactly. Like this:
- How do we communicate on the documentation pages for
QueryDatabasePathParametersandQueryDatabaseBodyParametersthat these types are unexported?
Get rid of "export" in the signature (highlighted section to be removed:)
I appreciate this discussion :)
One basic proposal is to add a new "includeForgottenExports": boolean setting to the api-extractor.json file. If false (default value), API Extractor works just as it does today. If true, then all forgotten exports are included in the API report and API doc model files.
In the future, we can still always add support for per-declaration ae-forgotten-export warning suppression or per-declaration control over whether something should be included in the API report and API doc model files.
Then, some work could be done in api-documenter to support some of the ideas in https://github.com/microsoft/rushstack/issues/3513#issuecomment-1191791900.
@octogonz, WDYT?
A little background about API Extractor's design philosophy:
if we were to change it to:
interface Response { user: User } interface User { fullName: { firstName: string lastName: string } }then if we had exported
FullNamethis would be a breaking change, while if we hadn't, this would not be a breaking change.
Avoiding such a "breaking change" makes life easier for the maintainer of the library, but perhaps at the expense of its consumers. A couple examples:
-
Suppose a consumer of your API wants to do something like this:
function formatFullName(fullName: FullName): string { return `${fullName.firstName} ${fullName.lastName}`; }Since
FullNameis an unexported type, it is effectively anonymous. There is no good way for the consumer to declare the type offullName. A diligent user might take the time to file an issue about this problem, but the answer is going to be "Oooh whoops, we didn't think of that. We'll export it for you. Please wait a month until the next SDK release." Most users won't even bother to report the problem. They'll just find an ugly hack and move on. Either way, not a great experience. -
The type name often conveys important information for consumers of your API. For example, compare this:
interface IDocument { recipient: { name: string | undefined; address: string; }; }...with this:
interface IDocument { recipient: IEmailAddress; }
One basic proposal is to add a new
"includeForgottenExports": booleansetting to theapi-extractor.jsonfile. If false (default value), API Extractor works just as it does today. If true, then all forgotten exports are included in the API report and API doc model files.
The setting name should probably distinguish "unexported APIs" versus "forgotten exports." If the person is intentionally NOT exporting it, then nothing was forgotten.
Whether or not an API is exported, I can imagine different combinations of answers to these questions:
-
Should this unexported API appear in the .api.md report? The @preapproved use case.
-
Should this unexported API appear in the .api.json file and API reference website? For example, IRushConfigurationJson is a big web of interfaces that are referenced by a public API but are uninteresting to API consumers. Maybe we could invent a doc tag that flags them to be excluded from the .api.json file?
If these states are individually controllable for each API using TSDoc tags, then perhaps @wcauchois's feature is a way to specify it globally for all unexported APIs. Something like excludeUnexportedApisFromApiReport=true and excludeUnexportedApisFromDocModel=true. Whereas a more careful user might prefer to specify it individually using TSDoc tags.
A couple other thoughts:
Declaration references
If unexported API items are documented, then the {@link} tag needs to be able to reference them. The old TSDoc declaration reference syntax doesn't support this; however the new syntax does. (Back in May @rbuckton did a big chunk of work to move forward with this new syntax, but I unfortunately haven't had time to review it yet. 😢)
Ambiguous IDs
Unexported APIs will encounter an instance of the "ae-ambiguous-ids" problem from #1308:
-
The .d.ts rollup is a single TypeScript module, so it needs unique names. For example, it's possible for
Thingfromsrc/internal/Thing.tsto be unexported, while at the same timeThingfromsrc/public/Thing.tsis exported. In this situation. the .d.ts rollup will renamesrc/internal/Thing.ts's declaration toThing_1 -
The API reference website needs separate names, otherwise it's highly confusing for users
-
The API reference website also needs distinct URLs, e.g.
example-api.com/pages/api/thing-1versusexample-api.com/pages/api/thing -
These names/URLs must be stable. Suppose a third duplicate was introduced
src/pathological/Thing.ts. It would be unacceptable for the existingexample-api.com/pages/api/thing-1to get shuffled toexample-api.com/pages/api/thing-2because a new API was introduced.
The setting name should probably distinguish "unexported APIs" versus "forgotten exports." If the person is intentionally NOT exporting it, then nothing was forgotten.
It sounds like you prefer something like includeUnexportedApis to includeForgottenExports. I think I still prefer the latter for two reasons:
- In your
IDocumentexample above, if someone purposefully doesn't exportIEmailAddress, you're right that this item isn't "forgotten" from their point of view. However, it is still "forgotten" from API Extractor's point of view. I'm not sure that API Extractor cares about intention when deciding whether something is a forgotten export. Thus, I don't think the nameincludeForgottenExportsis necessarily inaccurate. - I think it'll be clearer to users what
includeForgottenExportsdoes thanincludeUnexportedApis. If I sawincludeForgottenExports, I'd notice an immediate parallel to theae-forgotten-exportswarning (i.e. "Oh, this adds all of thoseae-forgotten-exportsdeclarations to my API report and API doc model!"). Whereas it wouldn't be immediately clear to me what an "unexported API" was.
If these states are individually controllable for each API using TSDoc tags, then perhaps @wcauchois's feature is a way to specify it globally for all unexported APIs. Something like
excludeUnexportedApisFromApiReport=trueandexcludeUnexportedApisFromDocModel=true. Whereas a more careful user might prefer to specify it individually using TSDoc tags.
I like this plan. I think we should first add a global setting to toggle it on a project-level, and then add per-declaration support as a follow-up.
However, is there a situation where we'd want an unexported API to be included in the API report but not the API doc model (or vice versa)? It's certainly easier to control inclusion to both files with a single setting, so I'd prefer to do that if possible.
Lastly, wouldn't we prefer include...=false to exclude...=true so that the flags can default to false?
If unexported API items are documented, then the {@link} tag needs to be able to reference them
Are you sure? I know that unexported items are prefixed with ~. I can test this on my own but I think this might already work.
Ambiguous IDs
This problem already seems solved for .d.ts rollups, as you stated (renaming items to Thing_<index>).
I agree that in a perfect world these names & URLs would be stable. I thought about this for a bit, and I couldn't come up with a way to good automatically derive a stable name from the file/item itself. What if instead we add support for a TSDoc @alias tag that allows developers to rename an item for the documentation site (e.g. https://jsdoc.app/tags-alias.html). Then onus is on the developer to come up with a unique, stable name, and API Extractor can warn or something if it encounters duplicate aliases.
I will say that this is an existing bug with api-documenter today so I'm not sure that it should block this feature. For example, notice how at https://api.rushstack.io/pages/api-extractor-model.apiinitializermixin/, merged declarations all have the same URL, and thus certain API item pages don't even appear. Thus, I think this should be potentially tackled as a follow up task.
However, is there a situation where we'd want an unexported API to be included in the API report but not the API doc model (or vice versa)?
No sure how realistic these are, but here's some ideas:
- API report but not doc model: Suppose we want to know if this API changes, because it would imply a MAJOR SemVer bump for the library. But for some reason we don't want it to appear on the website, for example maybe the interfaces describe another company's copyrighted API interface. Or maybe for some technical reason the build tools copy the interfaces from another project that already has its own documentation.
- Doc model but not API report: The documentation website is for an internal audience, who finds it convenient to include every single interface on the website. However the API review team doesn't care about some of these. (A classic example is telemetry data structures, which get endless adjustments by people who are monitoring service health, and nobody wants to review it.)
Lastly, wouldn't we prefer
include...=falsetoexclude...=trueso that the flags can default to false?
Yes. 👍 Those setting names were meant to be a rough sketch, not an actual proposal. :-)
Are you sure? I know that unexported items are prefixed with
~. I can test this on my own but I think this might already work.
The canonicalReference field in the .api.json file uses the new declaration reference syntax, which probably supports ~. The {@link} tag uses the old declaration reference syntax.
I agree that in a perfect world these names & URLs would be stable. I thought about this for a bit, and I couldn't come up with a way to good automatically derive a stable name from the file/item itself. What if instead we add support for a TSDoc
@aliastag that allows developers to rename an item for the documentation site (e.g. https://jsdoc.app/tags-alias.html). Then onus is on the developer to come up with a unique, stable name, and API Extractor can warn or something if it encounters duplicate aliases.I will say that this is an existing bug with api-documenter today so I'm not sure that it should block this feature. For example, notice how at https://api.rushstack.io/pages/api-extractor-model.apiinitializermixin/, merged declarations all have the same URL, and thus certain API item pages don't even appear. Thus, I think this should be potentially tackled as a follow up task.
We planned to solve the stability problem using the TSDoc @label tag. Some details appear in https://github.com/microsoft/rushstack/issues/881#issuecomment-461620185
I think we know how to solve most of the ae-ambiguous-ids GitHub issues. The challenge was just that it's difficult to keep straight all the different cases and their interrelationships (e.g. the URL should loosely correspond to the page title which should loosely correspond to the API name, but the forbidden characters and escaping mechanisms are different for each one). And the consequences of these bugs are somewhat rare, which did not motivate the potential work involved to fix them all.
I didn't mean to imply that #3513 should be blocked behind this problem, but it would be good to avoid digging the ae-ambiguous-ids hole deeper if possible. :-)
No sure how realistic these are, but here's some ideas
Thanks for the ideas. I'm okay adding two flags, but we should just know that this'll complicate the design of any per-declaration TSDoc inclusion/exclusion tag a bit.
I'll take a stab at opening a PR for this today.