power-assert-js / power-assert Public
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
Type definition policy #140
Comments
Some additional context, @falsandtru's question is specifically in regards to DefinitelyTyped/DefinitelyTyped#56062, which synced 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 because from TypeScript's perspective, the 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 |
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 |
This is a misleading comment, I gave you an example of how
I respectfully disagree, it is a feature which has been stable for some time and is ready for testing and use.
That section of the code hasn't been updated in years https://github.com/microsoft/TypeScript/blame/main/lib/lib.dom.d.ts |
This is a misleading comment. You just used a different function which is not using |
It merely points out that you are fixated on one function signature.
Since you want to focus on 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 |
The point is "Is the early adoption of the immature |
If the mature |
Answering this on several fronts:
My point is that the lack of type narrowing support, was breaking downstream adopters, who expect |
It is mature and stable. It also isn't changing anytime soon. |
Of course @twada's clients who use @types/power-assert will also have to change the tests. |
That would be true, if any use that specific type of mutation test, it would require the additional widening type annotation. It's also true that reverting this change would break adopters, like |
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.
FYI: From Library to Tool - power-assert as a General Purpose Assertion Enhancement Tool |
@twada It's impossible to apply the node type definitions to the power-assert library. |
Thanks for clarification @twada
It is possible, they can be copied and pasted, as they already have. |
Are you serious? power-assert can't be typed from the node types by such way. Try it if possible. |
I'm absolutely serious, install |
@types/node doesn't work for power-assert. And if you don't use power-assert, you don't need @types/power-assert. |
Yes, that is the point being made
|
So you don't need to maintain @types/power-assert. Micromark also stopped using @types/power-assert and power-assert. You troll. |
@twada Can you answer a technically feasible solution to resolve the broken tests? |
I played around the new typings and found that type narrowing of So I propose changing We lose 100% compatibility in type level. So sad, but reasonable for now. Update: |
@twada I'm having trouble with the breaks of tests caused by types. Do you want to make the next test a type error?
The
asserts
type predicate makes a type error on the test. Do you expect the type definition to useasserts
and break the existing valid tests?The text was updated successfully, but these errors were encountered: