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

Usage of deprecated APIs of ESLint #234

Closed
theoludwig opened this issue Dec 17, 2020 · 9 comments · Fixed by #275
Closed

Usage of deprecated APIs of ESLint #234

theoludwig opened this issue Dec 17, 2020 · 9 comments · Fixed by #275
Assignees
Milestone

Comments

@theoludwig
Copy link
Member

What version of this package are you using?

standard-engine@14.0.1

What problem do you want to solve?

I don't think we should use deprecated APIs of other packages. See ESLint Node.js API.
Currently standard-engine supports any version of eslint >= 1.0.0, we should instead support any version of eslint >= 7.0.0 to be able to use the "new" ESLint class API.

What do you think is the correct solution to this problem?

Refactor to make it work with the ESLint class. (BREAKING CHANGE)
For example at this line, we're using CLIEngine :

return new this.eslint.CLIEngine(opts.eslintConfig).executeOnText(text, opts.filename)

@theoludwig
Copy link
Member Author

I was trying to start a PR to fix this issue, but that would be a little bit harder and bigger than I thought.
The returns values and the options of the Linter methods change.
So before doing the work, I want to have some feedback, so we can go in the right direction.
Note: It is a much-needed update because the next major of eslint (v8) will not work, as it will remove the CLIEngine class.

  • I'm asking if we really need the sync and callback APIs, we could only provide a Promise based API like eslint does.
    We would have these 3 methods :

    • Linter.lintText and Linter.lintFiles that returns a Promise
    • Linter.parseOpts
      So we can remove Linter.lintTextSync.
  • Also doing theses big BREAKING CHANGE, we can take the opportunity to refactor to use ES6 class instead of prototypes.

Thoughts ? @standard/team

@voxpelli
Copy link
Member

voxpelli commented Aug 14, 2021

Good catch @divlo!

I generally agree with your thoughts on converting to Promises / async methods and converting to ES6 classes could be a good idea as well.

Some thoughts:

  • Who are dependent on this code? Is it just the modules within this org or is third parties also depending on it?
  • Um, that was probably my only current thought 🤔

@voxpelli
Copy link
Member

Should also bump minimum Node.js version from 10.x to 12.x then as well

@theoludwig
Copy link
Member Author

Who are dependent on this code? Is it just the modules within this org or is third parties also depending on it?

Yes, it is mostly repos within standard org.
Who is using standard-engine?
We would also need to update the VSCode extension (vscode-standard).

Should also bump minimum Node.js version from 10.x to 12.x then as well

Yes, it's finally time to drop Node.js v10 support, as it reached End Of Life.

@theoludwig
Copy link
Member Author

The first v8.0.0 beta prerelease has been published! (see: eslint/eslint#14872 and https://eslint.org/blog/2021/08/eslint-v8.0.0-beta.0-released)
It supports many new features, including Top-level await. 🎉

There is a migration guide available: https://github.com/eslint/eslint/blob/master/docs/user-guide/migrating-to-8.0.0.md

@voxpelli
Copy link
Member

Exciting stuff @divlo!

Should we target being compatible with ESLint 8 or require ESLint 8 for the 15.0.0 milestone?

Maybe have the require as a future major release, but already now open an issue to track what we would like to make use of when we're ESLint 8 only?

@theoludwig
Copy link
Member Author

Should we target being compatible with ESLint 8 or require ESLint 8 for the 15.0.0 milestone?

I think being compatible is enough for the 15.0.0 milestone.
We could require eslint >= 7.0.0 for 15.0.0.

Maybe have the require as a future major release, but already now open an issue to track what we would like to make use of when we're ESLint 8 only?

Sure!

@voxpelli
Copy link
Member

I'll beginning to work on this

@voxpelli
Copy link
Member

Draft PR is up: #275

Can you give it a look @divlo?

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

Successfully merging a pull request may close this issue.

2 participants