feat(expect, @jest/expect): infer type of `*ReturnedWith` matchers argument by mrazauskas · Pull Request #13278 · jestjs/jest

mrazauskas · 2022-09-18T13:09:04Z

Summary

Similar to #13268, but *ReturnedWith matchers. I have added logic to infer types of *ReturnedWith matchers argument from the type of received expression.

Test plan

Type tests added.

mrazauskas · 2022-09-18T13:13:23Z

@royhadad What you think about this approach? I create this PR just to illustrate the idea of restricting type of the received expression. It is not restricted directly, but TS compiler (or IDE) would show an error if a user would do this:

expect(123).toHaveLastReturnedWith(123);

Only function can be passed as received. Curious about your opinion: any down sides?

mrazauskas · 2022-09-18T13:15:09Z

 expectError(expect(jest.fn()).nthReturnedWith());
-expectError(expect(jest.fn()).nthReturnedWith(2));
+
 expectType<void>(expect(jest.fn()).toHaveNthReturnedWith(1, 'value'));


It works with () => unknown just like before. Infers return value, if there is one. Errors if received is not a function.

royhadad · 2022-09-18T13:17:00Z

   * Ensure that the last call to a mock function has returned a specified value.
   */
-  lastReturnedWith(expected: unknown): R;
+  lastReturnedWith<U extends EnsureFunctionLike<T>>(expected: ReturnType<U>): R;


you are a wizard :)
I'll add this to my PR as well

