[api-extractor] Better support for handling unexported base classes

[zelliott]

Summary

The purpose of this issue is to kick off a discussion for how API Extractor can provide better support for handling unexported base classes. Consider the following scenario:

class UserInputBase {
  disabled: boolean;
}

export class Button extends UserInputBase { ... }
export class Checkbox extends UserInputBase { ... }

In the case above, UserInputBase is an anonymous implementation detail of Button and Checkbox and purposefully not exported.

Today, API Extractor will generate documentation pages for Button and Checkbox but not for UserInputBase, as the latter isn't exported. Additionally, API Extractor will not generate documentation for any inherited members from UserInputBase within the documentation pages for Button and Checkbox. Lastly, it will surface an ae-forgotten-export warning for UserInputBase.

My understanding is that API Extractor treats the pattern above as "bad API design", hence the ae-forgotten-export warning and poor documentation support. While in some cases this may indeed be bad API design (e.g. if you know developers will want to write functions that operate on UserInputBase objects), I think the false positive rate is too high. There are valid reasons to not export UserInputBase. For example, the API author might want the ability to refactor the inheritance chain of these classes without making a breaking change. The author might want to rename UserInputBase, or remove it completely and move the inherited members directly into the sub-classes. These changes are harder to make if UserInputBase is exported.

For these reasons, I think we should reconsider the following for this case:

1. Whether an ae-forgotten-export warning should be surfaced.
2. The level of documentation support API Extractor provides.

Note that all of the above applies to interface inheritance as well.

Details

ae-forgotten-export warning

I think the behavior here should be determined by the false positive rate for the ae-forgotten-export warning for this case:

