-
-
Notifications
You must be signed in to change notification settings - Fork 6.6k
Fix type definitions for promises #10600
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
Conversation
|
Thanks! Could you add some tests that fails without this change to https://github.com/facebook/jest/blob/999ee461262c217db2827db79296129a13d6e23a/test-types/top-level-jest-namespace.test.ts as well? |
3da2a2b to
a09bd56
Compare
When calling `mockResolvedValue` (-`Once`), the argument should be the expected return value unwrapped from its Promise. Likewise, when mocking a rejected value, the passed argument needs not necessarily be of the same type as the expected successful return value.
| import {expectError, expectType} from 'mlh-tsd'; | ||
| //eslint-disable-next-line import/no-extraneous-dependencies | ||
| import {jest} from '@jest/globals'; | ||
| import type {Mock} from 'jest-mock'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally we'd allow access to this without having to explicitly go through jest-mock, but that's a problem for future us 🙂 For now this is a great improvement! 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, actually running into that in my own code as well, but unfortunately I don't know how to solve that...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, solving that problem is a blocker for DefinitelyTyped/DefinitelyTyped#44365
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if just a @jest/utility-types would work, which re-export e.g. Mock, some pretty-format stuff for serializers, Config etc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have enough insight on the inner workings of Jest to have a valid opinion on that, but one challenge I am running into is that the Mock type from jest-mock (i.e. the type of the return value of jest.fn()) is different from the Mock type that's available as jest.Mock from @jest/globals, and it's not clear to me why. W.r.t. to importing it from jest-mock or from @jest/utility-types, either seems fine to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
import {jest} from '@jest/globals';
import jestMock = require('jest-mock');
jest.fn === jestMock.fn;at least that's the intention. If that's not true, we've messed up somewhere 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't seem to be true here: https://codesandbox.io/s/admiring-water-licv4?module=%2Fsrc%2Fjest.test.ts
Am I doing something wrong or should I report a bug?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
jest.Mock there comes from @types/jest not from the local jest import, so it's essentially what DefinitelyTyped/DefinitelyTyped#44365 is meant to solve
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah got it. Thanks!
Since 9cc1379, Jest supports disabling the injection of globals. This makes tests easier to follow, and ensures that all types can be properly checked as well. (Unfortunately this also exposed a flaw in Jest's type definitions, which I've fixed here but which isn't released yet: jestjs/jest#10600)
Use the new release with a fix for jestjs/jest#10600.
With the release of a new Jest version with a fix for jestjs/jest#10600, a lot of type assertions in tests are no longer necessary.
Use the new release with a fix for jestjs/jest#10600.
Since 9cc1379, Jest supports disabling the injection of globals. This makes tests easier to follow, and ensures that all types can be properly checked as well. (Unfortunately this also exposed a flaw in Jest's type definitions, which I've fixed here but which isn't released yet: jestjs/jest#10600)
|
Just tested the new release in my code, works perfectly. Thanks for the speedy turnover! |
Use the new release with a fix for jestjs/jest#10600.
Since 9cc1379, Jest supports disabling the injection of globals. This makes tests easier to follow, and ensures that all types can be properly checked as well. (Unfortunately this also exposed a flaw in Jest's type definitions, which I've fixed here but which isn't released yet: jestjs/jest#10600)
|
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. |
Summary
When calling
mockResolvedValue(-Once), the argument should bethe expected return value unwrapped from its Promise. Likewise,
when mocking a rejected value, the passed argument needs not
necessarily be of the same type as the expected successful return
value.
Test plan
Without this change, the following code will cause TypeScript to complain:
That error will disappear with this PR.