Skip to content

[api-extractor] Extending anonymous classes errors that _base class inserted by compiler is not exported. #4429

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

Open
CraigMacomber opened this issue Nov 27, 2023 · 4 comments
Labels
enhancement The issue is asking for a new feature or design change needs design The next step is for someone to propose the details of an approach for solving the problem

Comments

@CraigMacomber
Copy link

Summary

export class Foo extends class {} {}

Produces:

declare const Foo_base: {
    new (): {};
};
export declare class Foo extends Foo_base {
}
export {};

Putting an API-Extractor release tag on "Foo" results in an "ae-forgotten-export" that there is no release tag on "Foo_base".
However this is no clean way to put the release tag on "Foo_base" other than refactoring the code to explicitly export the base class.

Playground link

Ideally extra non-exported types with mangled names which TSC adds as an implementation detail of d.ts files would not impact API-Extractor. I think suppressing the warning/error in this case would be the optimal outcome.

Unfortunately I don't think there is a truly robust way to detect this case just looking at the d.ts file, since developers can manually export types named SomeClass_base. (Note that in this case TSC may generate a name of the format SomeClass_base_1 to avoid the collision), but just suppressing the warning/error for all non-exported classes of the format SomeClass_base_1 or SomeClass_base would be enough for our use cases.

If adding a special case for this pattern is not a good solution, detecting these cases (as effectively as possible) as a heuristic to add a better error message for this case. For example:

The symbol "SomeClass_base" needs to be exported by the entry point index.d.ts. This type may have been implicitly generated by the TypeScript compiler for the base type of a class which extends an anonymous class expression. In this case this error can be resolved by explicitly exporting the result of that expression, then extending that exported constant.

Another option would be to include a config setting to opt out of this case being an error.

Repro steps

/**
 * @alpha
 */
export class Foo extends class {} {}

Expected result: No errors, or a more specific error.

Actual result: Error: src/class-tree/testRecursiveDomain.ts:32:1 - (ae-forgotten-export) The symbol "RecursiveObject_base" needs to be exported by the entry point index.d.ts

Details

This came up with the schema system we are creating for Fluid-Framework's trees. All users of our schema system which export schema and use API-Extractor will hit this.

A real world example of this is in https://github.com/microsoft/FluidFramework/pull/18350/files/5a2309887741c6e4a7976b824f062960ec32f02a..7dcf01f1717e5f021e2c2c9f9238fa65038b3e47#diff-7006e9416c4e849ea87906ed645eab13666c035fe928e478446c9bd1cf0f4ea8

This pattern was partially adopted to work around microsoft/TypeScript#55832 : Using classes like this lets the compiler refer to them easier and avoid generating any in some recursive cases. There are also other reasons we like this pattern (it allows apps to add methods to schema, works with instanceof, gets better intelisense).

I suspect other mixing type patterns might hit this as well.

Standard questions

Please answer these questions to help us investigate your issue more quickly:

Question Answer
@microsoft/api-extractor version? 3.38.3
Operating system? Linux
API Extractor scenario? reporting (.api.md)
Would you consider contributing a PR? Yes if the fix is trivial
TypeScript compiler version? 5.1.6
Node.js version (node -v)? 18.17.1
@octogonz
Copy link
Collaborator

octogonz commented Jan 23, 2024

Here's an expanded example:

example.ts

/** Docs for Foo */
export class Foo extends class { 
    /** Docs for X */ 
    x: number = 1;
} { 
    /** Docs for Y */ 
    y: number = 2;
}

example.js

/** Docs for Foo */
export class Foo extends class {
    constructor() {
        /** Docs for X */
        this.x = 1;
    }
} {
    constructor() {
        super(...arguments);
        /** Docs for Y */
        this.y = 2;
    }
}

example.d.ts

declare const Foo_base: {
    new (): {
        /** Docs for X */
        x: number;
    };
};
/** Docs for Foo */
export declare class Foo extends Foo_base {
    /** Docs for Y */
    y: number;
}
export {};

We can see that:

  • The TypeScript language must model this mess, because it is an inherent feature of ECMAScript class.
  • In a nutshell, class { } expressions are like a macro that generates two separate declarations.
  • Ideally API Extractor would roll them together like the @partOf suggestion from Handling/documenting merged declarations tsdoc#137, but it probably also would be fine to model them as separate API's...
  • ...in which case the main challenge is how to specify the /** */ information for the _base declaration

Unfortunately I don't think there is a truly robust way to detect this case just looking at the d.ts file, since developers can manually export types named SomeClass_base.

I think it would be a reasonable heuristic to say that:

  • IF Thing is accompanied by Thing_base
  • AND Thing_base has no /** */ comment
  • THEN we can assume that Thing_base is the TypeScript generated companion for Thing

That is very unlikely to conflict with any realistic API designs when using API Extractor.

@octogonz
Copy link
Collaborator

  • IF Thing is accompanied by Thing_base

🤔 But then this code...

export interface Foo_base { }
export class Foo extends class { } { }

...produces a _base_1 suffix... 😆

export interface Foo_base {
}
declare const Foo_base_1: {
    new (): {};
};
export declare class Foo extends Foo_base_1 {
}
export {};

@octogonz octogonz added enhancement The issue is asking for a new feature or design change needs design The next step is for someone to propose the details of an approach for solving the problem labels Jan 23, 2024
@octogonz
Copy link
Collaborator

octogonz commented Jan 23, 2024

Here's a different approach: Maybe we could introduce a new TSDoc tag (@applyToEmitted?) that allows a .ts file to include /** */ comments that will apply to emitted declarations in the .d.ts file (that do not exist in the .ts file).

Example:

/** Docs for Foo */
export class Foo extends class { 
    /** Docs for X */ 
    x: number = 1;
} { 
    /** Docs for Y */ 
    y: number = 2;
}

/** 
 * This declaration is part of the {@link Foo} type.
 * 
 * @public
 * {@applyToEmitted Foo_base}
 */

This is not as elegant as guessing that Foo_base goes with Foo, but assuming that you will be documenting them as separate declarations, it does provide a story for how to actually write the API docs for the Foo_base declaration.

Whereas, on the other hand, if we were taking the approach of @partOf, then it would be a valuable convenience for API Extractor to magically guess the relationship to avoid having to write something like:

/** {@partOf Foo} {@applyToEmitted Foo_base} */

...but that @partOf proposal had its open design questions that would need to be solved first.

@CraigMacomber
Copy link
Author

I field microsoft/TypeScript#59550 in the TypeScript repo which would avoid this issue if implemented.

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 needs design The next step is for someone to propose the details of an approach for solving the problem
Projects
Status: AE/AD
Development

No branches or pull requests

2 participants