1. If unexported base classes never/rarely represent bad API design, then the ae-forgotten-export warning should never be surfaced.
2. If unexported base classes sometimes represent bad API design, then there should be a way to suppress warnings (see [api-extractor] Getting around (ae-forgotten-export) for certain cases #1235).
3. If unexported base classes always represent bad API design, then we should be fine with the current behavior.

In my opinion, I think 1 or 2 make the most sense.

Better documentation support

Here are a few ideas for how API Extractor could provide better documentation support for this case. I don't have a super strong preference yet, I'll need some more time to think through them.

1. Include inherited members within exported sub class's API doc model

In this approach, we continue to omit the unexported base class from the API doc model and API report, but include any inherited members within the sub class's API doc model. So for the following scenario:

class B {
  b: string;
}

export class A extends B {
  a: string;
}

The API doc model would look like this:

A extends B
- a
- b (inherited)

If B was exported, then the API doc model would look like this:

A extends B
- a

B
- b

Open questions:

• Should we omit the "extends B" from the API doc model entirely, assuming B is truly just an implementation detail of A? Or rename B to something like (Anonymous) to indicate that its name is an unimportant? Or append something like (unexported) after B?
• How do we handle more convoluted inheritances & export patterns? For example, suppose A extends B extends C extends D, but only A and C are exported. I think A needs to include all inherited members from B, C, and D (note that we need to include the inherited members from C and D because there's nothing in the doc model indicating that B extends C). Alternatively, we could simply not support this case if we think it represents bad API design.
• What if B is in another package? If B is in another package, then it must be exported (in order for it to be extended by A), and thus it should be included in its package's API doc model. We shouldn't need to include any inherited members in this case.

2. Include unexported base classes in the API doc model

In this approach, we simply include unexported base classes in the API doc model and API report. Thus, an exported class's entire inheritance chain is always included in the API doc model. So for the following scenario:

class B {
  b: string;
}

export class A extends B {
  a: string;
}

The API doc model would look like this:

A extends B
- a

B (unexported)
- b

Open questions:

• How do we indicate on the API docs site that B is unexported?

3. Add a configuration to include forgotten exports in the API doc model

This is a solution to the more general use case of wanting to include "forgotten exports" in the API doc model. This is also the approach described at #1235 (comment). We could add a new includeForgottenExports config to API Extractor that would cause all forgotten exports to be by default included in the API doc model and API report. We could also use some kind of doc comment tag to include/exclude declarations from these files at a per-declaration level.

I'm not sure whether there's enough of a need for documenting forgotten exports other than the class/interface inheritance case. The only other example that comes to mind is something like this (see #1235 (comment)):

interface PartialComponentProps {
    a: string;
    b: string;
}

export type ComponentProps = PartialComponentProps & {
    c: string;
};

But I'm receptive to API Extractor's current stance of not spending too much time/effort on perfectly documenting arbitrary type algebra.

Open questions:

• What are other cases of wanting to document forgotten exports?
• What would the include/exclude tag be named?
• How do we indicate on the API docs site that something is unexported?

[octogonz]

1. If unexported base classes never/rarely represent bad API design, then the ae-forgotten-export warning should never be surfaced.

The ae-forgotten-export warning was introduced because forgotten exports were frequently causing bad API releases, and remembering is not easy.

Consider this hypothetical example (with these declarations buried throughout a big pile of source files):

interface ILinkManagerOptions { . . . }
class LinkManager {
  public constructor(options: ILinkManagerOptions);
  . . .
}

class Link {
  public constructor(manager: LinkManager);
  public get manager(): LinkManager;
  . . .
}

/** @public */
export class Document {
  private _createLink(): Link;
}

The trouble starts when somebody decides to promote _createLink() to be a public API, without realizing that Link is not exported yet. Unit tests seem to work fine when you call Document.createLink().url, so the NPM package gets published. But then customers start complaining that usage is awkward because they can't write const link: Link = doc.createLink();. To fix this, we go back and add export to Link, but now we realize that the API review people never looked closely at this API because they thought it was not exported. 😆 And then after that gets straightened out, the same embarrassment+correction plays out for LinkManager, ILinkManagerOptions, etc as we unravel a chain of forgotten exports.

Above, I am conflating several things:

• a declaration being exported (i.e. marked as export)
• a declaration being exposed as an API contract that consumers are meant to use
• a declaration having an official name that API consumers can refer to it as
• an internal developer workflow for catching mistakes

Certainly we can consider decoupling these things.

[octogonz]

• How do we handle more convoluted inheritances & export patterns? For example, suppose A extends B extends C extends D, but only A and C are exported. I think A needs to include all inherited members from B, C, and D (note that we need to include the inherited members from C and D because there's nothing in the doc model indicating that B extends C). Alternatively, we could simply not support this case if we think it represents bad API design.

Option 1 can only work in very narrow cases. We could approach it using @partOf:

/** {@partOf B} */
class A {
  public f(): void;
}

/** @public */
export class B extends A {
  public g(): void;
}

...with a bunch of constraints, for example we probably would forbid multiple copies:

/** {@partOf B} {@partOf C} */
class A {
  public f(): void;
}

/** @public */
export class B extends A {
  public g(): void;
}

/** @public */
export class C extends A {
  public g(): void;
}

Option 1 seems like a hacky solution with limited usefulness.

[octogonz]

• What are other cases of wanting to document forgotten exports?

Enumerating these use cases is probably the best way to straighten this out. Here are some ideas to get started:

Use Case: A normal API that chooses to be anonymous

This is your original motivating example I think:

• UserInputBase is a normal API in my opinion
• Technically, the lack of exporting can be easily thwarted by doing something like type UserInputBase = Button | Checkbox, so the "anonymous" aspect is really a human concern about cleanliness of IntelliSense or API compatibility commitments
• UserInputBase has multiple subclasses, so it would be confusing to pretend that its members are part of Button and Checkbox

The most natural API docs representation would be to show UserInputBase along side the other classes, just with an attribute like (unexported).

The solution might be like this:

// Tell API Extractor that we did not forget to export UserInputBase
// -- it was an intentional design decision.
/** @unexported */
interface UserInputBase {

With the @unexported tag, API Extractor will handle this class identically as a regular class, except that it will appear in the .d.ts rollup without export.

Use Case: Bulk-ignoring a complex object

// a sprawling set of interfaces describing a big JSON object
interface IThingJson {
  title: IThingTitleJson;
  body: IThingBodyJson;
  owner: IThingOwnerJson;
  detail: IThingDetailJson;
  . . .
}

/** @public */
export class Thing {
  /** @internal */
  public constructor Thing(json: IThingJson);
}

In this example, the constructor is marked as @internal because it is only ever called by other packages that we own. As a result IThingJson should be marked as @internal, which would imply marking 30 other interfaces as @internal as well. That's a big hassle however, because our internal caller is just loading a JSON object as any and validating it with a schema -- they don't care about these internal details. We want to avoid cluttering up the public API with all these JSON interfaces.

The solution might be like this:

// Tell API Extractor that we did not forget to export IThingJson
// -- it was an intentional design decision.
// Also tell API Extractor not to generate an .api.json entry for this type.
/** @unexported @excludeFromDocs */
interface IThingJson {

And maybe @unexported and @excludeFromDocs would NOT be required for the other referenced interfaces such as IThingTitleJson. They can be ignored implicitly unless API Extractor finds references to those interfaces from other APIs that were not marked as @unexported or @excludeFromDocs.

Use Case: An uninteresting auxiliary type

interface IJsonObject {
    [key: string]: JsonSerializable;
}
/** @public */
export type JsonSerializable = IJsonObject | number | string | Array<IJsonObject | number | string>;

In this (hypothetical and not recommended😁) example, IJsonObject must be given a name because the type algebra for JsonSerializable refers to it twice. But let's say that IJsonObject is not interesting by itself, it's just an implementation detail. So we don't want to export it.

The solution might be like this:

// Tell API Extractor to lump these two snippets together into a single API item
/** {@partOf JsonSerializable} */
interface IJsonObject {

The rendered API docs would look like:

---

JsonSerializable (type)

Represents an object that can be serialized using JSON.stringify().

snippet

interface IJsonObject {
    [key: string]: JsonSerializable;
}
export type JsonSerializable = IJsonObject | number | string | Array<IJsonObject | number | string>;

Remarks

. . .

[octogonz]

🤔 Here's a possible wrinkle for the @unexported tag:

internal-file.ts

/** @unexported */
export class Pathological { }

index.ts (entry point)

import { Pathological as Pathological2 } from './internal-file';

/** @public */
export class Pathological extends Pathological2 { }

In this case, API Extractor considers the official name of internal-file/Pathological is Pathological. (We treat Pathological2 as an irrelevant alias, because there can be many such aliases with conflicting names.) The Collector will rename the unexported Pathological to something like Pathological_1 when generating a .d.ts rollup.

By this reasoning, Pathological_1 is the name that should be used in the documentation website title and page URL for the (unexported) API item. But it is not a "stable" name (a general problem that's part of the ae-ambiguous-ids scope of work) -- the _1 numbering can get shuffled when more items are included in the future. Maybe we would use @label to make a stable name.

[zelliott]

Thanks for the thorough comments! Some responses below:

Re your first comment: For the record, I definitely see the value of ae-forgotten-export and am not proposing to remove the warning by any means. I'm more questioning the value of ae-forgotten-export in the case of specifically unexported base classes. It's less clear to me how the troublesome scenario you described plays out in that case.

Option 1 can only work in very narrow cases. We could approach it using @partOf:

I don't quite follow why Option 1 only works in very narrow cases or how using @partOf helps. Even in the "convoluted" case I provided, Option 1 can still work. What are the other cases you're thinking of that lead you to conclude that it can only work in very narrow cases?

Enumerating these use cases is probably the best way to straighten this out.

I'll share some thoughts on each of the use cases you described:

Use Case: A normal API that chooses to be anonymous

Technically, the lack of exporting can be easily thwarted by doing something like type UserInputBase = Button | Checkbox, so the "anonymous" aspect is really a human concern about cleanliness of IntelliSense or API compatibility commitments

I'm not convinced this is the case. An API consumer won't know that UserInputBase = Button | Checkbox, that's an implementation detail that the library does not share with the consumer. The library only exposes Button and Checkbox.

UserInputBase has multiple subclasses, so it would be confusing to pretend that its members are part of Button and Checkbox

I'm not convinced this would be confusing if these members were flagged as "inherited".

In general, I feel like we're approaching this example from two different angles. You see the relationship between Button, Checkbox, and UserInputBase as an important part of the API that should be communicated to API consumers, with dedicated pages for all three entities. I see Button and Checkbox as the important parts of the API, and UserInputBase as simply an implementation detail that ideally doesn't even have its own API page (with instead its inherited members appearing under Button and Checkbox).

My concern with the proposed solution (i.e. @unexported tag) is that I feel like you shouldn't need to explicitly communicate to API Extractor that this is an intentional design decision, because there's nothing wrong with not exporting UserInputBase in the first place. It doesn't feel to me like a "forgotten export". But I'll continue to think about it…

Use Case: Bulk-ignoring a complex object

If the constructor is marked as @internal, API Extractor should be able to determine that IThinkJson and any interfaces used within are not forgotten exports, and thus shouldn't require the library author to tag anything with @internal. Maybe I don't quite follow this example?

Use Case: An uninteresting auxiliary type

As I mentioned in my first comment, I'm comfortable not handling any type algebra cases for now. But assuming we did want to address it, the @partOf tag feels like a partial solution. What if multiple declarations wanted to include IJsonObject in their documentation? Multiple @partOf tags feels awkward.

This feels like one of those cases where I think API Extractor might be overzealous in its ae-forgotten-export warning. Why not just not report an ae-forgotten-export warning and just automatically include IJsonObject the documentation for JsonSerializable? I'm wondering if it's more common that the API author purposefully meant to not export IJsonObject as opposed to forgot to export IJsonObject.

[octogonz]

I'm not convinced this is the case. An API consumer won't know that UserInputBase = Button | Checkbox, that's an implementation detail that the library does not share with the consumer. The library only exposes Button and Checkbox.

Won't UserInputBase be visible in the .d.ts file? In VS Code, if I right click on button.disabled and do "Go to definition", it's going to take me to the UserInputBase declaration, right? That seems fairly conspicuous to developers.

My concern with the proposed solution (i.e. @unexported tag) is that I feel like you shouldn't need to explicitly communicate to API Extractor that this is an intentional design decision, because there's nothing wrong with not exporting UserInputBase in the first place. It doesn't feel to me like a "forgotten export".

In monorepos using @rushstack/eslint-config, ae-forgotten-export would normally matter for base classes as well.

🤔 The lint ruleset requires explicit type declarations:

import { Document, RichTextBase } from 'example-lib';

// @rushstack/eslint-config requires declarations like this:
function f(document: Document): RichTextBase {
  return document.generateSummary();
}

// NOT LIKE THIS, because an unfamiliar person who is reading this
// code snippet cannot easily guess what kind of object is returned:
function f(document: Document) {
  return document.generateSummary();
}

Thus if the RichTextBase base class is a forgotten export, then the lint rule is impossible to satisfy.

Use Case: Bulk-ignoring a complex object

If the constructor is marked as @internal, API Extractor should be able to determine that IThinkJson and any interfaces used within are not forgotten exports, and thus shouldn't require the library author to tag anything with @internal. Maybe I don't quite follow this example?

I think it's the same situation actually -- regardless of whether generateSummary() is called by internal code or not, the consuming project would normally need to declare its variable types, which means it needs a way to import RichTextBase.

If your company's code base has different conventions, it's fine to introduce an api-extractor.json setting to make ae-forgotten-export less "zealous." But is the criterion really that "Forgotten exports are not flagged if the API item is a class with at least one subclass that is exported"? That seems a bit arbitrary. Maybe a significant detail is that in your example, UserInputBase is neither returned nor accepted by any API function. Perhaps this is the real reason why consumers never need to import it? We could try to convert that into an automatic rule, although requiring the developer to explicitly mark it as @unexported doesn't seem like we're asking very much of them. (?)

[zelliott]

Won't UserInputBase be visible in the .d.ts file? In VS Code, if I right click on button.disabled and do "Go to definition", it's going to take me to the UserInputBase declaration, right? That seems fairly conspicuous to developers.

You're right. Let me try to rephrase my objection to "the lack of exporting can be easily thwarted by doing something like type UserInputBase = Button | Checkbox".

If the API consumer is trying to capture the unexported class hierarchy by writing this type alias, that's not a great idea, as there's nothing preventing the library from changing under the consumer's feet and making UserInputBase != Button | Checkbox.

Suppose Checkbox is refactored to no longer extend UserInputBase, but all of the previously inherited members are copied over to Checkbox. Checkbox thus has exactly the same members and behavior as it did before. Button still extends UserInputBase. Is this a breaking API change? In my opinion, no, as no API consumer should be relying upon the fact that Checkbox extended UserInputBase. If some API consumer had written UserInputBase = Button | Checkbox and suddenly their project "breaks", that's on them.

Thus if the RichTextBase base class is a forgotten export, then the lint rule is impossible to satisfy.

It's up to the API author to decide whether they want to support consumers writing functions that accept or return RichTextBase. If they don't export RichTextBase, then they're deciding not to support that case, and that seems perfectly fine.

This doesn't apply to other instances of ae-forgotten-export. Take the following API example:

type SomeType = number|boolean; // not exported
export A {
  x: SomeType;
}
export function b(x: SomeType): SomeType { … }

If a project has explicit type declarations turned on, they can't write any of the following:

const x: SomeType = true; // can't write
const a: A = new A();
a.x = x;
const y: SomeType = b(x); // can't write

That seems extremely limiting. I think there's a difference between the RichTextBase case and the SomeType case.

But is the criterion really that "Forgotten exports are not flagged if the API item is a class with at least one subclass that is exported"? That seems a bit arbitrary. Maybe a significant detail is that in your example, UserInputBase is neither returned nor accepted by any API function.

I think this is on the right track. If an API accepts & returns values that cannot be typed, that seems like bad design (is there ever an instance where you'd want this?). What would it look like if we simply changed the ae-forgotten-export warning to enforce that criterion instead? I'm also open to a "strictness" setting in the api-extractor.json, if we think the current ae-forgotten-export behavior is still useful for certain projects.

[octogonz]

What would it look like if we simply changed the ae-forgotten-export warning to enforce that criterion instead?

In the projects that I've worked on, I think what would happen is that people would still forget, and in some cases it would still create problems. ("Why didn't ae-forgotten-export catch this case?" "Well, the rule is complicated. Oh wait, the implementation has a bug. Or does it?? Someone remind me when the rule is applied...")

I think there's a difference between the RichTextBase case and the SomeType case.

I agree. I'm just suggesting that this distinction should maybe be explicit and managed by people, rather than an implicit rule. Keep in mind that most "forgotten exports" actually have the word export in their declaration:

Control.ts

// ae-forgotten-export
export class Control { }

Button.ts

import { Control } from './Control';
export class Button extends Control { }

index.ts (entry point)

export * from './Button';

It is easy to forget to export Control, and not easy to spot this problem by looking at its definition. You'd have to look carefully at the API Report in your pull request, and notice that export is missing there.

Do you find it cumbersome to have to suppress each warning by adding an explicit TSDoc tag?

/** @unexported */
export class Control { }

[octogonz]

I don't quite follow why Option 1 only works in very narrow cases or how using @partOf helps. Even in the "convoluted" case I provided, Option 1 can still work. What are the other cases you're thinking of that lead you to conclude that it can only work in very narrow cases?

To summarize, Option 1 is proposing that:

• IF an API item is a class or interface
• AND it inherits from a base class or interface that is not exported (the scenario of @unexported)
• THEN the API documentation website will display the inherited members as part of the child API item
• with a label like (inherited from UserInputBase)
• and if there are several child items (Button, Checkbox) then the content will be duplicated

BEFORE this feature, the page URLs might be like this:

API item	URL
Button.caption	https://api.example.io/pages/my-ui-library.button.caption/
Checkbox.checked	https://api.example.io/pages/my-ui-library.checkbox.checked/
UserInputBase.disabled	https://api.example.io/pages/my-ui-library.userinputbase.disabled/

AFTER this feature, the page URLs would look like this:

API item	URL
Button.caption	https://api.example.io/pages/my-ui-library.button.caption/
Button.disabled	https://api.example.io/pages/my-ui-library.button.disabled/
Checkbox.checked	https://api.example.io/pages/my-ui-library.checkbox.checked/
Checkbox.disabled	https://api.example.io/pages/my-ui-library.checkbox.disabled/

Is this what you meant? Note that these URLs will break if we later change UserInputBase to be exported.

Note that we don't necessarily need to duplicate the entries in the .api.json file. As long as UserInputBase is marked as "unexported", then API Documenter or a similar tool could squash the hierarchy while generating the docs, similar to the plan for #3429.

[zelliott]

In the projects that I've worked on, I think what would happen is that people would still forget, and in some cases it would still create problems. ("Why didn't ae-forgotten-export catch this case?" "Well, the rule is complicated. Oh wait, the implementation has a bug. Or does it?? Someone remind me when the rule is applied...")

I think there's a difference between the RichTextBase case and the SomeType case.

I agree. I'm just suggesting that this distinction should maybe be explicit and managed by people, rather than an implicit rule. Keep in mind that most "forgotten exports" actually have the word export in their declaration:

Gotcha, this is helpful to understand. I think perhaps the decision comes down to…

1. What we think the current false positive rate of this warning is.
2. How bad the wrong behavior is.
3. How easy suppression is.

What are other factors?

For 1: If we think most of the time a developer knows what they're doing and is making a purposeful decision to not export a base class, then it seems counterproductive for them to need to explicitly suppress the ae-forgotten-export warning. Conversely, if we think that most of the time a developer is accidentally not exporting a base class and actually wants the base class to be exported, then it seems helpful to by-default flag these instances and force developers explicitly suppress them.

I'm not sure what the right answer is here. My hunch is that in most cases, if developers get this warning for base classes, they decide they don't really care whether the base class is exported or not, and just end up exporting it to suppress the warning. I suppose I tend to be cautiously optimistic and assume that developers know what they're doing, unless they express otherwise.

For 2: If the wrong behavior is very bad for an API, then we should be willing to accept a higher false positive rate to avoid the wrong behavior (and vice versa).

I don't think the wrong behavior is necessarily any worse than forgetting to export any declaration. And we don't flag every unexported declaration as a potentially "forgotten export", so the wrong behavior doesn't seem particularly bad.

For 3: If it's easy to suppress the warning, then we should be willing to accept a higher false positive rate (and vice versa).

Agreed that TSDoc tags are an easy suppression mechanism… though an api-extractor.json config setting would be even easier.

Option 1 discussion

Yes, your understanding is mostly correct. Only difference: Option 1 does not discuss any TSDoc @unexported tag. You're right that if UserInputBase is changed to be exported, these URLs will break… but only if the API Documenter "show inherited members" feature described in #3429 is not turned on.

Note that we don't necessarily need to duplicate the entries in the .api.json file. As long as UserInputBase is marked as "unexported", then API Documenter or a similar tool could squash the hierarchy while generating the docs, similar to the plan for #3429.

Yes - this is essentially I think the proposal in Option 2 (although again, Option 2 does not discuss any @unexported tag). I think Option 2 is simpler than Option 1, and as of now I don't have a strong preference between the two.

[octogonz]

You're right that if UserInputBase is changed to be exported, these URLs will break… but only if the API Documenter "show inherited members" feature described in #3429 is not turned on.

When that feature is turned on, what do the URLs look like?

[zelliott]

When that feature is turned on, what do the URLs look like?

I was thinking the following for Option 1:

• UserInputBase not exported:
  • URL for Button.disabled (inherited) is https://api.example.io/pages/my-ui-library.button.disabled/
  • No API page for UserInputBase as it's not exported. Note that in Option 2, there could still be an API page for UserInputBase.
• UserInputBase exported:
  • URL for UserInputBase.disabled is https://api.example.io/pages/my-ui-library.userinputbase.disabled/
  • With fancy docs inheritance feature turned on, URL for Button.disabled (inherited) is https://api.example.io/pages/my-ui-library.button.disabled/ (same URL as above). With feature turned off, no Button.disabled entry.

Option 1 is somewhat limiting as it means that UserInputBase cannot have its own API page if it's not exported.

[octogonz]

UserInputBase exported:

• With fancy docs inheritance feature turned on, URL for Button.disabled (inherited) is https://api.example.io/pages/my-ui-library.button.disabled/ (same URL as above). With feature turned off, no Button.disabled entry.

docs.microsoft.com shows the inherited properties on the Button page:

https://docs.microsoft.com/en-us/dotnet/api/system.windows.controls.button?view=windowsdesktop-6.0

...but when you click on the hyperlink for the property, it takes you to the system.windows.frameworkelement.actualheight URL -- there is not a duplicated URL for system.windows.button.actualheight.

In other words, the #3429 inheritance is more like a viewing aid, that doesn't alter the basic page URLs or model.

Whereas the #3430 @unexported feature maybe is more invested in the fiction that UserInputBase.disabled belongs to Button. It might be interesting to look at your real website template. As long as the properties table for Button shows disabled, is it really a problem if Button.disabled and Checkbox.disabled take you to the same URL? Maybe this is really a question about the navigation hierarchy -- being able to click around and get back to Button easily while reading the docs for disabled.

Fluent UI has very brief documentation for each property, so they just put everything on one page, which sidesteps the problem of duplicated URLs:

https://developer.microsoft.com/en-us/fluentui#/controls/web/button#IButtonProps

[zelliott]

Fluent UI has very brief documentation for each property, so they just put everything on one page, which sidesteps the problem of duplicated URLs:

Our primary use case right now is the same as Fluent's. We have a single documentation page for every component in our component library. So, yeah, the whole URL discussion isn't particularly important to use at the moment.

Regardless, after some additional thought, I think I strictly prefer Option 2 to Option 1. Primary reasons being:

• I think it's wise for the .api.json to copy the approach .d.ts files take, and to emit UserInputBase (even thought it's unexported), instead of trying to "copy over" the inherited members to any sub classes.
• Option 1 bloats the .api.json file in the case of a base class with many members and many sub classes (each inherited member is copied to every sub class).

[zelliott]

Quick note: Another real-world example of unexported base classes is the mixin pattern used extensively within api-extractor-model.

[zelliott]

After a long discussion between @octogonz and myself today, we concluded that API Extractor should no longer treat base classes that are only "reachable" via inheritance as "forgotten exports". That means that B will no longer be a forgotten export in this scenario:

// Scenario 1
class B {
  prop: string;
}
export class A extends B {}

but will still be a forgotten export in this scenario:

// Scenario 2
class B {
  prop: string;
}
export function doThing(): B { ... }

The docs page for ae-forgotten-export says:

This message is reported when an exported API refers to another declaration that is not exported.

In scenario 1, the exported API does not refer to B. There is some kind of "weak reference" happening, where it refers to inherited members such as B.prop but not B itself. In scenario 2, the exported API doThing very much refers to B. If B isn't exported, this severely limits what an API consumer can do with doThing's return value. This same restriction is not present in scenario 1.

This conclusion means the following:

• The ae-forgotten-export warning will no longer be reported for unexported base classes that are only referenced via inheritance.
• These unexported base classes will be included in the API doc model.
• This will be the new default behavior for API Extractor (i.e. no config setting in api-extractor.json to get the old behavior).
• Some work may need to be done to handle name collisions. That is, suppose in scenario 1, there is another exported declaration called B.
• Some thought will need to be given regarding how to surface these unexported base classes on an API docs site. Should they just be treated identically to exported classes, albeit with some "(Not exported)" indicator?

@octogonz - please let me know if I've misrepresented/misunderstood anything.

[schmidt-sebastian]

I have hit this issue now in two projects I am working on. For the first project, I actually ended up writing a post-processor that cleaned up the API generated by API Extractor (https://github.com/firebase/firebase-js-sdk/tree/master/repo-scripts/prune-dts). This script and its integration with API Extractor adds two distinct features:

• We assume any types prefixed with "_" are internal. We do not use the @internal tag since we would have to add this to too many places in our code base.
• Related to this issue, if a type A extends a type _B, we hide this fact and pull all of Bs members into A:

class _B {
  b;
}
class A extends _B {
  a;
}

becomes:

class A {
  a;
  b;
}

I understand that this is probably not in the spirit of this project, but I have since started working on a new project and am hitting the exact same limitations again. I would like to have a clean, user-facing API that doesn't reveal my internal code structure. For the new project, I am likely moving away from inheritance and will instead use a delegate.

I am super excited to see the work on this issue and hope that we can resolve it soon.

[h-a-n-a]

We've encountered the same problem when utilizing api-extractor for api reports.

Assuming we have to packages and the interface of one package extends the other interface defined in the other package.

// package B
export interface B {}

// package A
export interface Foo extends Bar {}

With "includeForgottenExports", the Bar is not being included in the final api report of package A.