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

Run tsc on all test files #837

Closed
sindresorhus opened this issue Mar 20, 2024 · 26 comments
Closed

Run tsc on all test files #837

sindresorhus opened this issue Mar 20, 2024 · 26 comments
Labels
help wanted Extra attention is needed

Comments

@sindresorhus
Copy link
Owner

sindresorhus commented Mar 20, 2024

We should run tsc on all the test files.

Basically, remove this:

type-fest/tsconfig.json

Lines 13 to 15 in 14fb192

"exclude": [
"test-d/**/*"
]

And then fix the reported issues.

Upvote & Fund

  • We're using Polar.sh so you can upvote and help fund this issue.
  • The funding will be given to active contributors.
  • Thank you in advance for helping prioritize & fund our backlog.
Fund with Polar
@voxpelli
Copy link
Collaborator

Does tsd support this or do we need to move to eg expect-type?

@Emiyaaaaa
Copy link
Collaborator

Emiyaaaaa commented Mar 20, 2024

move to eg expect-type

If move to expect-type, we need to use tsc to "run" expect-type, so we also need to fix the reported issues in tsc.

So whatever support tsc is first step, then we can choose "move to expect-type" or "use them both"

I think tsd don't use built-in ts version is better both for type-fest and tsd, but this depends on the tsd code implementation and @sindresorhus

@voxpelli
Copy link
Collaborator

@Emiyaaaaa The helpers from tsd needs to be used through tsd I believe, using them through tsc isn’t an option I believe?

@mrazauskas
Copy link

Simply use TSTyche: https://tstyche.org

@trevorade
Copy link
Contributor

trevorade commented Mar 20, 2024

For some additional background and potentially relevant previous discussions with regards to using expect-type, see the closed PR: #499

@voxpelli
Copy link
Collaborator

@trevorade Do you have a fork of type-fest where you are using expect-type instead? I started looking into this issue now and I think that's what we need to do here and if you have already done it, then yay :)

@trevorade
Copy link
Contributor

trevorade commented Mar 20, 2024

@voxpelli So I work at Google. We have a repository of OSS libraries we depend on including type-fest. Without going into specifics (not because they're super proprietary but just complex), running tsd is a non-starter for us. In our input configuration, we perform some text transformations using regular expression powered replacements to rewrite the tsd expectations.

At the moment, we don't actually use expect-type. Internally, we authored a bare bones type checking test library. I will say that the error tsc messages for our library are bad. It just errors due to parameter count mismatches and does not output anything helpful about the types. I'm not sure about the state of expect-type these days but I imagine tsd error messages are still better FWIW.

To be clear, I'm still in support of moving off of tsd because, for us, we'd love to be able to just compile the type-fest tests as-is without needing to do any transformations. We'd trade away better error messages for tsc compatibility.

@trevorade
Copy link
Contributor

Also, we decided on using a bare bones type verification lib rather than expect-type because we were waiting on expect-type to stabilize before adoption (e.g. hitting a major 1.0 version)

@mrazauskas
Copy link

Simply use TSTyche: https://tstyche.org

To draw your attention, TSTyche is a type test runner and its latest version is 1.1.0. Stable release. Why so?


I was maintaining forked tsd-lite for more than two years. First I just wanted to have this API tsdjs/tsd#75. It is considered since year 2020, but there is no progress. Next I fixed all bugs mentioned in issues in tsd repo. tsd-lite did not had any of them.

The goal was to run more than 1000 type assertions in a large monorepo. It was fun to make tsd-lite work, but there was no way to organise the test and debugging was very clumsy.

I went own writing a type test runner from scratch. This is TSTyche.

It has test() and describe(). Supports .only, .skip. Pure gold for large scale project. Also it can run type test on several version of TypeScript: tstyche --target 4.8,current. The install size is under 200 kb. This is because by default TSTyche loads currently installed TypeScript.


expect-type is great library. It's . toEqualTypeOf() matcher is well implemented and tested. But others have just few tests and there are problems with handling any and never (see mmkal/expect-type#64, mmkal/expect-type#65 (comment)).

TSTyche ships with expect style assertions, but it compares type programmatically similar to tsd. While developing assertions I was checking their behaviour in expect-type. That’s how I found the bugs. TSTyche does not have these. It is very well tested and documented.

I wanted a tool which is lightweight, easy to start working and which can work at any scale. In comparison expect-type is really great for a small project, but try checking just one file using tsc. Or testing agains specific version of TypeScript. While developing new features or debugging something this is rather essential.


Thanks for reading. (By the way, 2.0.0-beta will ship in a couple of weeks with --watch mode.)

@trevorade
Copy link
Contributor

@mrazauskas Thx for sharing :) TSTyche looks pretty cool.

