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 #792

Merged
merged 13 commits into from
Oct 23, 2021
Merged

feat: support ESLint 8.x #792

merged 13 commits into from
Oct 23, 2021

Conversation

@brettz9
Copy link
Collaborator

brettz9 commented Oct 11, 2021

Thanks very much for the report and draft PR.

check-examples will unfortunately not work due to our waiting on eslint/eslint#14745 (so that the ESLint constructor can indeed fully replace CLIEngine for our needs with the rule).

I think we can work around this for now by:

  • Have the src/rules/checkExamples.js file immediately run something like the following if ESLint 8 is detected (and return):
      context.report({
        loc: {
          start: {
            column: 1,
            line: 1,
          },
        },
        message: `This rule cannot yet be supported for ESLint 8; you should either downgrade to ESLint 7 or disable this rule. The possibility for ESLint 8 support is being tracked at https://github.com/eslint/eslint/issues/14745`,
      });
  • Adjust test/rules/index.js to avoid testing the rule when when ESLint 8 is detected
  • Make a note of this under the check-examples documentation (adding a note to the top of /.README/rules/check-examples.md such as This rule currently does not work in ESLint 8 (we are waiting on [issue 14745](https://github.com/eslint/eslint/issues/14745)). and then running npm run create-readme to rebuild the main README).

Am a bit preoccupied these days, but I should be able to review if you (or someone) could assist with those changes.

@MichaelDeBoey
Copy link
Contributor Author

@brettz9 I think I did everything you mentioned.

Let me know what you think of it!

@brettz9
Copy link
Collaborator

brettz9 commented Oct 12, 2021

Thanks a lot. That mostly covered it. I've applied a few additional updates/fixes. Be sure to run npm i after pulling.

I've disabled linting in CI (and lint-staged) until eslint-config-canonical may be updated. (Since there is a mutual dependency between this project and that one, it will probably be good to have this project updated so it can target the updated one--then we can resume linting after that is updated.)

I checked the checkbox for @typescript-eslint/parser 5.0.0 as 5.0.0 is now released (and I guess the whole to-do for the parser is complete?).

There is one remaining problem though... Despite getting this working fine locally, for our ESLint 7 CI tests, we're still getting an error related to check-examples. I don't know if that means the CI environment isn't actually installing 7.0 there as it should or if it isn't properly detecting the version the same in these environments as it is on my machine (Mac).

BREAKING CHANGE: Requires ESLint@^7.0.0 || ^8.0.0
@brettz9 brettz9 marked this pull request as ready for review October 22, 2021 02:26
@brettz9
Copy link
Collaborator

brettz9 commented Oct 22, 2021

Looks like we have one remaining task now of getting the CI Node 17 environment (for which I'd like this PR to add support) updating to Python >=3.6.0

@brettz9 brettz9 force-pushed the eslint-8 branch 2 times, most recently from 7eab7ed to 0c6db1a Compare October 22, 2021 23:59
@brettz9 brettz9 force-pushed the eslint-8 branch 6 times, most recently from b25bb34 to 604b400 Compare October 23, 2021 02:06
@gajus
Copy link
Owner

gajus commented Oct 23, 2021

🎉 This PR is included in version 37.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support ESLint 8.x
3 participants