Mark publicly-visible internal properties as @internal

[cartant]

We should mark all publicly-visible properties - e.g. there are a whole bunch in Subject - as @internal - in the JSDoc - so that it's clear they should not be used.

Note that we cannot use TypeScript's stripInternal compiler option:

Strip Internal - stripInternal

Do not emit declarations for code that has an @internal annotation in it’s JSDoc comment. This is an internal compiler option; use at your own risk, because the compiler does not check that the result is valid.

However, we could look at using another tool to remove them:

If you are searching for a tool to handle additional levels of visibility within your d.ts files, look at api-extractor.

Also, the types that are used solely for driving return value inference should be marked as @internal, too. Even though we're exporting the, I think we should reserve the right to modify their implementations or rename them, etc.

[tmair]

Out of curiosity I had a look at the api-extractor tool. I noticed one limitation that could limit its use within the rxjs project:

The .d.ts rollup feature currently assumes that your package has a single entry point (the mainEntryPointFilePath setting).

Further explanation can be fount at the documentation and in the feature request microsoft/rushstack#664

It is possible to do an API Explorer rollup pass per entry point, but for example the Observable class declaration will not be referenced from the rxjs/operators enrty point to be imported from the rxjs entry point but included in the rollup d.ts bundle.

[benlesh]

FWIW: They should also be renamed to have an _name underscored name, so people who aren't using TypeScript will have a cue that it's internal as well. Love it or hate it, that's a been a standard forever, and it's embarrassing that we haven't followed that, IMO.

[benlesh]

I'd actually prefer that we just protected or private some _properties whenever possible. Now is the time, IMO.

[cartant]

For context, I'd like the @internal attributes to be added - in addition to a leading underscore - because they are easy to enforce with a lint rule. And it's possible to have protected properties that are also internal - there are a bunch in Subject. I'm not advocating adding tooling to mess with the .d.ts 'cause tooling is a PITA.

[JannesMeyer]

I think it would be fine to use @internal because TypeScript itself uses it all over its own codebase. Take a look at this file for example: https://github.com/microsoft/TypeScript/blob/master/src/compiler/corePublic.ts (there are many more)

The warning in the docs is about the fact that you can create a situation where you mark things as internal that might be needed in another place, that you haven't marked as internal.

Example:

function foo(bar: Bar) {
    console.log(bar);
}

/* @internal */
class Bar() {

}

In this case foo() is public, but the API doesn't provide you with a way to create a Bar object, so it's unusable.

[benlesh]

Also: There are @deprecated APIs that should really be @internal

[cartant]

Closed by #6215