-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
TypeScript types: add asserts
and return never
from relevant assertions
#2449
Comments
Ah, I missed these related issues: #2411, #2330 Both issues are focused on adding the test('test title', (t) => {
const result = { success: true, value: 2 } as
| { success: true; value: number }
| { success: false; error: string };
t.true(result.success); // Assertions require every name in the call target to be declared with an explicit type annotation.ts(2775)
} This can be fixed by declaring the type of test('test title', (t: ExecutionContext) => {
const result = { success: true, value: 2 } as
| { success: true; value: number }
| { success: false; error: string };
t.true(result.success);
const sum = result.value + 1;
} As I understand it, this is a fundamental limitation of the TypeScript compiler, and assertions with inferred types are not likely to be implemented any time soon. Unfortunately, most AVA users probably would have trouble figure out how the resolve the error (adding the So I have 2 recommendations: 1.
|
Whilst I share your desire to use AVA's assertions for control flow analysis, I'm not sure if it's worth the hoops we'd have to jump through. If you use I don't think we need to add more assertion methods, either, that will break unless you've explicitly typed I'm not sure why this is useful, but maybe it's a contrived example:
Also it's not unreachable… assertions can fail the test but they don't stop the test from executing. So you might very well want that return if you're doing something "expensive" after. |
@bitjson to emphasize that last point:
This is something I also didn't realize when filing #2411. The assert-like functions on (Edit: Unless you are using the "fail fast" option. But you cannot tell that from the code itself.) |
Note that the assertion doesn't actually have to throw for TypeScript to infer a type. E.g. given the proper annotations, this should work just fine: type Discriminated = { kind: 'foo', foo: string } | { kind: 'bar', bar: string }
const value: Discriminated = getDiscriminated()
t.true(value.kind === 'foo')
t.is(value.foo, 'foo') Unfortunately the necessary syntax may just be too awkward to recommend. |
Ah! I've always used the
@novemberborn yep, it's a little contrived. Here's a real example of the sort of discriminated type I'd love to be able to test without writing Anyway, now I understand that these types would rely on the I suppose it might be possible for AVA to export a named export Since this doesn't seem actionable any time soon, this can probably be closed. Hopefully future TypeScript releases will make this easier. |
That's not quite right… I appreciate this may be surprising, but so is interrupting the worker processes upon an error. It's a tough balance. I wonder if we're going about this the wrong way. AVA calls these functions assertions, and TypeScript now has an Here's an attempt at a interface Assertions {
discriminate <T, R extends T = T> (value: T, key: keyof T, comparator: R[typeof key]): value is R
} What do you think? |
Building on this, if we were to return booleans for whether an assertion passed, we could turn some of them into type guards. |
@novemberborn that looks very promising! And also agreed on assertions returning booleans, that would make some of my tests much more fluent 💯 |
Revisiting this, I ended up working around it by defining my own wrappers on ava assertions like so:
If that helps anyone :) |
As of TypeScript 3.7, it's now possible to type assertions using the new
asserts
syntax. It would be great if AVA's assertions used this syntax where possible.For example:
This produces the error:
If the
t.true
method had the following signature:The example would not produce an error. With the current types, I have to do something like:
Note also the extra required
return
– this can also be solved with anever
-returning function, as is now the case withprocess.exit()
.The text was updated successfully, but these errors were encountered: