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

Add generator function to accepted hook types #10526

Closed
wants to merge 1 commit into from
Closed

Add generator function to accepted hook types #10526

wants to merge 1 commit into from

Conversation

villasv
Copy link
Contributor

@villasv villasv commented Sep 17, 2020

Summary

Another go at #10066, generators weren't included in the last PR (#10480).

Test plan

Added more type tests.

Details

Reviewers will notice some ugly stuff going on. For starters, that isGeneratorFn type guard is checking against the GeneratorFunction interface, which doesn't seem to be the correct approach here (it's badly misunderstood just like Function). If we try to use GeneratorFunction as the receiving type for hook functions, it will fail.

So I declared a makeshift interface that should hold any generator function. But then, for some reason the type inference of a few fn.call inside jest-circus broke. I have no idea why, but manually providing the type arguments was possible to please the compiler.

Then another type inference gone awry and I had to specify the any type for the returnedValue variable, otherwise TypeScript inferred its value as possibly a Generator (thanks for nothing on the above type guard).

So all in all I'm not very pleased with the way things are, but I wasn't able to remove any of this cruft. If someone else feels like trying to, they're welcomed to try and supersede this PR.

@villasv villasv changed the title Add generator to possible return types of TestFn Add generator function to accepted hook types Sep 17, 2020
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.

I missed this 🙀 Sorry!

@@ -17,7 +22,7 @@ export type ConcurrentTestFn = (
) => Promise<void | undefined | unknown>;
export type BlockFn = () => void;
export type BlockName = string;
export type HookFn = TestFn;
export type HookFn = TestFn | GeneratorFn;
Copy link
Member

Choose a reason for hiding this comment

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

test functions can also be generators, so change that definition instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I couldn't be sure just by reading the docs. On it.

@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 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 this pull request may close these issues.

3 participants