For Google, I think we'd still prefer a tsc based approach like expect-type or similar. The main reason for this is that internally setting up and building TSTyche would be quite a bit more work than the single source file of expect-type (to briefly touch on why this is hard for us, it mostly boils down to having a customized TS compilation setup, being on a mono repo, and not being able to use npm or yarn in our build chain. Internally, we build using blaze which is close to https://bazel.build)

type-fest is great because its source is just a bunch of .d.ts files that can be directly consumed. The source itself has no dependencies. For testing type-fest, you get into needing additional dev dependencies like tsd. It would be great to keep type-fest as flat as possible so that verifying correctness in tests is as unencumbered as the library's sources are.

I suppose what I'd like to see is a type assertion library that can be directly used with tsc providing accurate type verification but low debuggability and an optional CLI that can process the test files and generate better type error messages. For Google, we're not modifying the sources of type-fest but we still want the tests to detect tsc upgrade incompatibilities (hence the bugs I filed recently) so debuggability is not a priority.

We're just one voice though so I'm supportive of whatever this repo goes with. If type-fest does opt to go with TSTyche, we'll likely transform the assertions as we are today.

@voxpelli
Copy link
Collaborator

And here's why tsd currently comes with its own typescript: tsdjs/typescript#1

My point of view:

  • using tsc should be enough to know if tests passes or not
  • a separate runner could optionally be used to gather better diagnostics info on why a fail happen (as well as to eg. gather test coverage)

@trevorade
Copy link
Contributor

I really wish the TypeScript team would just implement this FR: microsoft/TypeScript#27024

Would make this so much easier.

@mrazauskas
Copy link

Hm.. Not sure I follow. Checking a file with tsc or tstyche will show the same output. Try it.

tsc compares types programatically. tstyche and tsd are using the same APIs as tsc. expect-type is different solution. It does not compare types exactly like tsc does. For instance, subtype checks (or .toMatch) are exactly the same between tstyche and tsc, but not in expect-type (anys are not handled).

The main difference: tsc checks a project defined by tsconfig.json and tstyche checks a single file (plus all what is needed to understand that file including reference etc.)

In testing JavaScript it feels natural to be able to run just one file or a single test(). This is what tstyche does. It is like tsc for testing types.

tstyche has zero dependencies. Its install size is around 200 kb only. Not the size of library, but the full install size.

@voxpelli
Copy link
Collaborator

voxpelli commented Mar 20, 2024

@mrazauskas I think neither me or @trevorade has looked into tstyche deeply. If it does fulfill my wishes of #837 (comment), then that sounds great!

@mrazauskas
Copy link

mrazauskas commented Mar 20, 2024

@voxpelli To be honest, I am puzzled with the requirements in that comment. If running tsc should be enough, why it is important that tsd comes with its own typescript or not? I mean, why would you need tsd if tsc should be enough?

@tsd/typescript is hard patch. TSTyche does patch typescript in similar way, but this is done in memory. It’s a soft patch. Nothing is written to disk. This is why already installed TypeScript can be used. Any version of TypeScript can be used. Even those build by every-ts. Meaning you can use every-ts bisect with TSTyche to find which commit in TypeScript repo broke something. It can be test file, or just any .ts file.

Sorry. I drifted into details again. Could I ask to clarify the requirements, please?

  • I explain regarding @tsd/typescript
  • you can use TSTyche just like tsc. On any file (including .svelte and similar). TSTyche reports the same type errors like tsc does. Hence it can be used with expect-type assertions as well. It ships its own assertions, which have better error messages.
  • coverage? That’s interesting topic. First we have to agree what coverage is in this context.

Just for fun, here is test run output with error messages (they will be improved, this is just v1):

Screenshot 2024-03-20 at 21 30 30

@voxpelli
Copy link
Collaborator

To be honest, I am puzzled with the requirements in that comment. If running tsc should be enough, why it is important that tsd comes with its own typescript or not?

The comment on why tsd ships its own typescript was just a comment I made in the passing to give context on why tsd has opted for that solution.

