Don't declare private fields in user facing definition files

[spion]

When tsdoc is used from multiple libraries each having a slightly different version, private fields cause type incompatibilites. For example:

src/api-documenter/markdown/CustomMarkdownEmitter.ts:167:7 - error TS2345: 
Argument of type 
'import("node_modules/@microsoft/tsdoc/lib/nodes/DocDeclarationReference").DocDeclarationReference' 

is not assignable to parameter of type 
'import("node_modules/@microsoft/api-extractor-model/node_modules/@microsoft/tsdoc/lib/nodes/DocDeclarationReference").DocDeclarationReference'.

Types have separate declarations of a private property '_tagDefinitions'.

This is due to slightly different versions requested by the libraries, as shown in yarn.lock:

"@microsoft/tsdoc@0.12.14":
  version "0.12.14"
  resolved "https://registry.yarnpkg.com/@microsoft/tsdoc/-/tsdoc-0.12.14.tgz#0e0810a0a174e50e22dfe8edb30599840712f22d"
  integrity sha512-518yewjSga1jLdiLrcmpMFlaba5P+50b0TWNFUpC+SL9Yzf0kMi57qw+bMl+rQ08cGqH1vLx4eg9YFUbZXgZ0Q==

"@microsoft/tsdoc@^0.12.14":
  version "0.12.18"
  resolved "https://registry.yarnpkg.com/@microsoft/tsdoc/-/tsdoc-0.12.18.tgz#7e7aadc7009e57a25fbf6c9f48b44c2fe30ada88"
  integrity sha512-3rypGnknRaPGlU4HFx2MorC4zmhoGJx773cVbDfcUgc6zI/PFfFaiWmeRR6JiVyKRrLnU/ZH0pc/6jePzy/QyQ==

Yarn always prefers the newest version possible for each module - however, private fields make even "revision" versions incompatible.

[octogonz]

@spion There are a couple topics at play here:

The compiler's check: In my view, the error reported by the TypeScript compiler is correct and useful. When two different objects implement the same interface, there's a reasonable expectation that they can be interchangeable. Whereas when the type refers to a class (especially containing private members), it's not a behavioral contract, it is an implementation. It would be dangerous to substitute a different implementation (i.e. from a different source file), even if that source file happens to come from a different version of the same package. In many cases it probably works okay, but "probably works okay" isn't an appropriate quality bar for professional software. 🙂 (And note that the compiler does have special logic to allow a different source file from the same version of the same package, because that is safe.)

Yarn's installation strategy: In your example, Yarn is choosing versions differently from how NPM and PNPM do it. (There is a very long debate about whether this is a good idea or not, but we can't solve that here.) In any case, in your example Yarn has produced an installation that is not safe. The practical way to fix it is by adjusting version ranges in package.json or by using Yarn's resolutions setting.

I don't see how we could fix this in the TSDoc project (unless I'm missing something?).

Where is the ^0.12.14 coming from?

[spion]

^0.12.14 is coming from a module I wrote based on api-documenter that generates documentation to my liking.

As per semver, modules are not supposed to break their contract in a minor or patch release. However, adding a private field to the contract automatically breaks that compatibility for every single subsequent version released (major, minor or patch) with no way to opt-out.

The resolution strategy in Yarn is safe. It enforces the maximum version possible since that's the version least likely to contain vulnerabilities and to be fully patched and up to date.

There are several ways to solve this:

• TSDoc could remove private fields from its public library contract
• api-extractor could stop using exact versions and switch to more flexible semver-compatible ranges (assuming tsdoc follows semver)
• TypeScript could ignore private fields when checking version compatibility

Regarding the safety lesson:

1. In typescript, classes like interfaces are also structural, not nominal. The existence of private fields is the only thing making them nominal. This has no effect on safety until inheritance gets involved - therefore typescript doesn't need to check private fields except for that case
2. Private fields are too rigid for declaring incompatibility between the same type structures in different library versions. I've found the best method to do it is to use branded types with a versioned brand (for example, something like this module). The brand name could be "my-package.OrderId.v1" and can be bumped once the new type becomes incompatible with an old version of the library. This gives a high degree of control to type (in)compatibility between versions for the library author and is more in line with the various valid directory structures produced by different package managers.

