Drop tsd for tests in favor of expect-type

[trevorade]

tsd is a somewhat heavy-weight dependency that is unnecessary considering that expect-type is already a dependency of type-fest.

[trevorade]

For context, this update was written using pattern matching.

For our use case, tsd is infeasible. We saw that this project is already referencing expect-type so saw about rewriting the tests to use that.

[voxpelli]

Considering the fact that @sindresorhus is a (co-)maintainer of tsd I will let him weigh in on this primarily.

[skarab42]

expect-type does not give any information about the type of error but only tells you that there is a problem here. I don't find it very useful compared to tsd.

The example is simple here and it's easy to see what's wrong, but imagine the same message on a more complex type.

FI: vitest-dev/vitest#1954

// @voxpelli

[skarab42]

@voxpelli definitely not enthusiast. The error messages don't describe anything and are more confusing than anything else (or I missed something), see the screen in my previous comment.

[voxpelli]

@skarab42 Absolutely, was thinking you could highlight that as an actual review comment, that's why I added you in, sorry for confusion 🙈

[skarab42]

@skarab42 Absolutely, was thinking you could highlight that as an actual review comment, that's why I added you in, sorry for confusion 🙈

Sorry I don't know how to do this when it's not attached to code.

I don't like how the error messages are produced, but I love the Jest like API. So a few months ago, I have made a plugin for vitest that supports both APIs (tsd & jest like). https://github.com/skarab42/vite-plugin-vitest-typescript-assert We can either use my plugin, but we must to switch to vitest. Or I can make a PR on tsd to support the API a la Jest.

[trevorade]

Hey all!

Yeah, I realized this would be a controversial change. I did see that expect-type was already a dev dep so I figured there was a possibility that this was an eventual intended change.

As for bad error messages in expect-type, I agree. That's pretty straightforward to improve though. I'm working on a PR to improve that situation so that the mismatched args explicitly shows the expected and actual types.

I'll link that from here once it's ready to go.

[trevorade]

Here's the update I'm working on for expect-type:

[trevorade]

Here's the PR for expect-type to improve error messages: mmkal/expect-type#13

[skarab42]

Here's the PR for expect-type to improve error messages: mmkal/expect-type#13

It's still not an error message, what happens when tsc report the error?

At the moment I don't see any advantage in changing the test library, especially if it doesn't bring any benefits.

[skarab42]

I did see that expect-type was already a dev dep

Indeed it was introduced by its author when he added the Get type e40e640. I'm rather surprised that it passed the code review, but there may be a reason... @sindresorhus ?

[mmkal]

Hi all! expect-type author here. I'm a fan of tsd and borrowed from it plenty when writing expect-type - I didn't have any evil plan to sneak it in. expect-type trades off (some) DX for lightweightness, simplicity and most importantly, type-safety. IMO there are also some other DX advantages to compensate like the fluent API, but that's a matter of taste. The reason expect-type was used above was because of a bug that is pretty dangerous, IMO, in tsd, making it not trustworthy for testing generic functions:

tsdjs/tsd#142 (I am also the author of that issue).

I'll dig out the discussion with @sindresorhus around it, can't remember if the main back and forth was on dot-prop or type-fest right now. He did indeed request at first that we use tsd, but the above tsd issue was making tests pass in CI despite there being a bug in the library (which expect-type found - so I do support this change).

Points taken about the CLI error messages though - no doubt tsd is better in that area. My team use CI to catch when there's an issue, then vscode + hover + squinting to see what it is. But yes, it can get difficult to do that when it's complex. I'm excited to see if the above PR can improve them though. Also for those interested, here's a PR relying on the proposed "throw types" typescript feature which would be even better: mmkal/ts#152

[mmkal]

Here's the discussion I was thinking of in dot-prop: sindresorhus/dot-prop#92 (comment)

And here's the suggestion by @sindresorhus to use expect-type instead of tsd in type-fest: #153 (comment) (which I had forgotten was earlier than the dot-prop PR, but the point still stands - it's really, really important that type-fest has correct types, more important than its maintainers having nice error messages in CI, IMO)

As far as I know, the bug is unfixable with tsd's current design.

[skarab42]

@mmkal First of all, thank you for taking the time to answer so precisely. I must admit that I did not look very far in the commit messages.

I didn't have evil plan to sneak it in.

I suspected not, which is why I asked for clarification.

The reason expect-type was used above was because of a bug that is pretty dangerous, IMO, in tsd, making it not trustworthy for testing generic functions: SamVerschueren/tsd#142

This is a very good reason, I was not aware of this bug (which must also be present in my lib).

Also for those interested, here's a PR relying on the proposed "throw types" typescript feature which would be even better: mmkal/ts#152

I've been dreaming about this for a long time 💜

(which I had forgotten was earlier than the dot-prop PR, but the point still stands - it's really, really important that type-fear has correct types, more important than its maintainers having nice error messages in CI, IMO)

That's true, but it's even better to have both ;)

As far as I know, the bug is unfixable with tsd's current design.

I'll look into it as soon as I can.

[trevorade]

At the moment I don't see any advantage in changing the test library, especially if it doesn't bring any benefits.

Libraries with heavy dependencies make them less attractive.

Anecdotally, for us, keeping tsd in sync with tsc is a large cost. We have an engineer whose primary job is to land tsc version updates, which frequently takes on the order of months. Being able to verify that type-fest continues to work correctly when we're working on a tsc update without having to special case anything for type-fest would be of great benefit. Using expect-type, we can use our normal TS build infrastructure for the tests and have the tests be compatible with our tsc update strategy.

Also, having the exact same "tester" for the implementation + tests (tsc) provides a bit more confidence than one for the .d.ts files (tsc) and another for the tests (tsd).

[skarab42]

As far as I know, the bug is unfixable with tsd's current design.

@mmkal Juste made a PR to add a Jest like API to tsd tsdjs/tsd#168 which by the way makes the bug with generics disappear by design.

[skarab42]

Libraries with heavy dependencies make them less attractive.
(...)
Also, having the exact same "tester" for the implementation + tests (tsc) provides a bit more confidence than one for the .d.ts files (tsc) and another for the tests (tsd).

@trevorade I agree, This is why I created a hybrid solution https://github.com/skarab42/unleashed-typescript for my plugin, still not ideal but it avoids maintaining a custom version of TS and it allows to have a version of TS synchronized with your environment.

[trevorade]

Libraries with heavy dependencies make them less attractive.
(...)
Also, having the exact same "tester" for the implementation + tests (tsc) provides a bit more confidence than one for the .d.ts files (tsc) and another for the tests (tsd).

@trevorade I agree, This is why I created a hybrid solution https://github.com/skarab42/unleashed-typescript for my plugin, still not ideal but it avoids maintaining a custom version of TS and it allows to have a version of TS synchronized with your environment.

@skarab42 That's pretty cool. My only concern with adopting such an approach is that it appears to run the risk of an unstable API where the @internal parts of tsc can change between versions without constituting a breaking change but would still break usages of unleashed-typescript which relied on the @internal api remaining constant between version.

[trevorade]

Another naming option could be hyrums-law-typescript :)

https://www.hyrumslaw.com/

[trevorade]

Gonna go ahead and close this for now. I can recreate it in the future if there is a consensus that type-fest wants this.

[voxpelli]

@trevorade I personally would be in favor of picking up this discussion again, to get the canary tests working, especially as they are now part of the TS canary tests

(I reopened this thinking it was an issue, not a PR, feel free to open a new issue / PR and ping me as the one who requested it)