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

fix(expect, @jest/expect): do not infer argument types of *CalledWith and *ReturnedWith matchers #13339

Merged
merged 4 commits into from
Sep 30, 2022
Merged

fix(expect, @jest/expect): do not infer argument types of *CalledWith and *ReturnedWith matchers #13339

merged 4 commits into from
Sep 30, 2022

Conversation

mrazauskas
Copy link
Contributor

@mrazauskas mrazauskas commented Sep 29, 2022

Fixes #13337

Summary

Unfortunately #13268 and #13278 have to be reverted. As reported, type inference does not work with function overloads. I have to add that it also does not work with asymmetric matchers.


The asymmetric matchers return AsymmetricMatcher type. To make the inference work with an argument of [a: string] type, it is needed to remap it into [a: string | AsymmetricMatcher]. Could be possible with something like:

type Remapped<T> = T | AsymmetricMatcher

A tuple should be broken into elements and turned into remapped tuple. And this is the hard part:

type InferTest<T extends [...Array<unknown>]> = T extends [infer H] ? H : never;

InferTest<[a: number]>; // this is `number`
InferTest<[b?: number]>; // but this is `never`!!!

The optional args are simply discarded during inference. That is because TypeScript assumes that the optional args are simply not there. It could be there or it could not in a tuple type, but if a type of tuple member is turned into a type of a variable? There isn’t such a thing as an optional variable. That’s why the Remapped type has no chance to work.


I couldn’t find a solution. Perhaps it makes sense to revert it for now, because types are obviously buggy.

Test plan

Type tests with asymmetric matchers are added for the future. These are failing on main.

@mrazauskas mrazauskas changed the title fix(expect): do not infer argument types of *CalledWith and *ReturnedWith matchers fix(expect, @jest/expect): do not infer argument types of *CalledWith and *ReturnedWith matchers Sep 29, 2022
@@ -214,55 +214,65 @@ expectError(expect(jest.fn()).toHaveBeenCalledTimes());
expectType<void>(expect(jest.fn()).toBeCalledWith());
expectType<void>(expect(jest.fn()).toBeCalledWith('value'));
expectType<void>(expect(jest.fn()).toBeCalledWith('value', 123));
expectType<void>(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great you've added tests that catch this 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was sure you wouldn’t accept this PR without these tests (;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😀

@SimenB SimenB merged commit be0d4d1 into jestjs:main Sep 30, 2022
@mrazauskas mrazauskas deleted the fix-revert-infered-types branch September 30, 2022 06:49
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 31, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Type inferrence in toBeCalledWith broken for method with overloads
3 participants