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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: support ESLint 8.x #903

Closed
wants to merge 1 commit into from
Closed

feat: support ESLint 8.x #903

wants to merge 1 commit into from

Conversation

MichaelDeBoey
Copy link
Contributor

@MichaelDeBoey MichaelDeBoey commented Sep 20, 2021 •
edited

ESLint v8.0.0 is released 馃帀

Dependencies should be compatible with ESLint 8 too before we can merge this one:

devDependency compatibility with ESLint 8:

Supersedes #882

Closes #881

@MichaelDeBoey MichaelDeBoey mentioned this pull request Sep 20, 2021
Closed
This was referenced Sep 20, 2021
Closed
Closed
Merged
Merged
Merged
Merged
SimenB
SimenB requested changes Sep 20, 2021
View reviewed changes
Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some changes causes unnecessarily big diffs, would be nice to avoid them 馃檪

.github/workflows/nodejs.yml Outdated Show resolved Hide resolved
package.json Show resolved Hide resolved
src/rules/__tests__/utils.test.ts Show resolved Hide resolved
src/rules/prefer-expect-assertions.ts Outdated Show resolved Hide resolved
src/rules/prefer-strict-equal.ts Show resolved Hide resolved
@MichaelDeBoey MichaelDeBoey mentioned this pull request Sep 20, 2021
Merged
@SimenB SimenB mentioned this pull request Sep 20, 2021
Merged
2 tasks
SimenB
SimenB reviewed Sep 20, 2021
View reviewed changes
.eslintrc.js Show resolved Hide resolved
@SimenB
Copy link
Member

SimenB commented Sep 22, 2021

a9ae1de should fix the failing run, so this should be rebased 馃檪

@SimenB
Copy link
Member

SimenB commented Oct 10, 2021

#928

@SimenB SimenB deleted the branch jest-community:main October 10, 2021 09:26
@SimenB SimenB closed this Oct 10, 2021
@SimenB SimenB reopened this Oct 10, 2021
@SimenB SimenB changed the base branch from next to main October 10, 2021 09:27
@SimenB
Copy link
Member

SimenB commented Oct 10, 2021

OK, our major is out: https://github.com/jest-community/eslint-plugin-jest/releases/tag/v25.0.0

My assumption is that landing this is a semver minor when @typescript-eslint/experimental-utils makes a release 馃檪

(if not, numbers are cheap)

@SimenB SimenB mentioned this pull request Oct 10, 2021
Closed
1 task
This was referenced Oct 10, 2021
Closed
Closed
Closed
Closed
Merged
@SimenB
Copy link
Member

SimenB commented Oct 11, 2021

https://github.com/typescript-eslint/typescript-eslint/releases/tag/v5.0.0

@SimenB
Copy link
Member

SimenB commented Oct 11, 2021

I've updated @typescript-eslint/* in #941 and #942. For some reason testing with ESLint v8 still fails though (see #940), but at least for people who ignore peer dep warnings should be unblocked

@gwyneplaine gwyneplaine mentioned this pull request Oct 12, 2021
Merged
1 task
@targos targos mentioned this pull request Oct 12, 2021
Merged
@MichaelDeBoey MichaelDeBoey marked this pull request as ready for review October 12, 2021 09:55
@kimromi kimromi mentioned this pull request Oct 12, 2021
Closed
@branchvincent branchvincent mentioned this pull request Oct 13, 2021
Merged
@MichaelDeBoey
Copy link
Contributor Author

@SimenB Tests are failing but I can't figure out why, can you point me into the right direction so we can merge this one asap?

@G-Rath
Copy link
Collaborator

G-Rath commented Oct 13, 2021 •
edited

@MichaelDeBoey the error about @eslint/eslintrc/universal is because it uses package exports which isn't supported by jest - this is the fix.

The error about context.getPhysicalFilename is because that was added in ESLint v7.28, which eslint-plugin-prettier must now require (aka they dropped support for v6 & co) - we should be able to disable the rule for that run 馃 (I think the easiest way of doing that is probably to have an env variable in our config that sets it to "off" if present)

@MichaelDeBoey
Copy link
Contributor Author

@G-Rath The prettier errors seem fixed now, but seems like we missed something

@G-Rath
Copy link
Collaborator

G-Rath commented Oct 13, 2021

@MichaelDeBoey looks like eslint-plugin-eslint-plugin is having troubles with at least v6 of eslint.

The unbound-method errors is something I need to look into when I'm not working, because that is an extension of the existing rule shipped in @typescript-eslint, and so it's tests are copied from that + a couple of our own to test our extension.

Since I probably won't be able to look into this properly until the weekend, and because I think it might be the answer, if you want to look into adjusting our tests to skip running unbound-method on v8 I think that would be good (I'm half expecting it might be easiest for us to have two sets of the tests: ones for <v8/=v8/>=v5, depending on what is actually causing the errors.)

@SimenB
Copy link
Member

SimenB commented Oct 14, 2021

@MichaelDeBoey the error about @eslint/eslintrc/universal is because it uses package exports which isn't supported by jest - this is the fix.

Note that config workaround is not needed with 8.0.1 https://github.com/eslint/eslint/releases/tag/v8.0.1

SimenB
SimenB reviewed Oct 14, 2021
View reviewed changes
.eslintrc.js
@@ -28,12 +30,18 @@ module.exports = {
es6: true,
},
rules: {
// eslint-plugin-prettier removed ESLint v6 support
...(semver.gte(version, '7.28.0') ? {} : { 'prettier/prettier': 'off' }),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just keep using eslint-plugin-prettier@3, no reason to upgrade it

G-Rath
G-Rath reviewed Oct 14, 2021
View reviewed changes
package.json
@@ -60,6 +60,9 @@
"projects": [
{
"displayName": "test",
"moduleNameMapper": {
"@eslint/eslintrc/universal": "@eslint/eslintrc/dist/eslintrc-universal.cjs"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"@eslint/eslintrc/universal": "@eslint/eslintrc/dist/eslintrc-universal.cjs"
"eslint/use-at-your-own-risk": "eslint/lib/unsupported-api.js"

This is whats giving unbound-method trouble

(you can replace the existing map since we'll update to v8.0.1)

@SimenB SimenB mentioned this pull request Oct 14, 2021
Merged
4 tasks
@SimenB SimenB closed this in #940 Oct 14, 2021
@SimenB
Copy link
Member

SimenB commented Oct 14, 2021

I just landed the other PR as this one has a bunch of changes we don't need.

Thanks for your help here @MichaelDeBoey!

@github-actions
Copy link

馃帀 This issue has been resolved in version 25.1.0 馃帀

The release is available on:

Your semantic-release bot 馃摝馃殌

@Belco90 Belco90 mentioned this pull request Oct 17, 2021
Closed
66 tasks
@MichaelDeBoey MichaelDeBoey deleted the eslint-8 branch October 27, 2021 22:51
@ti10le ti10le mentioned this pull request Oct 28, 2021
Closed
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SimenB SimenB SimenB requested changes

@G-Rath G-Rath G-Rath left review comments

Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support ESLint 8.x
3 participants
@MichaelDeBoey @SimenB @G-Rath