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(@jest/types): add partial support for done callbacks in typings of each #13756

Merged
merged 11 commits into from
Jan 26, 2023
Merged

fix(@jest/types): add partial support for done callbacks in typings of each #13756

merged 11 commits into from
Jan 26, 2023

Conversation

mrazauskas
Copy link
Contributor

Based on DefinitelyTyped/DefinitelyTyped#63882

Summary

As it was pointed out on DT, current types of each do not support done callbacks. That is undocumented feature, but it works (at least it worked for me in few simple checks).

Worth to add this for completeness. Perhaps?

Test plan

Type tests added.

@mrazauskas mrazauskas changed the title fix(@jest/types): support done callbacks in each types fix(@jest/types): support done callbacks in typings of each Jan 11, 2023
@mrazauskas mrazauskas marked this pull request as draft January 11, 2023 18:32
@mrazauskas
Copy link
Contributor Author

mrazauskas commented Jan 11, 2023

This PR is introduced type errors in tests of Jest repo, for example:

test.each([
  ['Boolean', {automock: []}],
  ['Array', {coverageReporters: {}}],
  ['String', {preset: 1337}],
  ['Object', {haste: 42}],

// now needs this type cast to be added to pass type check
] as Array<[string, Record<string, unknown>]>)(
  'pretty prints valid config for %s',
  (_type, config) => {
    expect(() =>
      validate(config, {
        exampleConfig: validConfig,
      }),
    ).toThrowErrorMatchingSnapshot();
  },
);

Hm.. Looks breaking and less flexible for the user. Not sure if done callback support is so important.

@SimenB
Copy link
Member

SimenB commented Jan 14, 2023

A done callback is supported, yes.

@SimenB
Copy link
Member

SimenB commented Jan 14, 2023

/cc @mattphillips

@mrazauskas mrazauskas changed the title fix(@jest/types): support done callbacks in typings of each fix(@jest/types): add partial support for done callbacks in typings of each Jan 14, 2023
@mrazauskas mrazauskas marked this pull request as ready for review January 14, 2023 13:29
@mrazauskas
Copy link
Contributor Author

Perhaps partial support could be added for now?

Unfortunately I have no idea how to make this work, if table is an array of arrays. In other cases done is typed as expected.

@SimenB
Copy link
Member

SimenB commented Jan 15, 2023

Some support is better than nothing 👍

@mrazauskas mrazauskas marked this pull request as draft January 15, 2023 11:22
@mrazauskas
Copy link
Contributor Author

mrazauskas commented Jan 26, 2023

Took a better look. All seems to be correct.

It is possible to type the done arg, when the test function is called with one arg and done (optionally). Like this: (arg: T, done: DoneFn) => ReturnType<EachFn>. Examples:

// user can pass `done`:
test.each([3, 4, 'seven'])('some test', (c, done) => {
  //
}

// or simply omit it (equivalent to current typings):
test.each([3, 4, 'seven'])('some test', (c) => {
  //
}

test.each([
  {a: 1, b: 2, expected: 'three', extra: true},
  {a: 3, b: 4, expected: 'seven', extra: false},
  {a: 5, b: 6, expected: 'eleven'},
])(
  'some test',
  ({a, b, expected, extra}, done) => {
  // or:
  // ({a, b, expected, extra}) => {
    //
}

test.each`
  a    | b    | expected
  ${1} | ${1} | ${2}
  ${1} | ${2} | ${3}
  ${2} | ${1} | ${3}
`('some test', ({a, b, expected}, done) => {
// or:
// `('some test', ({a, b, expected}) => {
  //
}

I can’t find a way to add typings for done in cases where test function is called with more than one arg. Like this: fn: (...args: T) => ReturnType<EachFn>. That is the case of two-dimensional array.

@mrazauskas mrazauskas marked this pull request as ready for review January 26, 2023 10:04
Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

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

seems reasonable to me 👍 Not complete like you say, but better than nothing 😀

@SimenB SimenB merged commit 8884105 into jestjs:main Jan 26, 2023
@mrazauskas mrazauskas deleted the fix-support-done-callback-in-each-types branch January 26, 2023 14:54
@SimenB
Copy link
Member

SimenB commented Jan 26, 2023

@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 Feb 26, 2023
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.

3 participants