jest: Update for v27

[domdomegg]

Please fill in this template.

• Use a meaningful title for the pull request. Include the name of the package modified.
• Test the change in your own code. (Compile and run.)
• Add or edit tests to reflect the change.
• Follow the advice from the readme.
• Avoid common mistakes.
• Run npm test <package to test>.

If changing an existing definition:

• Provide a URL to documentation or source code which provides context for the suggested changes: Jest Changelog. In particular, these PRs:
  • fix: remove deprecated functions from the jest object jestjs/jest#9853
  • feat: use modern fake timers by default jestjs/jest#10874
  • feat(globals): make test return values stricter jestjs/jest#10512
• If this PR brings the type definitions up to date with a new version of the JS library, update the version number in the header.

[typescript-bot]

@domdomegg Thank you for submitting this PR! I see this is your first time submitting to DefinitelyTyped 👋 — I'm the local bot who will help you through the process of getting things through.

This is a live comment which I will keep updated.

1 package in this PR

• jest — on npm, on unpkg
  • 1 added owner: ✎@domdomegg

@domdomegg: I see that you have added yourself as an owner, are you sure you want to become an owner?

Code Reviews

Because this is a widely-used package, a DT maintainer will need to review it before it can be merged.

You can test the changes of this PR in the Playground.

Status

• ✅ No merge conflicts
• ✅ Continuous integration tests have passed
• ✅ Most recent commit is approved by a DT maintainer

All of the items on the list are green. To merge, you need to post a comment including the string "Ready to merge" to bring in your changes.

---

Diagnostic Information: What the bot saw about this PR

{
  "type": "info",
  "now": "-",
  "pr_number": 55013,
  "author": "domdomegg",
  "headCommitOid": "63b487b9c57fd9c1987bacd04c82105422600444",
  "lastPushDate": "2021-08-09T21:41:27.000Z",
  "lastActivityDate": "2021-08-10T18:17:11.000Z",
  "mergeOfferDate": "2021-08-10T09:02:15.000Z",
  "mergeRequestDate": "2021-08-10T18:17:11.000Z",
  "mergeRequestUser": "domdomegg",
  "hasMergeConflict": false,
  "isFirstContribution": true,
  "tooManyFiles": false,
  "popularityLevel": "Critical",
  "pkgInfo": [
    {
      "name": "jest",
      "kind": "edit",
      "files": [
        {
          "path": "types/jest/index.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/jest/jest-tests.ts",
          "kind": "test"
        }
      ],
      "owners": [
        "NoHomey",
        "jwbay",
        "asvetliakov",
        "alexjoverm",
        "epicallan",
        "ikatyang",
        "wsmd",
        "JamieMason",
        "douglasduteil",
        "ahnpnl",
        "joshuakgoldberg",
        "UselessPickles",
        "r3nya",
        "hotell",
        "sebald",
        "andys8",
        "antoinebrault",
        "gstamac",
        "ExE-Boss",
        "quassnoi",
        "Belco90",
        "tonyhallett",
        "ycmjason",
        "devanshj",
        "pawfa",
        "regevbr",
        "gerkindev"
      ],
      "addedOwners": [
        "domdomegg"
      ],
      "deletedOwners": [],
      "popularityLevel": "Critical"
    }
  ],
  "reviews": [
    {
      "type": "approved",
      "reviewer": "orta",
      "date": "2021-08-10T09:01:37.000Z",
      "isMaintainer": true
    },
    {
      "type": "approved",
      "reviewer": "G-Rath",
      "date": "2021-08-09T21:50:19.000Z",
      "isMaintainer": false
    }
  ],
  "mainBotCommentID": 894853661,
  "ciResult": "pass"
}

[typescript-bot]

🔔 @NoHomey @jwbay @asvetliakov @alexjoverm @epicallan @ikatyang @wsmd @JamieMason @douglasduteil @ahnpnl @JoshuaKGoldberg @UselessPickles @r3nya @Hotell @sebald @andys8 @antoinebrault @gstamac @ExE-Boss @quassnoi @Belco90 @tonyhallett @ycmjason @devanshj @pawfa @regevbr @GerkinDev — please review this PR in the next few days. Be sure to explicitly select Approve or Request Changes in the GitHub UI so I know what's going on.

