-
-
Notifications
You must be signed in to change notification settings - Fork 6.6k
chore(jest-jasmine2): remove usage of Function type
#10216
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
| return function <T>( | ||
| fn: Function | (() => Promise<T>) | GeneratorFunction | undefined, | ||
| fn: | ||
| | ((done: DoneFn) => void | PromiseLike<T>) |
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 should be an error to both take a done function and return a promise
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.
This will cause some major type problems, as everywhere else is expecting only (done: DoneFn) => void.
For the record, by unioning the functions TS can't support both of them when doing .call, so it favors (done: Done) => void, meaning that returnValue.then is invalid as it has type void.
This can be resolved by using an interface:
interface OneOrTheOther {
(done: DoneFn): void;
(): PromiseLike<void>;
}
This then takes us back to where we were before, but this time it's the opposite: TS is complaining asyncJestTest doesn't fit because it's of type (done: DoneFn) => void instead of OneOrTheOther 😬
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.
😞
|
Thanks! |
|
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
Improves the types for
jest-jasmine2to remove banned types (specificallyFunction&object) as part of #10177.The only two files that have banned types (well, type:
Function) are:errorOnPrivate: I suspect thatFunctionmight actually be the "right" type to use here, but more importantly it's usage is for a passthrough parameter toErrorWithStackwhich comes fromjest-utils, so the correct type will be whatever replaces theFunctionusage in that package.jasmineAsyncInstall: I've almost got this typed, but the remaining errors point towards a type being incorrect somewhere that I can't pin down. I think there's a bit of critical understanding that would make it all click that I'm missing.I've included the work in this commit, so CI will fail, but am happy to revert that section to get the rest of the changes landed.
Test plan
See if the tests pass :D