Type definition policy

[falsandtru]

@twada I'm having trouble with the breaks of tests caused by types. Do you want to make the next test a type error?

declare const set: Set<0>;
assert(set.size === 0);
set.add(0);
assert(set.size === 1); // Do you want to make this line a type error?

The asserts type predicate makes a type error on the test. Do you expect the type definition to use asserts and break the existing valid tests?

[ChristianMurphy]

Some additional context, @falsandtru's question is specifically in regards to DefinitelyTyped/DefinitelyTyped#56062, which synced power-assert's types with Node assert's, and brought in Node assert's ability to narrow types, for example:

declare const stringOrNull: string | null;
assert(stringOrNull);
stringOrNull // we now know stringOrNull can only be `string`

https://codesandbox.io/s/happy-booth-v5cv4?file=/src/index.ts

In the specific example of Set node's assert does raise an error https://codesandbox.io/s/reverent-paper-ek7vz?file=/src/index.ts

because from TypeScript's perspective, the Set example is equivalent to:

declare const object: { size: number }
assert(object.size === 0);
assert(object.size === 1); // likely an error

The TypeScript compiler does not have dependent types, which would allow it to know that set.add() changes set.size.

[falsandtru]

Since the asserts can't be used for testing mutations, it's clear that the asserts isn't ready to use for testing. If you want to use the premature asserts, it should be optional. In fact, TypeScript isn't officially providing the assertion function using asserts. It means the asserts predicate is inappropriate for common assert functions.

https://github.com/microsoft/TypeScript/blob/v4.4.3/src/lib/dom.generated.d.ts#L17750

[ChristianMurphy]

Since the asserts can't be used for testing mutations,

This is a misleading comment, I gave you an example of how assert/power-assert can continue to check mutations in DefinitelyTyped/DefinitelyTyped#56227 (comment)

it's clear that the asserts isn't ready to use for testing.

I respectfully disagree, it is a feature which has been stable for some time and is ready for testing and use.
I'd be happy to see TypeScript support dependent types such that set.add() automatically updates set.size, but that doesn't mean that asserts is invalid or inappropriate.
It was added to the language as a stable reason, so that it can be used.

TypeScript isn't officially providing the assertion function using asserts. It means the asserts predicate is inappropriate for common assert functions.

That section of the code hasn't been updated in years https://github.com/microsoft/TypeScript/blame/main/lib/lib.dom.d.ts
This isn't an assertion of readiness, it is most likely an oversight.

[falsandtru]

This is a misleading comment, I gave you an example of how assert/power-assert can continue to check mutations in DefinitelyTyped/DefinitelyTyped#56227 (comment)

This is a misleading comment. You just used a different function which is not using asserts predicate.

[ChristianMurphy]

This is a misleading comment

It merely points out that you are fixated on one function signature.
And that there are more appropriate methods which could and potentially should be used.

You just used a different function which is not using asserts predicate.

Since you want to focus on assert() and asserts specifically, here is an example showing that you can still use the same JS syntax you use today (with an additional TS annotation to widen the type before asserting the second time).

import assert from "power-assert";

const set = new Set();
assert(set.size === 0);
set.add(0);
assert((set.size as number) === 1);

https://codesandbox.io/s/stoic-spence-6gkii?file=/src/index.ts

[falsandtru]

The point is "Is the early adoption of the immature asserts predicate worth breaking the existing tests and forcing your ugly test patterns?".

[falsandtru]

If the mature asserts predicate will be adopted later, developers don't have to change a lot of existing tests. You should wait to adopt until the asserts predicate matures.

[ChristianMurphy]

Is the early adoption of the immature asserts predicate worth breaking the existing tests and forcing your ugly test patterns

Answering this on several fronts:

1. It is not early adoption, TS has had this feature for years, Node's assert has used it for some time as well.
2. It does not break existing tests, the JS continues to run as expected, only the TS annotations need to be clarified
3. TypeScript is a static typed language, doing dynamic mutations goes against the grain of the language, so yes, it may seem ugly. That is the nature of doing very dynamic things in a static context, it is inherent to TypeScript and other statically typed languages.

---

