-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
TS migration jest-circus
#7916
TS migration jest-circus
#7916
Conversation
NodeJS |
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.
Awesome start!
This doesn't work? https://github.com/facebook/jest/blob/c14a45d89d469c8bf226c5db62bac52d59597a9a/packages/jest-mock/src/index.ts#L10 |
importing Global type is working but I cannot assign to global type for this case |
It turns out TS does not support keys as symbols yet. There is workaround for it microsoft/TypeScript#24587 (comment). It is not great but it works for now |
packages/jest-circus/src/legacy-code-todo-rewrite/jestAdapter.ts
Outdated
Show resolved
Hide resolved
Codecov Report
@@ Coverage Diff @@
## master #7916 +/- ##
==========================================
+ Coverage 58.32% 65.42% +7.09%
==========================================
Files 164 255 +91
Lines 6047 9949 +3902
Branches 5 1038 +1033
==========================================
+ Hits 3527 6509 +2982
- Misses 2518 3193 +675
- Partials 2 247 +245
Continue to review full report at Codecov.
|
Co-Authored-By: doniyor2109 <[email protected]>
Co-Authored-By: doniyor2109 <[email protected]>
Exported variable 'jestAdapter' has or is using name '$JestEnvironment' from external module "packages/jest-types/build/Environment" but cannot be named. microsoft/TypeScript#5711
|
||
describe.each([['beforeEach'], ['beforeAll'], ['afterEach'], ['afterAll']])( | ||
'%s hooks error throwing', | ||
fn => { | ||
(fn: 'beforeEach' | 'beforeAll' | 'afterEach' | 'afterAll') => { |
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.
would be cool to type jest-each
so that it would be inferred.
/cc @mattphillips
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 again for a the fantastic work! I've tweaked this a bit (as you probably noticed since we pushed at the same time), and I think this is good to go now. Gonna wait for someone else to review it as well, though 🙂
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
TS migration
jest-circus
Test plan
WIP