My point of view is that we shouldn't be using tsd but instead:

  • using tsc should be enough to know if tests passes or not
  • a separate runner could be an optional extra if that gives better diagnostics info on why a fail happen (or eg. gathers test coverage) but it shouldn't be required

@mrazauskas
Copy link

mrazauskas commented Mar 20, 2024

Thanks. If you think it is enough to use tsc, go for it.

From my understanding, a type test runner does much more. For instance, tsc is not able to check if types are identical. expect-type provides assertions based on generic types, which can make it look that tsc can replace a type test runner. It cannot. Mostly because generics compare types indirectly. This is error prone (as I already mentioned) and also slow. Compiler has to resolve generic type for each assertion instead of comparing types directly. Benchmark proves that comparing types via TypeScript APIs is faster (needles to say that these checks are also 100% correct).

A type test runner provides .not assertions. Inverted assertions are way better than using // @ts-expect-error, because the comment can hide missing imports, or renamed properties, or spelling mistakes, or anything else including syntax issues. And tsc will stay silent, but .not will always shout.

Edit: here is real world example from reduxjs/redux-toolkit repo where // @ts-expect-error is hiding missing import in a type test file. Simple copy and paste to another file introduced the mistake.

There are more benefits which I already mentioned. It is not like I am trying to convince someone. To be honest, my first note on TSTyche was so short, because I did not believe it could be ever adopted by type-fest. I wrote some longer text, just because @trevorade mentioned stable releases. I simply like writing / thinking about type testing. That’s fun.

@Emiyaaaaa
Copy link
Collaborator

Emiyaaaaa commented Mar 21, 2024

using them through tsc isn’t an option I believe?

Hi @voxpelli , sorry I'm a little confused, you mean there is no way to run tsd throuth tsc?

What I mean is:
I think rewrite all the test cases is a huge work (Suppose we decide to do this), before this work done, we should ensure as quickly as possible that expect-type or other type test library witch base on tsc could be run without error from current test cases.

update:
Of course, we can also run tsc on single test file by npm script, but I think this way is complicated

@voxpelli
Copy link
Collaborator

Right now not even latest tsd passes so first step is to make sure our current tests are actually passing 😝 #839

@Emiyaaaaa
Copy link
Collaborator

Right now not even latest tsd passes so first step is to make sure our current tests are actually passing 😝 #839

I noticed tsd update @tsd/typescript from 5.0.1 to 5.3.3, this could be why the test failed.

And If we update tsd then fix that issue base [email protected], we still can't be sure that issue will be ok on 5.0.1 or other version.

so, we also need a type test library that doesn't depend on the ts version, It looks like the existing libraries are all based on tsc or similar to tsc

@mrazauskas
Copy link

mrazauskas commented Mar 21, 2024

so, we also need a type test library that doesn't depend on the ts version, It looks like the existing libraries are all based on tsc or similar to tsc

TSTyche is fully independent from the installed version of tsc. You can run tstyche --target 4.9,5.3.2 and it will test against TypeScript 4.9.5 and 5.3.2. Or you can run a daily cron job in CI: tstyche --target next to test agains nightly releases.

(I do understand this is not relevant. Sorry about the noise. Feel free to hide my comments.)

@trevorade
Copy link
Contributor

I think rewrite all the test cases is a huge work (Suppose we decide to do this), before this work done, we should ensure as quickly as possible that expect-type or other type test library witch base on tsc could be run without error from current test cases.

@Emiyaaaaa We're transforming all these tests already in our copy of type-fest. We're using a library similar to expect-type and it's working pretty well. I'd be happy to contribute the initial rewrites to whatever library we decide on. I think we'll also want handwritten changes to remove the variables and expressions that are currently required to pass to the tsd assertion functions.

@voxpelli
Copy link
Collaborator

@trevorade PR would be very welcome 🙏 I guess we should fix #839 first, did you get a chance to look at the failures there?

@trevorade
Copy link
Contributor

@voxpelli I haven't looked at the failures over there, no. I'm assuming you're seeing #831 and #833?

Just let me know which library you decide on and I can help create the initial rewrite. I can't promise that what I produce will fully work. More that I can produce a starting point for others to iterate on.

@voxpelli
Copy link
Collaborator

@trevorade Exactly, #833, I haven't looked into why it fails, but migrating to a new testing approach while current tests are failing will be hard :)

@fregante
Copy link
Collaborator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

6 participants