[domdomegg]

I see that you have added yourself as an owner, are you sure you want to become an owner?

Yes, I am happy to be an owner (also fine not being one, if others object 🤷).

[G-Rath]

While this is correct for test suite & case functions, it's technically not for hooks because you can return any value so long as you don't have the done callback, as it's meant to make it easier compatibility with short-handed arrow functions; currently this type will require folks to return a promise even if they're not doing anything async.

I think we should make a new HookCallback to cover this (I'm impartial to if we create a new Lifecycle-like type or make Lifecycle take a generic of ProvidesCallback | HookCallback)

[domdomegg]

Thanks for the review! 🙌

currently this type will require folks to return a promise even if they're not doing anything async

The ProvidesCallback type should allow them to return void or undefined or a promise if they don't use done (the tests verify this I believe). If they use done, they must return void or undefined. We could make this more explicit perhaps by changing this line to:

type ProvidesCallback = ((cb: DoneCallback) => void | undefined) | (() => Promise<unknown> | void | undefined);

---

make it easier compatibility with short-handed arrow functions

I'm not sure of exactly what you mean here. This change will disallow code like:

const getMagicNumber = () => 1234;
jest.beforeAll(() => getMagicNumber());

because we would then be returning a number. This is not void, undefined or Promise.

This is intentional for Jest 27 to match @SimenB's change: "Disallow return values other than a Promise from hooks and tests"

If you think we should allow hooks to return any we could easily implement that with something like:

type ProvidesHookCallback = (cb: DoneCallback) => any;
// or just so it's clear that it's a subtype (but wouldn't change the actual typing)
type ProvidesHookCallback = (cb: DoneCallback) => any | ProvidesCallback;

type Lifecycle = (fn: ProvidesHookCallback, timeout?: number) => any;

...but I believe this will create a discrepancy between DefinitelyTyped and Jest which is undesirable. Happy to take the consensus of active reviewers though.

[G-Rath]

What I'm meaning is that it's still valid to return anything from a hook so long as you don't have done - I've confirmed this with this on latest jest:

describe('a test', () => {
  beforeEach(() => 1);

  it('passes', () => {
    expect(true).toBe(true);
  });
});

We're even doing it in eslint-plugin-jest:

// no-deprecated-functions.ts
/** @internal */
export const _clearCachedJestVersion = () => (cachedJestVersion = null);

// __tests_/no-deprecated-functions.ts
describe('the rule', () => {
  beforeEach(() => _clearCachedJestVersion());
  ...
});

The problem I'm foreseeing is that people will be using this pattern without issue on Jest 27 right now, only for them to update their types and suddenly get it flagged as an error despite it working at runtime - and unless they scour the changelog and find the entry mentioning this breaking type-only change, I expect they'll open issues and PRs here saying the types are wrong.

[domdomegg]

Sure, happy to - I only made this change to match the changelog and internal jest types.

As long as nobody else objects I'll change this, as per the end of my previous comment (adding ProvidesHookCallback).

[G-Rath]

as per the end of my previous comment (adding ProvidesHookCallback).

I believe the change you described there won't be correct: Lifecycle is currently used to type both hooks (which do allow returning anything unless you're using done) & tests (which don't allow returning anything ever).

The current change you've made looks correct for tests, so I think we just need to make a new type (or really duplicate the types as they were before your change) for hooks - which is why I suggested maybe using a generic might be tidier since it would save having two Lifecycle types.

Nope I'm wrong - it's ProvidesCallback that is being used for both hooks and tests currently 😅

In saying that, the ProvidesHookCallback should not allow returning if the callback function is present, as that will cause an error in v27.

[enisdenjo]

What about when using jest-jasmine2 as the test runner? In such cases, returning a Promise together with the done callback is completely ok and safe. I had to revert the version back exactly because of this.

We should rely on the runner reporting runtime errors instead of the type system, especially in cases where the runtime is configurable (like for jest). When using jest-circus, the combination of a done callback and a returned promise will raise a descriptive error:

Test functions cannot both take a 'done' callback and return something. Either use a 'done' callback, or return a promise.

Suggested change

	type ProvidesCallback = ((cb: DoneCallback) => void | undefined) | (() => Promise<unknown>);
	type ProvidesCallback = (cb: DoneCallback) => Promise<void | undefined> | void | undefined;

[domdomegg]

Hi @enisdenjo, can you give an example of a test in jest-jasmine2 that requires both a done callback and returning a promise?

As per the Jest changelog and blog, breaking changes in Jest 27 include making TypeScript types stricter to prevent returning a Promise and taking a done callback - no matter the runtime environment.

It might be worth raising an issue on Jest directly rather than the types if you disagree with the change to disallow using both a done callback and returning a promise. If you think here in the types we've misinterpreted what Jest has changed, then happy to discuss - but otherwise we're debating something that is out of scope for DefinitelyTyped.

[enisdenjo]

Hey @domdomegg,

can you give an example of a test in jest-jasmine2 that requires both a done callback and returning a promise?

I use done+promise all around, here are a few examples:

• https://github.com/enisdenjo/graphql-ws/blob/06e3326e0467c91535a3d0697f249c1b7db8bfbd/src/__tests__/client.ts#L171-L192
• https://github.com/enisdenjo/graphql-ws/blob/06e3326e0467c91535a3d0697f249c1b7db8bfbd/src/__tests__/client.ts#L380-L402
• https://github.com/enisdenjo/graphql-ws/blob/06e3326e0467c91535a3d0697f249c1b7db8bfbd/src/__tests__/server.ts#L26-L59
• https://github.com/enisdenjo/graphql-ws/blob/06e3326e0467c91535a3d0697f249c1b7db8bfbd/src/__tests__/server.ts#L509-L532

Do note that all above examples run on Jest v27 with the test runner set to jest-jasmine2.

This PR makes all of them fail with TS, even though its perfectly type legal and safe to do so with jest-jasmine2.

As per the Jest changelog and blog, breaking changes in Jest 27 include making TypeScript types stricter to prevent returning a Promise and taking a done callback - no matter the runtime environment.

I think you're wrong here, I guess you're talking about this entry in the changelog:

[jest-circus] [BREAKING] Fail tests when multiple done() calls are made (jestjs/jest#10624)

The change is exclusive to jest-circus, not whole jest (as stated in the scope on the beginning of the changelog entry). jest-circus is a test runner and jestjs/jest#10624 fails tests during runtime.

See #55013 (comment).

It might be worth raising an issue on Jest directly rather than the types if you disagree with the change to disallow using both a done callback and returning a promise.

Personally, I am against this change, it just complicates stuff unnecessarily. You want async tests to leverage the await and still use a done in a callback to indicate completion where tests have mixes of promises and events (as my examples show above).

Furthermore, there is already an issue raised for jest-circus: jestjs/jest#10529. Yeah, sure, you can just do jestjs/jest#10529 (comment); but, test suites are meant to be convenient and easy to use.

[enisdenjo]

My bad, I referenced the wrong changelog entry before. You were probably talking about:

[jest-globals] [BREAKING] Disallow mixing a done callback and returning a Promise from hooks and tests (jestjs/jest#10512)

which does indeed belong to jest-globals. 🤔

[enisdenjo]

@domdomegg even though the v27 changelog claims done+promise is disallowed globally, it seems not to be. The done+promise error is only present when using the jest-circus test runner and the assertion can be found here (inside jest-circus) only:

https://github.com/facebook/jest/blob/7c6cea2696e6f08553189a5e077126d610751576/packages/jest-circus/src/utils.ts#L212-L220

[domdomegg]

Ready to merge

[typescript-bot]

I just published @types/jest@27.0.0 to npm.

[billyjanitsch]

I believe this change should have also updated the dependencies on Jest's type-exposing packages to v27:

DefinitelyTyped/types/jest/package.json

Lines 4 to 5 in 32ac206

	"jest-diff": "^26.0.0",
	"pretty-format": "^26.0.0"