Skip to content
This repository has been archived by the owner on Nov 16, 2023. It is now read-only.

Validating correctness of typings for libraries written in typescript. #249

Closed
bradfordlemley opened this issue Sep 5, 2019 · 7 comments
Labels

Comments

@bradfordlemley
Copy link

From the readme:

If you are writing the library in TypeScript, don't use dtslint. Use --declaration to have type definitions generated for you.

Aren't $ExpectError assertions generally necessary to validate correctness of types?

If dtslint is the only tool that honors $ExpectError, shouldn't libraries that are written in TypeScript also be using dtslint to validate that their types are correct?

Or is there a way to do that without dtslint?

@sandersn
Copy link
Member

sandersn commented Sep 5, 2019

For lighter-weight checks, try tsd, which works within the type system, giving you a library you can import into existing unit tests. It's also more usable and gives you feedback as you type.

For more comprehensive checks, I would recommend that you check in baselines of your .d.ts and write a unit test that asserts the newly generated .d.ts file is equal to it. That way you'll keep on top of all your API changes, not just those that result in a different return type.

@bradfordlemley
Copy link
Author

You mean tsc, right? I am using tsc, and it does validate typings to some extent, but I'm looking for a way to validate that the typings result in type errors appropriately, too, (ie. via $ExpectError assertions), and I don't think it's possible with tsc, is it? (Would be awesome if it was!)

Validating typings via $ExpectError assertions is common practice for DefinitelyTyped definitions, and it seems like it should be encouraged to ensure quality type definitions for libraries written in TypeScript, too, no? Without $ExpectError, it's a very tedious manual process to ensure typings give type errors appropriately. (The typings for something like react-redux, for example, are quite complex and wouldn't be sufficiently validated without $ExpectError assertions.)

Making sure the type definitions don't change isn't an appropriate solution IMO, as it still requires a tedious untrackable manual process to ensure correctness in the beginning and also every time the typings are updated in any way. Am I missing something?

@sandersn
Copy link
Member

sandersn commented Sep 5, 2019

sorry, I thought I put a link in my reply. https://github.com/SamVerschueren/tsd#readme

tsd is (1) a package that provides expectType and expectError assertions (2) a command-line utility that can evaluate those expectError calls. It basically wraps tsc.

As for the big picture, I guess I don't understand two things:

  1. Why you need to write tests that assert type errors happen for certain uses. The compiler will prevent those uses from running, so you shouldn't need to worry about them.
  2. Why a baseline .d.ts is initially untrackable and manual, compared to a series of unit tests. The type syntax of the d.ts file seems more compact and easier to compare than tests expressed using code. Additions will fail in exactly the places where you'd want to write a unit test for them.

@ersimont
Copy link

ersimont commented Sep 7, 2019

I can chime in here. I agree w/ the OP.

Why you need to write tests that assert type errors happen for certain uses. The compiler will prevent those uses from running, so you shouldn't need to worry about them.

This won't help if the typing is too permissive. I.e. I want to validate that the compiler will actually prevent certain uses. Otherwise a type that is too wide can slip past the tests. E.g. fn<T>() could slip by when what I really wanted was fn<T extends any[]>().

tsd has the same problem, which is why I'm using dtslint. It looks there is an open PR to address this, but it has been open for 5 months, so I'm not sure.

Why a baseline .d.ts is initially untrackable and manual, compared to a series of unit tests.

This doesn't help verify that typings are correct, it only prompts you to look at them carefully. You may still make a mistake. Especially for complex overloads, this simply doesn't help, the same way looking at diff of your code doesn't help verify that the code changed correctly. We use tests for that.

@bradfordlemley
Copy link
Author

It should be a given that type correctness must be verified via failure cases. The value of a type system is really in the failures it produces. This seems to be recognized by DefinitelyTyped and FlowTyped -- both repos have thousands of ExpectError assertions, and FlowTyped even requires at least one ExpectError for type definitions.

Even if a library itself, and its tests, are written in TypeScript, it seems like the generated type definitions should still be verified via failure cases. But that seems not to be standard, and I don't understand why. Maybe I'm missing something. Or it could be just that the tooling and docs don't support or encourage it yet.

At any rate, I got to this point because I'm not able to trust my own libraries' type definitions without verifying them via failure cases, and I came looking for a way to do that. What I've found is that both tsd and dtslint can do it, tsdjs/tsd#10 tsd discusses the differences between the tools. Neither tool really fits nicely into to my workflow, tho, as they both require a processing outside normal unit test flow, and the processing is inefficient because it duplicates type checking/processing.

I'm currently using ts-jest because it honors type failures as tests are running in watch mode. I think I'm going to try to get something added there to support ExpectError. I think it could be done in ts-jest without native tsc support, but it would be even better to support something like @ts-expect-error in tsc.

There is an existing issue for adding @ts-expect-error to tsc that seems to be accepted and ready for a PR. I'll leave all the commentary above, but close out this issue as I think @ts-expect-error in tsc is the feature I came looking for.

@SamVerschueren
Copy link

@ersimont FYI, tsd is now strict in type assertions.

Let's assume a function fn<T>() which returns T.

expectType<string | number>(fn<string>());
//=> Parameter type `string | number` is declared too wide for argument type `string`.

expectType<Observable<string | number> | Observable<string | number | boolean>>(
	fn<Observable<string | number> | Observable<string>>()
);
//=> Parameter type `Observable<string | number> | Observable<string | number | boolean>` is declared too wide for argument type `Observable<string | number> | Observable<string>`.

Someone brought to my attention that any was still slipping through because any can be assigned to anything and anything can be assigned to any. This was fixed this morning and will be available in the next release.

expectType<string | number>(fn<any>());
//=> Parameter type `string | number` is not identical to argument type `any`.

Hope this solves most of the issues.

I'm working on more assertions as well. So feel free to stop by the issue tracker to help sort out some of the discussions :).

@bbugh
Copy link

bbugh commented Dec 20, 2019

FYI anyone who ends up at this issue looking for an answer to the title, neither dtslint nor tsd are it. They do not support asserting on typescript source files.

The best possibility is here: microsoft/TypeScript#29394, adding a @ts-expect-error comment to the compiler.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

5 participants