Thanks (; Glad you liked it. So much easier to explain with an example.

royhadad · 2022-09-18T13:38:26Z

@royhadad What you think about this approach? I create this PR just to illustrate the idea of restricting type of the received expression. It is not restricted directly, but TS compiler (or IDE) would show an error if a user would do this:
expect(123).toHaveLastReturnedWith(123);
Only function can be passed as received. Curious about your opinion: any down sides?

@mrazauskas
I see just one downside, it is a bit less backward compatible than the other approach (which is also not totally backward compatible)
Therefore I think your solution is better.

royhadad · 2022-09-18T13:40:47Z

   * Ensure that a mock function has returned a specified value at least once.
   */
-  toReturnWith(expected: unknown): R;
+  toReturnWith<U extends EnsureFunctionLike<T>>(expected: ReturnType<U>): R;


Maybe change to this syntax? it works and causes less indirection

Suggested change

toReturnWith<U extends EnsureFunctionLike<T>>(expected: ReturnType<U>): R;

toReturnWith(expected: ReturnType<EnsureFunctionLike<T>>): R;

Thanks. That’s good idea. I was hoping that type argument could allow something like: expect(123).toHaveReturnedWith<() => number>(123). But it doesn’t.

royhadad · 2022-09-18T14:09:54Z


-type M = Matchers<void>;
+type M = Matchers<void, unknown>;
+type N = Matchers<void>;


what is this N for? 🤔

Just three test – takes two type args, but allows passing juts one and (below) errors if type args are missing.

Some time ago I added second arg without default. That was a breaking change and a user raised an issue. This test was added back in these days, but became unnecessary then @jest/expect package was introduced. Now it is needed again.

One day this will be wrapped with nice test("required type arguments", () => { /.... Good enough for now (;

mrazauskas · 2022-09-18T14:22:52Z

 };

-export interface Matchers<R extends void | Promise<void>> {
+type EnsureFunctionLike<T> = T extends (...args: any) => any ? T : never;


Unfortunately anys are necessary for toHaveBeenCalled* matchers. See #13268 (comment)

SimenB

awesome stuff!

mrazauskas · 2022-09-20T06:33:01Z

Thanks! I was wondering if toBe should be typed in similar fashion. I mean, something like:

<T>expect(actual: T).toBe(expected: T)

Simple thing. Does it make sense?

royhadad · 2022-09-20T06:57:52Z

Thanks! I was wondering if toBe should be typed in similar fashion. I mean, something like:
<T>expect(actual: T).toBe(expected: T)
Simple thing. Does it make sense?

Seems about right. For weird edge cases the user can always pass any.
I see a lot of matchers where it's possible to replace the expected unknown with something more accurate

/packages/expect/src/types.ts

  toBe(expected: unknown): R;
  toContain(expected: unknown): R;
  toContainEqual(expected: unknown): R;
  toEqual(expected: unknown): R;
  toMatchObject(
    expected: Record<string, unknown> | Array<Record<string, unknown>>,
  ): R;
  toStrictEqual(expected: unknown): R;

mrazauskas · 2022-09-20T07:14:54Z

Just wanted to say that some of these are allowed to take asymmetric type matchers and that can be complicated. Quick look at the docs, there is also an example with toBeCalledWith and asymmetric matchers (added types for testing):

class Cat {}
function getCat(fn: (c: Cat) => void) {
  return fn(new Cat());
}

{
  const mock = jest.fn<(c: Cat) => void>();
  getCat(mock);
  expect(mock).toBeCalledWith(expect.any(Cat));
}

function randomCall(fn: (n: number) => void) {
  return fn(Math.floor(Math.random() * 6 + 1));
}

{
  const mock = jest.fn<(n: number) => void>();
  randomCall(mock);
  expect(mock).toBeCalledWith(expect.any(Number));
}

First example works, but second one errors: "Argument of type 'AsymmetricMatcher_2' is not assignable to parameter of type 'number'." Ups.. That’s a regression.

mrazauskas · 2022-09-21T13:37:39Z

Same issue with *ReturnWith. Asymmetric matchers can be used, but currently TS complains about the following snipped:

expect(jest.fn<() => {one: string; two: string}>()).toHaveReturnedWith({
  one: expect.stringContaining('a'),
  two: expect.stringContaining('x'),
});

I was trying to see if it is possible to fix toBeCalledWith. Seems to be complex or even impossible. No luck so far. Unfortunately.

SimenB · 2022-09-28T07:39:33Z

https://github.com/facebook/jest/releases/tag/v29.1.0

github-actions · 2022-10-30T00:11:50Z

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.

feat: typed *ReturnedWith argument

1b0121b

facebook-github-bot added the cla signed label Sep 18, 2022

mrazauskas commented Sep 18, 2022

View reviewed changes

royhadad reviewed Sep 18, 2022

View reviewed changes

mrazauskas added 2 commits September 18, 2022 16:24

use unknown

00315ce

add changelog entry

0272238

royhadad reviewed Sep 18, 2022

View reviewed changes

royhadad pushed a commit to royhadad/jest that referenced this pull request Sep 18, 2022

refactor: use idea from pr jestjs#13278

4080950

simplify

96ab005

royhadad reviewed Sep 18, 2022

View reviewed changes

back to anys

9db869c

mrazauskas commented Sep 18, 2022

View reviewed changes

mrazauskas and others added 4 commits September 19, 2022 07:09

Merge branch 'main' into feat-typed-ReturnedWith

a3ccd47

fix order

9c166b5

Merge branch 'main' into feat-typed-ReturnedWith

539f974

changelog

90aa595

SimenB approved these changes Sep 20, 2022

View reviewed changes

SimenB merged commit 3d626a5 into jestjs:main Sep 20, 2022

mrazauskas deleted the feat-typed-ReturnedWith branch September 20, 2022 06:33

This was referenced Sep 29, 2022

[Bug]: Type inferrence in toBeCalledWith broken for method with overloads #13337

Closed

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

Merged

github-actions Bot locked as resolved and limited conversation to collaborators Oct 30, 2022

	toReturnWith<U extends EnsureFunctionLike<T>>(expected: ReturnType<U>): R;
	toReturnWith(expected: ReturnType<EnsureFunctionLike<T>>): R;

Uh oh!

Conversation

mrazauskas commented Sep 18, 2022

Summary

Test plan

Uh oh!

mrazauskas commented Sep 18, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

royhadad commented Sep 18, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SimenB left a comment

Choose a reason for hiding this comment

Uh oh!

mrazauskas commented Sep 20, 2022

Uh oh!

royhadad commented Sep 20, 2022

Uh oh!

mrazauskas commented Sep 20, 2022

Uh oh!

mrazauskas commented Sep 21, 2022

Uh oh!

SimenB commented Sep 28, 2022

Uh oh!

github-actions Bot commented Oct 30, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants