Skip to content
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: support ESLint 8.x #882

Closed
wants to merge 1 commit into from

Conversation

MichaelDeBoey
Copy link
Contributor

@MichaelDeBoey MichaelDeBoey commented Aug 28, 2021

Superseded by #903

BREAKING CHANGE: Requires Node@^12.22.0 || ^14.17.0 || >=16.0.0
BREAKING CHANGE: Requires ESLint@^6.x
@G-Rath
Copy link
Collaborator

G-Rath commented Aug 29, 2021

Thanks but I don't know when we'll be able to land this since we tend to match supported node versions with Jest, which still supports v10 and I'd like to avoid adding more long-living PRs to our already decent list.

@SimenB
Copy link
Member

SimenB commented Aug 29, 2021

I'm fine making a breaking change dropping node 10 (and eslint 5 if needed)

@MichaelDeBoey
Copy link
Contributor Author

MichaelDeBoey commented Aug 29, 2021

@SimenB Since we have @typescript-eslint/experimental-utils as a dependency and it will drop support for Node v10 & ESLint v5, we don't really have a choice other than making these breaking changes.

I'm happy to re-open the PR.

@SimenB
Copy link
Member

SimenB commented Sep 6, 2021

@MichaelDeBoey let's do it, lot's of other deps we can update at the same time 🙂 But let's start just by dropping eslint 5 and node 10 without adding pre-release @typescript-eslint/experimental-utils until it's properly out. That should just be a minor change at that point, right?

@MichaelDeBoey
Copy link
Contributor Author

@SimenB Since @typescript-eslint/eslint-plugin is a peerDependency, this will be breaking whenever we update

BREAKING CHANGE: Requires Node@^12.22.0 || ^14.17.0 || >=16.0.0
BREAKING CHANGE: Requires ESLint@^6.0.0 || ^7.0.0 || ^8.0.0-0
BREAKING CHANGE: Requires @typescript-eslint/eslint-plugin@^5.0.0-0

@SimenB
Copy link
Member

SimenB commented Sep 6, 2021

Not if we support both v4 and v5, right?

EDIT: I see the range is already >=4, so that should be fine already I believe (we should have a test run with v5 verifying it works, though, and it should probably be 4.x || ^5.0.0-0 or something)

@MichaelDeBoey
Copy link
Contributor Author

MichaelDeBoey commented Sep 6, 2021

@SimenB Running it with v5 will break peerDependency of @typescript-eslint/experimental-utils (which is in our dependencies and is still v4 for now).
Updating our @typescript-eslint/experimental-utils version to v5, will break our peerDependency then.

I see the range is already >=4,

That should actually be ^4.0.0 for the same reason mention just above the quote

@SimenB
Copy link
Member

SimenB commented Sep 6, 2021

Why would it break? @typescript-eslint/eslint-plugin pulls in its own version of the utils, I don't think this plugin cares what version it is.

@MichaelDeBoey
Copy link
Contributor Author

@SimenB
Copy link
Member

SimenB commented Sep 13, 2021

Right, but that only affects users of the plugin, not us (I think) as we don't pull in the plugin, the user does

@MichaelDeBoey
Copy link
Contributor Author

It technically affects our users too since it's our peerDependency, no? 🤔

@SimenB
Copy link
Member

SimenB commented Sep 13, 2021

For us it's an optional one, we don't care which one the user pulls in. Us supporting two versions with different peer deps of their own doesn't matter, the user just pulls in the correct versions they use

@SimenB
Copy link
Member

SimenB commented Sep 13, 2021

Regardless, we can start by dropping node v10 and eslint v5 on a next branch, then dig more into whether or not adding eslint v8 and ts-eslint v5 to our matrix messes anything up

@SimenB
Copy link
Member

SimenB commented Sep 13, 2021

next now exists without node 10 or 15 and without eslint 5: https://github.com/jest-community/eslint-plugin-jest/tree/next

@MichaelDeBoey MichaelDeBoey mentioned this pull request Sep 20, 2021
50 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants