Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[api-extractor] Getting around (ae-forgotten-export) for certain cases #1235

Open
akshay-madhukar opened this issue Apr 15, 2019 · 10 comments
Open
Labels
enhancement The issue is asking for a new feature or design change

Comments

@akshay-madhukar
Copy link

Is there any way to suppress warnings from being written into the .api.md files, but only for particular instances. I do not want to turn off ae-forgotten-export entirely. Is a tag or command that could be used to disable warnings similar to tslint-disable?

For example, in the following react code:

/** Properties for the [[Backstage]] React component.
* @public
*/
export interface BackstageProps {
. . .
}
 
/** State for the [[Backstage]] React component.
* @internal
*/
interface BackstageState {
. . .
}
 
/** Backstage React component.
* @public
*/
export class Backstage extends React.Component<BackstageProps, BackstageState> {

I do not want to export the interface BackstageState as the state of the React component would only be available in the component itself. The interface is purely an implementation detail. If I were to export the interface and set it to internal, there would be an ae-incompatible-release-tags warning instead.

Would there be a way to turn of the warnings just for this component and not for the entire package?

@octogonz octogonz added the enhancement The issue is asking for a new feature or design change label Apr 15, 2019
@octogonz
Copy link
Collaborator

This is a pretty easy enhancement. There was a plan for a config/api-extractor-suppressions.txt file (see PR #1120), but that feature got cut from AE7 so we could release it faster.

FYI I also proposed an @suppress tag for TSDoc, but the consensus was that doc comments aren't a good place to represent suppressions.

@octogonz
Copy link
Collaborator

octogonz commented Apr 16, 2019

@akshay-madhukar I should also point out that by default the ae-forgotten-export message is configured with addToApiReportFile assigned to true. This causes the warning to be added to the API report file rather than printed to the console.

api-extractor-defaults.json

{
  . . .
  "messages": {
    "compilerMessageReporting": {
      "default": {
        "logLevel": "warning"
      }
    },
    "extractorMessageReporting": {
      "default": {
        "logLevel": "warning"
      },
      "ae-forgotten-export": {
        "logLevel": "warning",
        "addToApiReportFile": true  // <---  THIS
      },
      . . .
    },
    "tsdocMessageReporting": {
      "default": {
        "logLevel": "warning"
      }
    }
  },
  . . .
}

The resulting API report will look like this:

// Warning: (ae-forgotten-export) The symbol "Y" needs to be exported by
// the entry point index.d.ts
// 
// @public (undocumented)
export class X extends Y {
}

When this happens, the message is not counted as a console warning, and thus it will NOT break the build. In other words, the api-extractor tool will return process exit code 0 which indicates success. Thus, although it's slightly annoying to see these warnings in the API report file, they shouldn't cause any trouble.

If any new warnings are introduced later, that would cause a diff in the report file, so you'll be able to catch them during a PR code review.

@gregjacobs
Copy link

Hey @octogonz, I'm coming across this as well. I have an internal-only type which is used to create a larger exported type, but I don't want to export the internal-only type. Basically something like this:

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

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

We would like to be able to mark PartialComponentProps as @internal to suppress the "ae-forgotten-export" error for that particular type not being exported, while still exporting ComponentProps.

By the way, we have api-extractor configured with "ae-forgotten-export" set to "error" so that we get an automated check that we're exporting everything we intend to during the build, but we need the ability to remove certain types from the forgotten-export check.

@octogonz
Copy link
Collaborator

octogonz commented Jul 6, 2021

@gregjacobs It sounds like you want to eliminate the ae-forgotten-export warning by doing something like this:

/**
 * Nobody is supposed to import this interface directly; it is merely an
 * auxiliary definition that helps us declare `ComponentProps`, which is
 * the thing that people should import.
 * @internal
 */
interface PartialComponentProps {
    a: string;
    b: string;
}

/**
 * @public
 */
export type ComponentProps = PartialComponentProps & {
    c: string;
};

But then API Extractor is going to report ae-incompatible-release-tags because a @public type signature cannot depend on an @internal type.

In your example, PartialComponentProps is very definitely a "public" API -- you just want that interface name to be unreachable (effectively anonymous) from the perspective of your consumers.

Thus it seems that @internal is not the right tool for this job. In microsoft/tsdoc#134 people did not like the idea of a generalized @suppress tag for TSDoc. But maybe it would make sense to use a TSDoc tag for your situation.

Maybe the tag name would be something like @anonymous or @unexported or @partial?

/**
 * @partial
 */
interface PartialComponentProps {
    a: string;
    b: string;
}

(Related, I recall that @rbuckton had proposed a declaration reference syntax for linking to unexported members of a module.)

If we can decide on a design, making the PR is pretty easy.

@octogonz
Copy link
Collaborator

octogonz commented Jul 6, 2021

I'm not 100% sure about the tag name, but @partial seems the best of these 3 suggestions, because it conveys WHY it is anonymous/exported: Because it is an uninteresting part of some larger API that is properly exported.

Other words along those lines: @constituent, @subordinate, @fragment -- actually I like these better than @partial because they avoid confusion with Partial<Type>

@octogonz
Copy link
Collaborator

octogonz commented Jul 6, 2021

Another idea would be to model this using @partOf as proposed in microsoft/tsdoc#137 (comment)

For example:

/** {@partOf ComponentProps} */
interface PartialComponentProps {
    a: string;
    b: string;
}

/**
 * Represents the properties for a component
 * @remarks
 * More details here.
 * @public
 */
export type ComponentProps = PartialComponentProps & {
    c: string;
};

Then the generated API docs would treat them as a single entity:

ComponentProps Type

Represents the properties for a component

Signature:

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

Remarks

More details here

Advantages of this approach:

  • The API docs website could present PartialComponentProps and its members as part of ComponentProps, which might be more intuitive
  • If multiple APIs use PartialComponentProps, exactly one of them will be designated as the official "owner" so there aren't duplicated copies of the PartialComponentProps signature

Disadvantages of this approach:

  • The @partOf proposal had some open design questions that we never followed up on
  • Maybe there are some advantages to giving PartialComponentProps its own page in the API website?

@zelliott
Copy link
Contributor

For the example shared earlier:

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

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

It's not clear to me why an ae-forgotten-export warning should even be thrown. Instead of globally suppressing this warning or suppressing it with some kind of new TSDoc tag, can cases like this be exceptions to the ae-forgotten-export warning. Is there some "bad API design" in the example above?

@bdwain
Copy link

bdwain commented Apr 14, 2022

Another idea would be to model this using @partOf as proposed in microsoft/tsdoc#137 (comment)

For example:

/** {@partOf ComponentProps} */
interface PartialComponentProps {
    a: string;
    b: string;
}

/**
 * Represents the properties for a component
 * @remarks
 * More details here.
 * @public
 */
export type ComponentProps = PartialComponentProps & {
    c: string;
};

Then the generated API docs would treat them as a single entity:

ComponentProps Type

Represents the properties for a component
Signature:

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

Remarks

More details here

Advantages of this approach:

  • The API docs website could present PartialComponentProps and its members as part of ComponentProps, which might be more intuitive
  • If multiple APIs use PartialComponentProps, exactly one of them will be designated as the official "owner" so there aren't duplicated copies of the PartialComponentProps signature

Disadvantages of this approach:

  • The @partOf proposal had some open design questions that we never followed up on
  • Maybe there are some advantages to giving PartialComponentProps its own page in the API website?

I really like the partof tag idea!

@craigkovatch
Copy link

craigkovatch commented May 17, 2022

This issue is three years old now, are there any recommended options for solving it? I have an API report which complains of ae-forgotten-export on dozens of React *State interfaces, which I don't want to actually library-export. But I also don't want to turn the rule off, because it's one of the biggest reasons I want to use api-extractor in the first place! 😭

@zelliott
Copy link
Contributor

@octogonz and I just spoke a bit about this today and agreed that there's an opportunity to handle this better. My use case is slightly different from yours and is as follows:

class UserInputBase {
  disabled: boolean;
}

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

In the case above, my team considers UserInputBase as essentially an anonymous implementation detail of Button and Checkbox and doesn't want to export it. But we still want to surface API docs for the disabled member inherited by Button and Checkbox. In your case, you have "forgotten exports" that you don't want to export and presumably (?) don't want to document. In my case, I have "forgotten exports" that I don't want to export and do want to document.

One idea we discussed (@octogonz correct me if I've misremembered anything) was to add a new api-extractor.json configuration to control whether "forgotten export" declarations should by default be included in the API report and doc model. If they are included, they can be included with some kind of indication in the API report and doc model that they're unexported.

Then, we discussed using some doc comment tag to (1) silence the ae-forgotten-export warning on a per-declaration basis and (2) control on a per-declaration basis whether the declaration should be included in the API report and doc model. Notably, we didn't talk much about the @partOf tag.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement The issue is asking for a new feature or design change
Projects
Status: AE/AD
Development

No branches or pull requests

6 participants