avajs / ava

Node.js test runner that lets you develop with confidence 🚀
MIT License
20.74k stars 1.41k forks source link

Use TypeScript assertion function signatures #2411

Closed earthlyreason closed 4 years ago

earthlyreason commented 4 years ago

Our team is just checking out AVA, and so far so good. Thanks for making it!

Background

TypeScript 3.7 introduces assertion functions. This is a control-flow-analysis (CFA) feature targeted at functions like several of those that AVA provides.

Example

Suppose that you have a Result union type that discriminates on whether its success property is true or false.

type Result =
  | { success: true, good_stuff: any }
  | { success: false, bad_stuff: any }

Currently, if you say

const result = some_function_returning_result()
t.assert(result.success)
// result is not narrowed, so
// Error: Property 'good_stuff' does not exist on type 'Result'
const { good_stuff } = result;

TS doesn't know that result can now be narrowed to half of the union, even though that's clearly the intent (and effect) of the assertion.

Instead, you have to write

const result = some_function_returning_result()
t.assert(result.success)
if (result.success) {
  // now result is narrowed
  const { good_stuff } = result;
}

With assert signatures (which essentially implies the if block as written above), TS will know that after the assert call, result has good_stuff and not bad_stuff.

Solution

For assert this can be accomplished through a simple change to the AssertAssertion interface in index.d.ts:

-   (actual: any, message?: string): void;
+   (actual: any, message?: string): asserts actual;

Stronger signatures are possible for the other functions. I first wanted to see whether this was a non-starter for some reason before digging into the others.

Note that you can't achieve the same thing with module augmentation. The following attempt

declare module "ava" {
  interface AssertAssertion {
    (actual: any, message?: string): asserts actual;
  }
}

Results in error TS 2775

Assertions require every name in the call target to be declared with an explicit type annotation

I believe this is because the type extension does not remove the existing overload, which returns void rather than an assertion. See https://github.com/microsoft/TypeScript/pull/33622

Drawbacks

I always stay on the latest TypeScript version, and I haven't researched how this change would affect users who are pegged to earlier versions. This is the only downside I can think of.

novemberborn commented 4 years ago

Thanks!

I looked into this with https://github.com/avajs/ava/issues/2330 but best I could tell it wouldn't actually work. I get the feeling they added this because it would work in certain scenarios, but it doesn't in many others.

If you were to get it working we'd definitely consider this (and AVA 3 assumes TypeScript 3.7 so that'd be fine), but for now I'm going to close this issue, for house keeping purposes.

earthlyreason commented 4 years ago

Thanks for the reply, @novemberborn! (Which I am also :) )

I swear I searched issues first, but missed that one.

The complication with generics does appear to be a showstopper for is-like assertions.

AFAICT, the simpler case of a truthiness test works as desired for the assert function, but I understand that it's also desirable to keep the API consistent where possible.