feat: improve expectTypeOf error messages #4206

mmkal · 2023-10-01T20:08:46Z

Description

Update expect-type version to the prerelease from mmkal/expect-type#16 - it should make error messages much better. This is hard to see in a repo where the assertions are passing, but see the snapshot update in that PR. (Note: the title is "improve expectTypeOf error messages" rather than referencing the update, since that's the net effect for end-users).

Here are what the examples of failing-test.d.ts in this repo look like:

Before:

After:

Related to #1954 - @sheremet-va FYI. I don't think it closes it as that issue seems. to be more about how it fits into the vitest API/usage generally.

I also linked to my library in the readme.

~~I've opened this as a draft as I'd like to publish v0.17.0 (rather than under the next dist-tag). I might actually bump to v1.0.0 too, I'm thinking of this as a release candidate (candidate).~~

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

It's really useful if your PR references an issue where it is discussed ahead of time. If the feature is substantial or introduces breaking changes without a discussion, PR might be closed.
Ideally, include a test that fails without this PR but passes with it. (There is one fix in Improve CLI error messages mmkal/expect-type#16 - expectTypeOf<any>().toBeBoolean() now produces an error, but I don't think that test needs to be duplicated here? I've checked this checkbox because I updated the error message snapshot tests.)
~~- [ ] Please, don't make changes to pnpm-lock.yaml unless you introduce a new test example. (err I needed to update it because I was updating a dependency? Should I delete this item?)~~

Tests

Run the tests with pnpm test:ci (also updated test/typescript/test/runner.test.ts and related snapshots to reflect the new error messages)

Note: test/coverage-test failed with the below error but I think it's unrelated? _Edit: Yep, I got the same error on main_

Also node.spec.ts and math.ts seemed to have a newline added to them for some reason. Not sure what that's about.

Documentation

If you introduce new functionality, document it. You can run documentation with pnpm run docs command. (_I haven't added docs, but I wrote lots in the pull request on expect-type. Is there a way to copy them here automatically and keep them in sync? Should I copy paste?)

Changesets

Changes in changelog are generated from PR name. Please, make sure that it explains your changes in an understandable manner. Please, prefix changeset messages with feat:, fix:, perf:, docs:, or chore:.

netlify · 2023-10-01T20:08:51Z

✅ Deploy Preview for fastidious-cascaron-4ded94 ready!

Name	Link
🔨 Latest commit	`e308e6b`
🔍 Latest deploy log	https://app.netlify.com/sites/fastidious-cascaron-4ded94/deploys/654e6a988b50d2000825fe1c
😎 Deploy Preview	https://deploy-preview-4206--fastidious-cascaron-4ded94.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

sheremet-va · 2023-10-02T08:14:22Z

I also linked to my library in the readme.

You can also add a link in the docs!

I don't think it closes it as that issue seems.

Yeah, it doesn't close the issue yet as there are other improvements that we need to make for typechecking.

Overall, PR looks good to me 👍🏻

mmkal · 2023-10-02T13:38:54Z

Oh I should update these docs too: https://vitest.dev/guide/testing-types.html#reading-errors

mmkal · 2023-10-03T01:33:11Z

@sheremet-va ok, added more docs re the error messages: https://github.com/vitest-dev/vitest/pull/4206/files#diff-58c52997981aa78c64ba261e445d5f5866f73205518e556a06beaa2d11fbcbff

I also updated typechecker.ts to put the type error on a single line so the useful information is as prominent as possible: https://github.com/vitest-dev/vitest/pull/4206/files#diff-02bca40f9c80e61170f4d69ec1aa9f68ff87c5ba3f0e49f7b5ab760362c9e69d

Here's the snapshot result: https://github.com/vitest-dev/vitest/pull/4206/files#diff-86e5c8d303f3cc24cb69cd88757cd83b4076c02f5d4c6b36d2f42792a1808d99

I don't know why the builds are red, but I don't think it's related to this pull request.

Fixes #17 Closes #36 (by documenting that `toMatchTypeOf` ~= `toExtend`) Closes #37 (by documenting that helper types are not part of the API surface) Closes #32 Closes #4 Closes #6 (by documenting that you _can't_ use `.not.toBeCallableWith(...)`) Closes #38 Use generics to give better error messages than "Arguments for the rest parameter 'MISMATCH' were not provided" for most `toEqualTypeOf` cases and many `toMatchTypeOf` cases. This trades off some implementation complexity for better end-user error messages. Essentially, write a special `<Expected extends ...>` constraint for each overload of each method, which does some crazy conditional logic, which boil down to: - `<Expected extends Actual>` for `toEqualTypeOf` (when we aren't in a `.not` context) - `<Expected extends Satisfies<Actual>>` for `toMatchTypeOf` Anyone interested, have a look at the snapshot updates in `errors.test.ts.snap` to see what the real world difference is. Each of these constraints apply only when we know it's going to "fail" - i.e. `Satisfies<...>` is a fairly meaningless helper type that is used to try to show errors at the type-constraint level rather than the `...MISMATCH: MismatchArgs<...>` level which won't give good error messages. When using `.not`, the constraint just becomes `extends unknown`, and you'll have to squint as before. See also: #14 for the better long-term solution, _if_ the TypeScript team decide to merge the throw types PR. See also: #13 for a hopefully-complementary improvement to the information on hover, which will improve the cases this doesn't cover. TODO: - [x] See if the `expectTypeOf({a: 1}).toMatchTypeOf({a: 'one'})` case can also be improved. - [x] Document. The constraints are a bit different to what most users would be used to, so it's worth highlighting the best way to read error messages and clarify when they might default to "Arguments for the rest parameter 'MISMATCH' were not provided" Note: I have publish v0.17.0-1 based on this PR and will hopefully be able to use [that version in vitest](vitest-dev/vitest#4206) as a test before merging.

mmkal · 2023-10-05T20:24:56Z

@sheremet-va loved the talk(!). I could do with some help with the pnpm lockfile when you get a minute. I ran pnpm install but CI doesn't seem to like what it generated.

sheremet-va · 2023-10-06T07:17:52Z

I ran pnpm install but CI doesn't seem to like what it generated.

I think you have the older version installed. I run the command for you, should be fine now.

sheremet-va · 2023-11-05T11:05:19Z

Do you think we can have this in Vitest 1.0?

mmkal · 2023-11-05T14:10:52Z

Are you saying you'd rather hold off on this change until vitest v1.0.0? If so, let me know if there's a alternate branch I should be comparing to, or what I should do.

sheremet-va · 2023-11-05T14:26:36Z

Are you saying you'd rather hold off on this change until vitest v1.0.0? If so, let me know if there's a alternate branch I should be comparing to, or what I should do.

I asked when do you think this can be merged?

mmkal · 2023-11-05T16:09:43Z

Whenever it's green. I'll take another look today or tomorrow to see why it started failing. What's the timeline for v1?

sheremet-va · 2023-11-06T09:29:06Z

I'll take another look today or tomorrow to see why it started failing.

Thank you.

What's the timeline for v1?

This month.

It can always be merged later, I just wanted to know when you think this is ready since it's still in draft.

mmkal · 2023-11-06T14:54:47Z

@sheremet-va now passing on all but windows. Looking at the job I don't see what the failure actually is, the test I affected seemed to have passed, but I just pushed a change to account for potential \r\n line endings on Windows just in case. I'm using a Mac to develop so that's pretty much all I can do I think.

Ready for review at this point, would be great to get any help getting it over the line.

sheremet-va · 2023-11-06T15:48:22Z

Ready for review at this point, would be great to get any help getting it over the line.

CI is in a poor state currently, so if your tests are working fine you shouldn't worry. From what I can see here, all the issues are also present on main.

mmkal · 2023-11-07T15:39:46Z

Ready for review at this point, would be great to get any help getting it over the line.

CI is in a poor state currently, so if your tests are working fine you shouldn't worry. From what I can see here, all the issues are also present on main.

@sheremet-va in that case, feel free to merge. If any issues arise I will be able to help, but I'd like to get this in ASAP to avoid future merge conflicts.

mmkal mentioned this pull request Oct 2, 2023

Improve CLI error messages mmkal/expect-type#16

Merged

2 tasks

mmkal mentioned this pull request Oct 2, 2023

expectTypeOf allows an incorrect any value instead of the specified type mmkal/expect-type#38

Closed

kwangure mentioned this pull request Oct 3, 2023

expectTypeOf allows an incorrect any value instead of the specified type #4205

Open

6 tasks

sheremet-va approved these changes Oct 3, 2023

View reviewed changes

sheremet-va force-pushed the expect-type-0.17.0-1 branch from 979ac96 to 9f82401 Compare October 6, 2023 07:17

sheremet-va marked this pull request as ready for review October 7, 2023 08:10

sheremet-va marked this pull request as draft October 7, 2023 08:10

mmkal marked this pull request as ready for review November 6, 2023 14:54

mmkal added 7 commits November 10, 2023 18:36

udpate expect-type

7163726

update test snapshot

933ff37

another way of snapshotting?

3100ff9

lockfile again

2ee3892

lockfile

fc16b20

copy expect-type docs

df96e96

run pnpm i again

9d82bf1

mmkal deleted the expect-type-0.17.0-1 branch March 10, 2024 21:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: improve expectTypeOf error messages #4206

feat: improve expectTypeOf error messages #4206

mmkal commented Oct 1, 2023 •

edited

netlify bot commented Oct 1, 2023 •

edited

sheremet-va commented Oct 2, 2023

mmkal commented Oct 2, 2023

mmkal commented Oct 3, 2023

mmkal commented Oct 5, 2023

sheremet-va commented Oct 6, 2023

sheremet-va commented Nov 5, 2023

mmkal commented Nov 5, 2023

sheremet-va commented Nov 5, 2023

mmkal commented Nov 5, 2023

sheremet-va commented Nov 6, 2023

mmkal commented Nov 6, 2023 •

edited

sheremet-va commented Nov 6, 2023

mmkal commented Nov 7, 2023 •

edited

feat: improve expectTypeOf error messages #4206

feat: improve expectTypeOf error messages #4206

Conversation

mmkal commented Oct 1, 2023 • edited

Description

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

Tests

Documentation

Changesets

netlify bot commented Oct 1, 2023 • edited

✅ Deploy Preview for fastidious-cascaron-4ded94 ready!

sheremet-va commented Oct 2, 2023

mmkal commented Oct 2, 2023

mmkal commented Oct 3, 2023

mmkal commented Oct 5, 2023

sheremet-va commented Oct 6, 2023

sheremet-va commented Nov 5, 2023

mmkal commented Nov 5, 2023

sheremet-va commented Nov 5, 2023

mmkal commented Nov 5, 2023

sheremet-va commented Nov 6, 2023

mmkal commented Nov 6, 2023 • edited

sheremet-va commented Nov 6, 2023

mmkal commented Nov 7, 2023 • edited

mmkal commented Oct 1, 2023 •

edited

netlify bot commented Oct 1, 2023 •

edited

mmkal commented Nov 6, 2023 •

edited

mmkal commented Nov 7, 2023 •

edited