-
Notifications
You must be signed in to change notification settings - Fork 82
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
Codemod for auto-creating @jest/globals
imports
#508
Conversation
Interesting! Thanks for another contribution! I just read quickly through this, and here is my feedback. Note that we have similar code for this for all the transformations – in the CLI you can pick what we internal call We also have a bunch of import helpers defined here, that could be useful for this PR. Maybe they handle more corner cases than you do, or maybe you handle additional corner cases: https://github.com/skovhus/jest-codemods/blob/main/src/utils/imports.ts Currently all codemods converts from one test runner to another, this one is different. I guess it shouldn't be exposed in the CLI or? But the transformation here could just be exposed directly as these: https://github.com/skovhus/jest-codemods#usage-jscodeshift |
@skovhus sorry for the lag on this; do you want the content of this PR (a If "yes", I can probably find some time to e.g. update to If "no": I can close.
I don't have strong feelings here; whatever you prefer. |
This is great. Let us just document it in the README and then I think we are good to go. |
@jest/globals
imports
I can add more prose about this codemod, but it seems fairly discretionary about what you'd like to say (e.g. do you want another sentence before the status images? I don't know that this is worth highlighting like that.) |
@skovhus anything else you'd like me to do here? I haven't quite figured out how to CI checks passing (I'm getting different results locally, I think?). Note: when I tried updating the lock file, the format was different and I don't think the CI checks passed then either (but I think perhaps it was a different error at least?). |
hey @skovhus 👋 just following up again here: anything I can do to get this done? it's not particularly urgent, I'm just hoping to tie up loose ends by getting this in eventually! (when it's right by you) let me know if you can! (if you're on holiday or something no sweat.) |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #508 +/- ##
==========================================
+ Coverage 92.38% 92.59% +0.20%
==========================================
Files 26 27 +1
Lines 1944 1999 +55
Branches 405 414 +9
==========================================
+ Hits 1796 1851 +55
Misses 102 102
Partials 46 46 ☔ View full report in Codecov by Sentry. |
Thanks for adding support for this! |
I recently had to convert a bit of code that used jest globals (e.g.
describe()
,it()
,expect()
) to explicitly importing these APIs from@jest/globals
.Before:
After:
While doing that manually I thought: this would make a good codemod!
So I wrote a fairly simple one that actually scrapes the exports from the
@jest/globals
package itself[1], finds global usage, and adds imports.[1] I don't use
require.resolve()
nor actuallyimport
it cuz both are enforcedly disallowed, which is a mild bummer, but overall I think it's still kinda slick. If you know a better way, let me know!