Regardless of debates around package managers, libraries should generally strive to accommodate many different configurations, rather than pick a subset of configurations that they're compatible with. "Works with my setup" isn't an appropriate quality bar for professional software. :grinning:

(Sorry for the counter-jab, I found it too hard to resist!)

[octogonz]

As per semver, modules are not supposed to break their contract in a minor or patch release. However, adding a private field to the contract automatically breaks that compatibility for every single subsequent version released (major, minor or patch) with no way to opt-out.

Your usage of the word "contract" here seems somewhat loose. Consider this code:

import { Service, Manager } from "the-library";

const manager: Manager = new Manager();
cons service: Service = new Service(manager);
manager.start(service);

Generally SemVer means that a consumer can develop the above application using the-library@1.1.0. And then later, this code should continue to compile/run correctly when we substitute the-library@1.2.3.

Whereas if I understand correctly, your use case is more like this:

import { Service } from "the-library@1.1.0";
import { Manager } from "the-library@1.2.3";

const manager: Manager = new Manager();
cons service: Service = new Service(manager);
manager.start(service);

In other words, we are expecting two instances of the-library to run side-by-side, and to mix together their objects. But what if the Service and Manager access each other's internals, which are not part of the public contract? Or what if they refer to a singleton resource, but we now have two singletons because of the two different instances of the library? Supporting that kind of thing requires either getting lucky, or else carefully engineering contracts around these requirements. It seems to go well beyond the conventional expectations of SemVer.

TSDoc could remove private fields from its public library contract

🤔I wonder how easy this would be. It seems like a lot of trouble to enable a use case that we didn't really intend to support. I'm open-minded about it, though.

api-extractor could stop using exact versions and switch to more flexible semver-compatible ranges (assuming tsdoc follows semver)

This can happen after TSDoc reaches version 1.0.0.

Currently TSDoc does try to follow SemVer (using NPM's ^ rules, it goes 0.MAJOR.MINOR when the first part is 0). But it's not rigorous enough for a tool like API Extractor to rely on.

TypeScript could ignore private fields when checking version compatibility

They might have an option for that. Also ECMAScript has a new #private syntax that might have different rules.

[spion]

I'm guessing you meant to give an example with two connected instances of the same class, one taking the other as an argument - at least that's the easiest example I can think of.

Unfortunately, the reality in JS (due to how npm and package managers that are alternative implementations of the same philosophy work) is that we have to accept that any user-passed inputs could come from version-incompatible libraries, even those that are instances of the same class. We've had this actually happen with Bluebird for example - bluebird had a fast path for "thenable" conversion to promise when the thenable was coming from Bluebird, however it used "instanceof" checks which would fail with different versions installed in different places in the filesystem. Bluebird switched to property based recognition and detection to avoid that particular issue.

In TypeScript I'm guessing the right solution is to make the private field check structural (unless inheritance is in play) and rely on deliberate modifications for private fields and methods to signal incompatibility at compile-time

[octogonz]

@spion Some questions:

1. How did you work around this problem?

2. Isn't it problematic to mix together two different versions of tsdoc? Rather than trying to make that work, wouldn't a better solution be to make the versions consistent (i.e. upgrading api-documenter to the latest version)?

TSDoc could remove private fields from its public library contract

3. How would we do this? I counted 168 private members in the public API.

[spion]

1. Forcing the same pinned versions that api-extractor uses
2. I don't know - but also, there is no way for tsdoc to even signal whether it is or it isn't problematic - the mere existence of private fields ensures it's not possible to use different versions together even if its just a simple patch version that has made no changes to that class. (1)
3. I'm actually not sure I have anything actionable for tsdoc here that is worth the effort. I guess interfaces can be exported instead of the classes. But I'm leaning towards the opinion that maybe the type checker needs to be revised

(1): Its actually a bit worse than that. If library A is using libraries, B, C, tslib and D, together; and B and C use one version of tslib (say 1.0.1); while A and D depend on another (say 1.0.2), it's possible for yarn to choose the main tslib (1.0.2) as a primary and the one used by B and C as copies in the filesystem (even though they are the same 1.0.1 version). In this scenario, even if you're not passing objects from the top-level tslib to B and C, there would still be compile-time errors if B and C interact - while their copies are exactly the same version, they have to be placed in different places in the filesystem and would, therefore, be considered different and incompatible by the compiler.