jest-community
feat: support Jest v30
ok so the E2E tests for this are failing:
Which I think is because of a bug in Jest / the new resolver introduced in v30 - the e2e tests are setting ../../../ as the runner, which Jest v29 apparently finds as a "node module" and its resolved to /projects/jest-runner-eslint/build/runner/index.js:
whereas in Jest v30 that doesn't happen resulting in jest-config/build/index.js being resolved for some reason, as the runner for some reason and then the test suite crashes as that is not a runner:
(the above logs are coming from a hacky file-based logger I put together to try and trace this which starts from the config normalizer here - its literally just writing stuff to file in a console.log manner, with a basic attempt to do some tracking of the call stack - the patched I log calls are the same in my v30 and v29 clones right up until the "base resolver" part as that's where the two major versions actually differ).
I've traced this all the way through and that path (jest-runner-../../../) seems to go all the way through to the external resolver package (resolve in Jest v29, and unrs-resolver in Jest v30) so assumingly this is a result of them treating this path differently for whatever reason
@SimenB I think I've reached the point where I'll need to hand over to someone else (assumingly @JounQin) - I'm happy to create a new bug report over in Jest if folks are ok with this being the reproduction; I have already had a look though the issues, prs, and discussions, and couldn't find anything that looked like this is already known - I suspect its a bit of an edge case.
fwiw the latest version of this package already seems to work fine with Jest v30 as we're using it in eslint-plugin-jest
Where is jest-runner-../../../ generated? jest-resolve or jest-runner-eslint?
It comes from within the jest-resolve function I linked: https://npmfs.com/package/jest-resolve/29.7.0/build/utils.js#L116 & https://npmfs.com/package/jest-resolve/29.7.0/build/utils.js#L124
And here is where the prefix value gets passed in: https://npmfs.com/package/jest-resolve/29.7.0/build/utils.js#L216
@JounQin cool, I can probably find time to open an issue on the jest repo tomorrow, but also happy if you want to make one so you can work on it
@G-Rath
Sorry, after another thinking.
Then it depends on should we consider ../../.. as a valid runner name. 🤔
cc @SimenB @cpojer
But why is that set like this at the first place? It seems just depending on unintended behaviour.
By the way, resovling jest-runner-../../.. successfully could an issue of resolve actually.
cc @ljharb
To be clear, what's happening here is the runner is pointed to as a package i.e. ./../../ leads to the root of this repo which has a package.json so then jest/node looks at its main field and resolved the runner to it's actually js entry point.
That sounds reasonable to me, though there might be preference to have this done another way (I've not tried, but I expect using npm link or pointing at the actual js file (../../../build/runner/index.js) would also work).
However either way it feels like there's a separate bug with the fact we end up having a path leading to jest-config returned - there's nothing pointing to that afaik, so it feels like something has tried to do something akin to a find-pkg-up and the jest-config package is the first thing it finds that matches
Sorry I think I still can't fully understand the root cause, a failing test case to jest-resolve would be appreciated if you think it's a upstream issue.
I think the question you posed earlier is a good place to start: should Jest support a relative path to a directory that is a package, or does it require that relative paths be to files?
I'm not entirely sure of the root issue myself which is why I posted my original comment - but what I do know is that if I attempt to import the repository it will work fine (e.g. require('./../../../') will give the package, same as doing require('jest-runner-eslint') if it was installed in node_modules)
Hence I think if the answer to this question is "yes", then we have a bug in jest-resolve, otherwise, we need to update our test configs here (and jest should probably update its docs).
Fwiw I've looked over the jest e2e tests and the ones I can find that use custom runners do <rootdir>/file.js - I don't think that should be taken as the answer to the above question being "no", but it explains why this hasn't been caught and means the answer might not be "yes"...
@G-Rath I think you you need post a new issue on jest repository to get @SimenB and @cpojer involved.
@SimenB this should be ready to go - I've got a completely green CI on my local and in my fork repo. There is a bit of a weird situation though regarding create-jest-runner as the latest 0.x version of that doesn't allow Jest v30 as a peer dependency even though it does seem to work fine with it (we've already been using Jest v30 on eslint-plugin-jest for bit), but you've done v1 with that change - so this PR will mean the peer constraints for jest-runner-eslint will be happy but not create-jest-runner...
Let me know what you'd like to do about that - I'll also look to open an issue on the Jest repo about the ../../../ change after we've gotten the ball rolling on this PR
the top level config references the runner itself via an unresolved string and is working fine so there is still a sort of test covering that
it looks like it's because it is prefixed with ./ - happy to do that instead of use require.resolve if it'd make you happier
I'm not concerned with the usage of require.resolve; I'm concerned with the change of existing tests, which constitutes a breaking change. Is there a way we can make the path be conditional, and be resolved only in jest 30+?
Sure we should be able to, but I don't think the concern is founded because:
- the previous versions of Jest are not going to change
- the expected way to use this package is with
jest-runner-eslint, not to be providing a relative path to the package - this seems like a bug in Jest v30 so ideally it'll be a non-issue in the near future
- it's likely a new major version will be needed anyway for bumping
create-jest-runner
Confirming what changes are to be done for that last point is what I'm more interested in at this stage - I can push the conditional path stuff up tomorrow morning
I'd really prefer if we didn't since currently its very hard to use this package with NPM due to peer dependency enforcement (which I believe is made more complicated by the create-jest-runner dependency) - currently the smoke test workflow for eslint-plugin-jest is broken because we have to use NPM for installing for, and thus we run into this.
If you'd like to try speed up getting it resolved in Jest I've still not had the time to open an issue officially reporting it there, so if you want to do that it'd really help
I've opened https://github.com/jestjs/jest/issues/15712 with a reproduction - interestingly, it seems that the issue only happens when nesting at least three directories deep and when using Yarn
@SimenB @ljharb there we go, green CI with nothing but version constraint changes 😄
@SimenB just fyi this is moot as the available version of create-jest-runner doesn't officially support Jest v30 - so either a new 0.x version of that needs to be released (which would be my preference since it seems to be compatible without change), or a new major of this needs to be released dropping the node versions required to use create-jest-runner v1 (and in that case I think ESLint v9 support should be included too as breaking changes might make that easier)