standard / standard-engine

:fire_engine: The guts of `standard` modularized for reuse
MIT License
145 stars 39 forks source link

Usage of deprecated APIs of ESLint #234

Closed theoludwig closed 2 years ago

theoludwig commented 3 years ago

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 : https://github.com/standard/standard-engine/blob/b695a0219dfbf236f02e6ae0f540e0a9bc0c0862/index.js#L78

theoludwig commented 3 years ago

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.

Thoughts ? @standard/team

voxpelli commented 3 years ago

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:

voxpelli commented 3 years ago

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

theoludwig commented 3 years ago

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 commented 3 years ago

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

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

voxpelli commented 3 years ago

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 commented 3 years ago

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 commented 2 years ago

I'll beginning to work on this

voxpelli commented 2 years ago

Draft PR is up: https://github.com/standard/standard-engine/pull/275

Can you give it a look @divlo?