jest-community / eslint-plugin-jest

ESLint plugin for Jest
MIT License
1.13k stars 230 forks source link

feat: allow @typescript-eslint/utils@7 #1567

Closed JoshuaKGoldberg closed 5 months ago

JoshuaKGoldberg commented 5 months ago

Fixes #1566.

~Upgrades the devDependency versions of @typescript-eslint packages to v7 too. Pleasing to see the yarn.lock mostly remain the same but for a removal of a duplicate minimatch version.~ Does not do this, as the CI matrix had them testing against incompatible ESLint versions, and I'm in a bit of a rush.

SimenB commented 5 months ago

Can we teak the install to also install v7 of the utils? https://github.com/jest-community/eslint-plugin-jest/blob/20c870387cee61440e4d4570dca5716613c449e5/.github/workflows/nodejs.yml#L103

Might make sense as its own matrix entirely 🤔

G-Rath commented 5 months ago

Personally I'm happy to ship this since I use latest Node already, but how sure you are that this will actually result in the right behaviour across Node versions and all package managers? While I'd love if it just works, I've only ever seen this be used for peer dependencies and we're had previous comments from users blaming this plugin for pulling in old versions when really it's because of the package managers.

github-actions[bot] commented 5 months ago

:tada: This PR is included in version 28.5.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

SimenB commented 4 months ago

Note that this does indeed pull in v7, and without manually messing in the lockfile, there's no way to get v6. So our minimum supported node version lies out of the box

G-Rath commented 4 months ago

I wouldn't say it's an outright lie - that's arguably a package manager bug since all our constraints and versions are valid but I expect this'll be a hard one to get fixed on their end (as it'll be considered "too hard")

I did have a play with it though after publishing and I thought npm at least did do the right thing - I'll double check tomorrow morning

SimenB commented 4 months ago

Missing feature rather than a bug, I'd say 😀 None of them say they prioritze the least amount of deps or choosing a dep based on engines

rickhanlonii commented 4 months ago

Fwiw this broke the react repo from upgrading to 28.5, so I had to pin to 28.4, but only happens in CI where we're probably using a different node version there, still annoying to happen in a minor.

SimenB commented 4 months ago

😢

But ok, package managers don't deal with this in a desirable way (meaning in practice it's a breaking change), so we should revert

nklowns commented 3 months ago

Note that this does indeed pull in v7, and without manually messing in the lockfile, there's no way to get v6. So our minimum supported node version lies out of the box

This can be circumvented by using resolutions, setting the @^6 version for the package, this is how I used to run the project compatible with node@16

Resolutions Documentation on yarn

{
    ...
    "resolutions": {
        "@typescript-eslint/utils": "^6",
    },
}