My point is that the lack of type narrowing support, was breaking downstream adopters, who expect power-assert to be a drop in replacement, with the same API as assert. (see discussion in micromark/micromark#87)
Type narrowing is a valuable feature, I appreciate that TypeScript added it, and believe it adds a lot of value in this context.

[ChristianMurphy]

You should wait to adopt until the asserts predicate matures.

It is mature and stable.

It also isn't changing anytime soon.
Scanning both TypeScript's and DefinitelyTyped's issue trackers show no plans for TS to change the syntax behavior or for @types/node to remove asserts.

[falsandtru]

Of course @twada's clients who use @types/power-assert will also have to change the tests.

[ChristianMurphy]

That would be true, if any use that specific type of mutation test, it would require the additional widening type annotation.
Scanning publicly available information on dependents, this doesn't appear to be the case, the dependents of @types/power-assert list on NPM don't leverage power assert for mutation testing https://www.npmjs.com/browse/depended/@types/power-assert

It's also true that reverting this change would break adopters, like micromark and remark that leverage the type narrowing feature today.

[twada]

My opinion is simple. Since power-assert is intended to be 100% compatible with Node Assert API and you don't need to require('power-assert') directly anymore, you better not to use power-assert type definition. Please use Node Assert API Type definition instead.

[NEW] Now you don't need require('power-assert') any more. Keep using require('assert'), and power-assert enhances them transparently.

(from power-assert README)

FYI: From Library to Tool - power-assert as a General Purpose Assertion Enhancement Tool
https://speakerdeck.com/twada/from-library-to-tool-power-assert-as-a-general-purpose-assertion-enhancement-tool

[falsandtru]

Please use Node Assert API Type definition instead.

@twada It's impossible to apply the node type definitions to the power-assert library.

[ChristianMurphy]

Thanks for clarification @twada 🙇

It's impossible to apply the node type definitions to the power-assert library.

It is possible, they can be copied and pasted, as they already have.
That said, please read the slide deck @twada linked https://speakerdeck.com/twada/from-library-to-tool-power-assert-as-a-general-purpose-assertion-enhancement-tool
The whole presentation, and particularly slides 13 and 14, explain that power-assert should no longer directly be imported, that assert should be used directly, and power-assert will automatically enhance it.

[falsandtru]

The whole presentation, and particularly slides 13 and 14, explain that power-assert should no longer directly be imported, that assert should be used directly, and power-assert will automatically enhance it.

Are you serious? power-assert can't be typed from the node types by such way. Try it if possible.

[ChristianMurphy]

I'm absolutely serious, install @types/node, then import assert from 'assert'.
power-assert works to wrap assert as @twada describes, and the node typings work perfectly fine.

[falsandtru]

@types/node doesn't work for power-assert. And if you don't use power-assert, you don't need @types/power-assert.

[ChristianMurphy]

if you don't use power-assert, you don't need @types/power-assert.

Yes, that is the point being made 🙂
When power-assert is used only to wrap assert, you can use @types/node directly.

@types/power-assert can probably be deprecated, if there are strong objections to it being deprecated it should be (manually or automatically) kept in sync with @types/node
So that it is 100% compatible with Node Assert.

[falsandtru]

So you don't need to maintain @types/power-assert. Micromark also stopped using @types/power-assert and power-assert. You troll.

[falsandtru]

@twada Can you answer a technically feasible solution to resolve the broken tests?

[twada]

@falsandtru @ChristianMurphy

I played around the new typings and found that type narrowing of assert function (asserts value) introduces some false positives for testing purpose. It's valid and convenient in context of general purpose assertion in production code, but not for test code. Node Assert API is intended to be used as general purpose assertion function so its typing is fine, but a bit confusing for testing purpose.

So I propose changing asserts value in @types/power-assert to void again.

We lose 100% compatibility in type level. So sad, but reasonable for now.

Update:
I'd like to say thanks to @falsandtru for your notification and effort.

[RebeccaStevens]

Just thought I should mention that if the expected value is typed as a number, you don't get the type error.

const size = 0;
assert(set.size === size);

// or

assert(set.size === (0 as number));