-
Notifications
You must be signed in to change notification settings - Fork 456
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
feat(jest): bump to 24 #957
Conversation
Thanks for opening this PR @thymikee 👍 |
package.json
Outdated
@@ -70,7 +70,7 @@ | |||
"yargs-parser": "10.x" | |||
}, | |||
"peerDependencies": { | |||
"jest": ">=22 <24" | |||
"jest": ">=22" |
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.
Considering the changes to babel-core (bridge) in this PR, it is safe to say it won't work with Jest <24.
Also considering that Jest has changed quite a bit between versions, it doesn't hurt to stay conservative and still explicitly require <25
IMHO.
"jest": ">=22" | |
"jest": ">=24 <25" |
Probably wanna tweak the presets as well - I think all config but the transformer can be removed? |
Would be cool to simplify it. Configuration of this preset is, let's say it lightly, pretty complicated :P |
Jest 2.4.1.0 released yesterday, including the changes from jestjs/jest#7707 : https://github.com/facebook/jest/releases/tag/v24.1.0 Is this unblocked now? |
@jrr yup, working on upgrading the deps, these e2e tests are a pain to work with 😅 |
I've also bumped Jest typings to 24 and it spawned some new errors. Can somebody with TS experience can take a look? Help appreciated :) |
Took a look while something long running was happening, part of me is wondering if the implementation for Either that or this repo has consistently used e.g. jest.mock('fs')
const fs = mocked(_fs)
// fs.existsSync = (path: string) => boolean
// Fails
fs.existsSync.mockImplementation(() => true)
// Passes
fs.existsSync.mockImplementation(() => () => true) If jest accepts passing in the direct function instead of the thunk to the function, then maybe the types should too? |
I bet this is fixed by DefinitelyTyped/DefinitelyTyped#32816 - I wonder if it's worth making these mocks a Partial instead of the full objects, a lot of the |
orta@28fdcd7 should handle a bunch of the type issues when that PR is merged and updated in here. Might be worth skipping the types/jest update for now. |
@orta you should ban |
@orta @thymikee Your problem seems to be that MockWithArgs should be :
|
@thymikee @orta fixes to |
The problem, when I tested, is that this is incompatible with |
45768ff
to
d0f2231
Compare
dbfc03d
to
1d67101
Compare
Pull Request Test Coverage Report for Build 2384
💛 - Coveralls |
I agree .. I found this problem because we use babel plugins after ts and they break with v24.1 |
would it be possible for someone to test this with a real project? That would increase the confidence in releasing a new version to some extent. I'll give this a day or two to wait for some feedback and will then merge it in and publish a new version even if there isn't any. |
Running this with a real project now... |
Can you update it in NPM as |
maybe can release it as alpha @kulshekhar ? |
ok, I'll do that after work today |
Works just fine with one of our (kinda big: 16 suites/231 tests) applications! 🎉 |
ok, in that case I'll just release the full version later today to save on time. I mean, folks can pin it down to 23.x if it turns out to be a problem. Not ideal but doesn't seem like a dealbreaker |
Also confirmed it works, ran it against Danger:
|
v24.0.0 has been published. Thank you everyone for getting this through! |
@kulshekhar Moving to jest v24 also means that My tests break with the following error:
This problem still persists. |
@kulshekhar Thanks for your time and efforts into this project, I was about to test the P/R today, look I've been a bit late. With the new release, I face the same error as @artola mentioned. I guess it might work merging #960, but I found out that I could simply remove reference to .babelrc from my globals: {
window: {},
'ts-jest': {
tsConfig: './tsconfig.jest.json',
compiler: 'typescript',
// COMMENT THIS, and the partialConfig problem disappear with ts-jest 24
// babelConfig: '.babelrc',
},
}, @artola can you try ? Works for me. |
That will "work" if you have no need for babel plugins when running tests. That'll be the case in a lot of cases but an easy example where you do need them is, for example, with |
@Jessidhia Good explanation thanks. |
@belgattitude @Jessidhia I have in the global I have no clue how to go around Without a solution, I can not profit of v24. |
I find it a bit odd this was merged (or at least released) without fixing the babel-jest integration, or a big note somewhere that it's currently broken. Maybe edit the OP to include the info? |
Yes, it's not ideal. From my previous post
It was a tradeoff between
Yeah, I think that's a good idea. I'll update the OP later today after work and also add a note to the readme (PR welcome too!) that reflects this Additionally, if someone thinks that becoming a collaborator would help them get this (or other issues) moving faster, just let me know |
Everything passing here. Thanks to @thymikee, @kulshekhar and others! |
For everyone with an interest in ts-jest, as you can probably tell we are a little short-staffed. We'd love to get some more active contributors :) |
@thymikee @kulshekhar Regarding the issue with babel and the cached config ts-jest/src/config/config-set.ts Line 348 in 1ac501a
I hope that this is the tip of the iceberg, but for sure it is not a fix, neither I have a complete understanding of of the other conditions around this block. |
Jest 24 is out, so why not bump it? :)
Let's wait until 24.0.1 is out before continuing on it (e2e tests are affected by jestjs/jest#7707).
EDIT: This still needs some work to be done with regards to
babel-jest
integration