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

Incorrect types for lifecycle functions #10066

Closed
Mossop opened this issue May 20, 2020 · 7 comments · Fixed by #10480
Closed

Incorrect types for lifecycle functions #10066

Mossop opened this issue May 20, 2020 · 7 comments · Fixed by #10480

Comments

@Mossop
Copy link

Mossop commented May 20, 2020

🐛 Bug Report

The types for the lifecycle functions (beforeEach etc.) are declared incorrectly in packages/jest-types/src/Global.ts. These functions are documented as being able to return a promise or generator and accepting a timeout parameter but the defined types disallow anything except a void returning function.

To Reproduce

Take the following code:

import { beforeEach } from "@jest/globals";

beforeEach((): Promise<void> => {
  return Promise.resolve();
}, 2000);

This causes TypeScript to complain with Expected 0-1 arguments, but got 2. ts(2554) and causes the typescript-eslint rule no-misused-promises to complain with Promise returned in function argument where a void return was expected..

Expected behavior

The given code should not show any issues.

I think that the type for HookFn should be changed to something like this:

export type LifecycleFn = (done?: DoneFn) => Generator | Promise<void | undefined | unknown> | void | undefined;
export type HookFn = (fn: LifecycleFn, timeout?: number) => void;

envinfo

  System:
    OS: macOS 10.15.4
    CPU: (12) x64 Intel(R) Core(TM) i9-8950HK CPU @ 2.90GHz
  Binaries:
    Node: 14.1.0 - /usr/local/bin/node
    npm: 6.14.4 - /usr/local/bin/npm
  npmPackages:
    jest: ^26.0.1 => 26.0.1
@villasv
Copy link
Contributor

villasv commented Sep 4, 2020

Happening with me too. It looks like the lifecycle hooks got typed as HookFn, but that's probably a mistake. HookFn is probably meant to type the fn parameter for the hooks, like TestFn types the fn parameter for tests.

This would fix it:

export interface TestFrameworkGlobals {
    it: ItConcurrent;
    test: ItConcurrent;
    fit: ItBase & {
        concurrent?: ItConcurrentBase;
    };
    xit: ItBase;
    xtest: ItBase;
    describe: Describe;
    xdescribe: DescribeBase;
    fdescribe: DescribeBase;
    // beforeAll: HookFn;    <--- this is wrong, replace with:
    beforeAll: (fn: HookFn, timeout?: number) => void;
    ...
}

@SimenB
Copy link
Member

SimenB commented Sep 13, 2020

We still don't support generator functions in the types - anybody up for a PR adding it? 🙂

@villasv
Copy link
Contributor

villasv commented Sep 13, 2020

Oops, forgot about it. Should the typing of it also support generators? Might as well go ahead and update both. (In fact, updating TestFn should already fix for both because HookFn = TestFn).

@villasv
Copy link
Contributor

villasv commented Sep 17, 2020

I failed to find examples of usage of Generators on hooks and tests. It wasn't clear to me if TestFn also has to accept generators. It's also not clear if HookFn should accept a Generator or a GeneratorFunction or a () => Generator. Anyway, all of those will result in compilation errors on packages/jest-circus/src/utils.ts's callAsyncCircusFn that I wasn't sure about how to solve.

Help Wanted indeed.

Wording on the docs is a bit ambiguous.

If the function returns a promise or is a generator, Jest waits for that promise to resolve before running the test.

Generators are not mentioned in the test* methods.

I'll try to use the GeneratorFunction in a way such that the whole project compiles, assuming the implementation of callAsyncCircusFn is correct, and see how it goes as a Pull Request.

@villasv
Copy link
Contributor

villasv commented Sep 21, 2020

Just pinging issue subscribers, the PR is ready.

@SimenB
Copy link
Member

SimenB commented Dec 4, 2020

Closed by #10836, thanks @villasv!

@SimenB SimenB closed this as completed Dec 4, 2020
@github-actions
Copy link

This issue 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 May 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants