microsoft / rushstack

Monorepo for tools developed by the Rush Stack community
https://rushstack.io/
Other
5.94k stars 598 forks source link

[api-extractor] private class members #1823

Open johansten opened 4 years ago

johansten commented 4 years ago

Please prefix the issue title with the project name i.e. [rush], [api-extractor] etc.

Is this a feature or a bug?

Please describe the actual behavior.

Private members get exposed in the *.d.ts of publicTrimmed. I can get around it by marking everything private as @internal, but that seems awfully redundant.

If the issue is a bug, how can we reproduce it? Please provide detailed steps and include a GitHub branch if applicable. Your issue will get resolved faster if you can make it easy to investigate.

What is the expected behavior?

I would assume that a member marked private would implicitly be @internal

If this is a bug, please provide the tool version, Node.js version, and OS.

octogonz commented 4 years ago

Private members get exposed in the *.d.ts of publicTrimmed. I can get around it by marking everything private as @Internal, but that seems awfully redundant.

This is by design. In the TypeScript language, a subclass is not allowed to reuse the same name for a private field from the base class:

export class B {
    private _x: number = 0;
}

export class C extends B {
    // ERROR (TS2415): Class 'C' incorrectly extends base class 'B'.
    // Types have separate declarations of a private property '_x'.
    private _x: boolean = false;
}

The reason is that private is a compile time concept, so the JavaScript runtime sees _x as public and "overriding" the base implementation. Such behavior would be highly counterintuitive.

Thus private names need to appear in the .d.ts files. If you look closely, you will notice that the type information is hidden -- only the name is shown, because that's all the compiler needs to detect these mistakes.

If you intend for callers to subclass your API classes, then it would be dangerous to mark privates as @internal.

octogonz commented 4 years ago

@johansten API Extractor does understand the TSDoc @sealed attribute. So I suppose it would be safe to trim private members of sealed classes only, since they aren't intended to be subclassed... But that seems like a fairly narrow edge case. We'd need some justification for why it's important enough to implement.

icy0307 commented 1 year ago

@octogonz Here is a case: We want to be able to mangle our library's class property except for public API. Google closure compiler provides such a feature by feeding it with an extern file Here is an example:

// The `@externs` annotation is the best way to indicate a file contains externs.

/**
 * @fileoverview Public API of my_math.js.
 * @externs
 */

// Externs often declare global namespaces.

const myMath = {};

// Externs can declare functions, most importantly their names.

/**
 * @param {number} x
 * @param {number} y
 * @return {!myMath.DivResult}
 */
myMath.div = function(x, y) {};  // Note the empty body.

Tsickle is a TypeScript to Closure Translator which generates externs.js from TypeScript d.ts.

This is when api-extractor fit into the picture, it can produce a trimmed version of declaration file by release tag 😊 However, only properties with internal tag get trimmed, it is quite a lot of work to mark all the private fields in the library to be internal. Also, as you mentioned above, it is not safe to trim private fields unless it is internal. Do you think this case justifies why this feature is useful?