Skip to content
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

change assert.ok to narrow the type #105

Merged
merged 1 commit into from
Jun 4, 2021
Merged

change assert.ok to narrow the type #105

merged 1 commit into from
Jun 4, 2021

Conversation

ryanatkn
Copy link
Contributor

@ryanatkn ryanatkn commented Mar 20, 2021

This changes the return type of assert.ok from void to asserts actual, making type narrowing work:

type A = {ok: true, data: any} | {ok: false};
const a = {ok: true, data: 1} as A;
assert.ok(a.ok);
a.data; // type error! without this change, type does not narrow

This is the same as assert.ok in @types/node:

function ok(value: any, message?: string | Error): asserts value;

This is more complicated for assert.not.ok. The syntax asserts !val does not work, you have to say asserts val is $type. So, does a falsy union type like asserts actual is false | '' | null | undefined | 0 | -0 | typeof NaN work correctly for all cases? It does for my code, but I don't know if it's correct - I think it is? As a workaround, you can negate the condition and use ok to get that behavior with this PR. I'll follow up on this and some other types in an issue soon. (sorry this may be straightforward, but I believe I couldn't get this to work in a previous TS version, but it works now for the cases I have)

AFAIK this change is only a win - because it only narrows the type, I don't think it can cause errors. The other changes I want to make may break existing typechecks.

@ryanatkn ryanatkn changed the title make assert.ok narrow the type change assert.ok to narrow the type Mar 20, 2021
Copy link
Owner

@lukeed lukeed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Could probably do with some tricks for is, equal, and instance too but it's probably not worth the effort. This one for ok is a safe addition

@lukeed lukeed merged commit a1f9bfb into lukeed:master Jun 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants