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: Resolve getAncestors and getScope calls in eslint v9 #466

Merged
merged 9 commits into from
May 27, 2024

Conversation

scagood
Copy link
Contributor

@scagood scagood commented May 5, 2024

What is the purpose of this pull request?
This is phase 1 of getting eslint v9 working.

The features this implements are:

  1. Use the sourceCode form of getAncestors
  2. Use the sourceCode form of getScope
  3. Allow for the RuleTester class to be usable in both eslint v7, v8 and v9.

As you may be able to tell, I copied a few files over from eslint-plugin-n (Specifically from eslint-community/eslint-plugin-n#161) (@aladdin-add Thank you for the prior art 🙏)


This is one part of #449.

Copy link

codecov bot commented May 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (bbcfcbf) to head (1147fe2).
Report is 6 commits behind head on main.

Current head 1147fe2 differs from pull request most recent head 9a9eec3

Please upload reports for the commit 9a9eec3 to get more accurate results.

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #466   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           25        24    -1     
  Lines          649       656    +7     
  Branches       250       250           
=========================================
+ Hits           649       656    +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@scagood scagood changed the title feat: Attempt to support eslint v9 feat: Resolve getAncestors and getScope calls in eslint v9 May 6, 2024
@scagood scagood marked this pull request as ready for review May 6, 2024 20:21
Comment on lines +1 to +5
/**
* @fileoverview Helpers for tests.
* @author 唯然<[email protected]>
*/
'use strict'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This file was originally 'borrowed' from eslint-community/eslint-plugin-n#161, then I made a handful of changes,

I can remove the header if prefered!

@voxpelli voxpelli self-requested a review May 14, 2024 13:06
Copy link
Contributor

@aladdin-add aladdin-add left a comment

Choose a reason for hiding this comment

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

also update the ci to test eslint v9?

}

if (config.parserOptions) {
Object.assign(config.languageOptions, config.parserOptions)
Copy link
Contributor

Choose a reason for hiding this comment

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

please note parserOptions is not 100% same as languageOptions:

Since this is only used for this project, it is not required to cover 100% of cases. so, it's be fine if you were not using other options. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤔 I was actually thinking about this, and there are a couple of things I could change:

  1. I am considering making a module for this rule-tester compat package of some sort so it can be used in more places
  2. It may be best to migrate the config backwards, not forwards. The more I think about it the less I like the v8 -> v9 compat, and instead prefer the idea of v9 -> v8

Copy link
Contributor

Choose a reason for hiding this comment

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

I also see the value for rule authors want to support eslint v8 & eslint v9. maybe support both - auto-detecting eslint version and the config format:

  • eslint v8 + eslintrc : no change
  • eslint v8 + flat : flat => eslintrc
  • eslint v9 + eslintrc: eslintrc -> flat
  • eslint v9 + flat: no change

__tests__/rule-tester.js Show resolved Hide resolved
__tests__/rule-tester.js Show resolved Hide resolved
@scagood
Copy link
Contributor Author

scagood commented May 16, 2024

ESLint v9 is not actually fixed in this PR 👀

I want to do this lovely little deprecation in a different PR.

(node:1888) DeprecationWarning: "no-multiple-resolved" rule uses CodePath#currentSegments and will stop working in ESLint v9. Please read the documentation for how to update your code: https://eslint.org/docs/latest/extend/code-path-analysis#usage-examples

Copy link
Contributor

@aladdin-add aladdin-add left a comment

Choose a reason for hiding this comment

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

LGTM. Waiting a few days to see if others want to review.

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.

2 participants