From b1840be75f172b7388e72ba6e5c04175da796fda Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Sat, 28 Sep 2019 23:03:20 +0900 Subject: [PATCH 01/47] New: Moving to Asynchronous API --- designs/2019-move-to-async-api/README.md | 74 ++++++++++++++++++++++++ 1 file changed, 74 insertions(+) create mode 100644 designs/2019-move-to-async-api/README.md diff --git a/designs/2019-move-to-async-api/README.md b/designs/2019-move-to-async-api/README.md new file mode 100644 index 00000000..eb619b98 --- /dev/null +++ b/designs/2019-move-to-async-api/README.md @@ -0,0 +1,74 @@ +- Start Date: 2019-09-28 +- RFC PR: (leave this empty, to be filled in later) +- Authors: Toru Nagashima ([@mysticatea](https://github.com/mysticatea)) + +# Moving to Asynchronous API + +## Summary + +This RFC changes the methods of `CLIEngine` class returning `Promise` and adds `CLIEngineSync` class for soft-migration. + +## Motivation + +- Dynamic loading of ES modules requires asynchronous API. Migrating to asynchronous API opens up doors to write plugins/configs with ES modules. +- Linting in parallel requires asynchronous API. We can improve linting logic to run it in parallel. And we can improve config loading logic and file enumeration logic to run it in parallel. (E.g., load `extends` list, traverse child directories.) + +## Detailed Design + +### Change the methods of `CLIEngine` class + +This RFC changes the following methods as returning `Promise`. + +- `executeOnFiles()` +- `executeOnText()` +- `getConfigForFile()` +- `getFormatter()` +- `isPathIgnored()` +- `outputFixes()` + +The following methods are as-is because those don't touch both file system and module system. + +- `addPlugin()` +- `getErrorResults()` +- `getRules()` +- `resolveFileGlobPatterns()` + +### Add `CLIEngineSync` class + +This RFC adds a new public class `CLIEngineSync` for soft-migration. The `CLIEngineSync` class has the completely same interface as the current `CLIEngine`. + +### Deprecate `CLIEngineSync` class + +This RFC soft-deprecates the new public class `CLIEngineSync`. + +Because it's tough to maintain two versions (sync and async) of implementation. The two are almost copy-pasted stuff, but hard to share the code. Therefore, this RFC deprecates the sync version to improve our code with the way which needs asynchronous behavior in the future. For example, `CLIEngineSync` cannot support parallel linting, plugins/configs as ES modules, etc... + +### Out of scope + +- Not change API for rules. This RFC doesn't change APIs that rule implementation uses. We may be able to support asynchronous stuff in rules in the future, but it's out of this RFC's scope. +- Not change internal logics. This RFC just changes the public interface. The internal logic is still synchronous. + +## Documentation + +- This change needs the entry in the migration guide because of a breaking change. +- The "[Node.js API](https://eslint.org/docs/developer-guide/nodejs-api)" page should describe the new public API. + +## Drawbacks + +People that use `CLIEngine` have to update their application with the new asynchronous API. This may be hard work. + +## Backwards Compatibility Analysis + +Yes, this is a drastic breaking change. New `CLIEngineSync` class is for soft-migration. + +## Alternatives + +- Adding `engine.executeAsyncOnFiles()`-like methods rather than changing existing methods. +- Adding new `CLIEngineAsync`-like class that has async API rather than we move the existing APIs to `CLIEngineSync`. + +## Related Discussions + +- https://github.com/eslint/eslint/issues/3565 - Lint multiple files in parallel +- https://github.com/eslint/eslint/issues/12319 - `ERR_REQUIRE_ESM` when requiring `.eslintrc.js` +- https://github.com/eslint/rfcs/pull/4 - New: added draft for async/parallel proposal +- https://github.com/eslint/rfcs/pull/11 - New: Lint files in parallel From 6135383b3b506176fe641a52dad67b25da748f08 Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Sat, 28 Sep 2019 23:06:33 +0900 Subject: [PATCH 02/47] add PR link --- designs/2019-move-to-async-api/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/designs/2019-move-to-async-api/README.md b/designs/2019-move-to-async-api/README.md index eb619b98..73b7a81c 100644 --- a/designs/2019-move-to-async-api/README.md +++ b/designs/2019-move-to-async-api/README.md @@ -1,5 +1,5 @@ - Start Date: 2019-09-28 -- RFC PR: (leave this empty, to be filled in later) +- RFC PR: https://github.com/eslint/rfcs/pull/40 - Authors: Toru Nagashima ([@mysticatea](https://github.com/mysticatea)) # Moving to Asynchronous API From 0af99b0af7f379a0d5e3566e9aa7e91a7d2eca48 Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Sun, 29 Sep 2019 01:04:38 +0900 Subject: [PATCH 03/47] fix to not change existing methods --- designs/2019-move-to-async-api/README.md | 37 ++++++++++++------------ 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/designs/2019-move-to-async-api/README.md b/designs/2019-move-to-async-api/README.md index 73b7a81c..36c5395b 100644 --- a/designs/2019-move-to-async-api/README.md +++ b/designs/2019-move-to-async-api/README.md @@ -6,18 +6,23 @@ ## Summary -This RFC changes the methods of `CLIEngine` class returning `Promise` and adds `CLIEngineSync` class for soft-migration. +This RFC adds a new class `ESLint` that has asynchronous API and deprecates `CLIEngine`. ## Motivation -- Dynamic loading of ES modules requires asynchronous API. Migrating to asynchronous API opens up doors to write plugins/configs with ES modules. +- Dynamic `import()` has arrived at Stage 4. The dynamic loading of ES modules requires asynchronous API. Migrating to asynchronous API opens up doors to write plugins/configs with ES modules. - Linting in parallel requires asynchronous API. We can improve linting logic to run it in parallel. And we can improve config loading logic and file enumeration logic to run it in parallel. (E.g., load `extends` list, traverse child directories.) +Because the migration of public API needs long time, we should start to migrate our APIs earlier. + +And the name of `CLIEngine`, our primary API, causes confusion to the community. People try `Linter` class at first, then they notice it doesn't work as expected. We have a lot of issues that say "please use `CLIEngine` instead." +The name of new class, `ESLint`, is our primary API clearly. + ## Detailed Design -### Change the methods of `CLIEngine` class +### Add new `ESLint` class -This RFC changes the following methods as returning `Promise`. +This RFC adds a new class `ESLint`. It has almost the same methods as `CLIEngine`, but the following methods return `Promise`. - `executeOnFiles()` - `executeOnText()` @@ -33,38 +38,34 @@ The following methods are as-is because those don't touch both file system and m - `getRules()` - `resolveFileGlobPatterns()` -### Add `CLIEngineSync` class - -This RFC adds a new public class `CLIEngineSync` for soft-migration. The `CLIEngineSync` class has the completely same interface as the current `CLIEngine`. +### Deprecate `CLIEngine` class -### Deprecate `CLIEngineSync` class +This RFC soft-deprecates `CLIEngine` class. -This RFC soft-deprecates the new public class `CLIEngineSync`. - -Because it's tough to maintain two versions (sync and async) of implementation. The two are almost copy-pasted stuff, but hard to share the code. Therefore, this RFC deprecates the sync version to improve our code with the way which needs asynchronous behavior in the future. For example, `CLIEngineSync` cannot support parallel linting, plugins/configs as ES modules, etc... +Because it's tough to maintain two versions (sync and async) of implementation. The two are almost copy-pasted stuff, but hard to share the code. Therefore, this RFC deprecates the sync version to improve our code with the way which needs asynchronous behavior in the future. For example, `CLIEngine` cannot support parallel linting, plugins/configs as ES modules, etc... ### Out of scope - Not change API for rules. This RFC doesn't change APIs that rule implementation uses. We may be able to support asynchronous stuff in rules in the future, but it's out of this RFC's scope. -- Not change internal logics. This RFC just changes the public interface. The internal logic is still synchronous. +- Not change internal logics. This RFC just adds the public interface that is asynchronous. It would be a wrapper of `CLIEngine` for now. ## Documentation -- This change needs the entry in the migration guide because of a breaking change. -- The "[Node.js API](https://eslint.org/docs/developer-guide/nodejs-api)" page should describe the new public API. +- The "[Node.js API](https://eslint.org/docs/developer-guide/nodejs-api)" page should describe the new public API and deprecation of `CLIEngine` class. ## Drawbacks -People that use `CLIEngine` have to update their application with the new asynchronous API. This may be hard work. +People that use `CLIEngine` have to update their application with the new API. It will need hard work. ## Backwards Compatibility Analysis -Yes, this is a drastic breaking change. New `CLIEngineSync` class is for soft-migration. +Deprecating `CLIEngine` is a drastic change. But people can continue to use `CLIEngine` as-is until we decide to remove it. The decision would not be near future. + +We can do both adding a new class and the deprecation in a minor release. ## Alternatives -- Adding `engine.executeAsyncOnFiles()`-like methods rather than changing existing methods. -- Adding new `CLIEngineAsync`-like class that has async API rather than we move the existing APIs to `CLIEngineSync`. +- Adding `engine.executeAsyncOnFiles()`-like methods and we maintain it along with the existing synchronous API. But as what I wrote in the "[Deprecate `CLIEngine` class](#deprecate-cliengine-class)" section, it would be tough. ## Related Discussions From d9444ef00263b7a190478a59b53c5f93a6eb43f8 Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Wed, 2 Oct 2019 07:51:22 +0900 Subject: [PATCH 04/47] change getFormatter --- designs/2019-move-to-async-api/README.md | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/designs/2019-move-to-async-api/README.md b/designs/2019-move-to-async-api/README.md index 36c5395b..329af274 100644 --- a/designs/2019-move-to-async-api/README.md +++ b/designs/2019-move-to-async-api/README.md @@ -27,10 +27,32 @@ This RFC adds a new class `ESLint`. It has almost the same methods as `CLIEngine - `executeOnFiles()` - `executeOnText()` - `getConfigForFile()` -- `getFormatter()` - `isPathIgnored()` - `outputFixes()` +And the following method returns a `(results: LintReport, metadata: any) => stream.Readable`. + +- `getFormatter()` + +
+ +```js + getFormatter(name) { + const formatter = cliEngine.getFormatter(name) + return (...args) => { + const text = formatter(...args) + const s = new stream.PassThrough() + process.nextTick(() => { + s.write(text) + s.end() + }) + return s + } + } +``` + +
+ The following methods are as-is because those don't touch both file system and module system. - `addPlugin()` From e5cc07c6a60c072142d23b59be7b114def3af7be Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Wed, 2 Oct 2019 11:58:55 +0900 Subject: [PATCH 05/47] fix getFormatter --- designs/2019-move-to-async-api/README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/designs/2019-move-to-async-api/README.md b/designs/2019-move-to-async-api/README.md index 329af274..143a9652 100644 --- a/designs/2019-move-to-async-api/README.md +++ b/designs/2019-move-to-async-api/README.md @@ -30,7 +30,7 @@ This RFC adds a new class `ESLint`. It has almost the same methods as `CLIEngine - `isPathIgnored()` - `outputFixes()` -And the following method returns a `(results: LintReport, metadata: any) => stream.Readable`. +And the following method returns a `Promise<(results: LintReport, metadata: any) => stream.Readable>`. - `getFormatter()` @@ -39,7 +39,7 @@ And the following method returns a `(results: LintReport, metadata: any) => stre ```js getFormatter(name) { const formatter = cliEngine.getFormatter(name) - return (...args) => { + return Promise.resolve((...args) => { const text = formatter(...args) const s = new stream.PassThrough() process.nextTick(() => { @@ -47,7 +47,7 @@ And the following method returns a `(results: LintReport, metadata: any) => stre s.end() }) return s - } + }) } ``` From be5878cb23c9fe41a3bbd49256393f0db7d1c5e1 Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Thu, 3 Oct 2019 16:17:38 +0900 Subject: [PATCH 06/47] update --- designs/2019-move-to-async-api/README.md | 172 ++++++++++++++++++++--- 1 file changed, 150 insertions(+), 22 deletions(-) diff --git a/designs/2019-move-to-async-api/README.md b/designs/2019-move-to-async-api/README.md index 143a9652..a0274a81 100644 --- a/designs/2019-move-to-async-api/README.md +++ b/designs/2019-move-to-async-api/README.md @@ -15,50 +15,178 @@ This RFC adds a new class `ESLint` that has asynchronous API and deprecates `CLI Because the migration of public API needs long time, we should start to migrate our APIs earlier. -And the name of `CLIEngine`, our primary API, causes confusion to the community. People try `Linter` class at first, then they notice it doesn't work as expected. We have a lot of issues that say "please use `CLIEngine` instead." +And the name of `CLIEngine`, our primary API, causes confusion to the community. People try `Linter` class at first because the name sounds the main class of ESLint, then they notice it doesn't work as expected. We have a lot of issues that say "please use `CLIEngine` instead." The name of new class, `ESLint`, is our primary API clearly. ## Detailed Design ### Add new `ESLint` class -This RFC adds a new class `ESLint`. It has almost the same methods as `CLIEngine`, but the following methods return `Promise`. +This RFC adds a new class `ESLint`. It has almost the same methods as `CLIEngine`, but the return value of some methods are different. -- `executeOnFiles()` -- `executeOnText()` -- `getConfigForFile()` -- `isPathIgnored()` -- `outputFixes()` +So, for now, `ESLint` class will be a tiny wrapper of `CLIEngine` that modifies the type of the return values. + +#### The `executeOnFiles()` method + +This method returns a `AsyncIterator` object iterates the lint result of each file in random order. + +
+A rough sketch of the `executeOnFiles()` method. + +```js +class ESLint { + async *executeOnFiles(patterns) { + // Verify files and push the results. + for (const result of this.cliEngine.executeOnFiles(patterns).results) { + yield result + } + } +} +``` + +
+ +
+Example: Show the results step by step. + +```js +const { ESLint } = require("eslint") +const eslint = new ESLint() + +for await (const result of eslint.executeOnFiles(patterns)) { + print(result) +} +``` + +
+ +
+Example: Show the results in the stable order. + +```js +const { ESLint } = require("eslint") +const eslint = new ESLint() +const results = [] + +for await (const result of eslint.executeOnFiles(patterns)) { + results.push(result) +} + +results.sort(byFilePath) +print(results) +``` + +
+ +Once the `executeOnFiles()` method got this change, we can support "linting in parallel", streaming, and plugins/configs which are ES modules in the future. -And the following method returns a `Promise<(results: LintReport, metadata: any) => stream.Readable>`. +#### The `getFormatter()` method -- `getFormatter()` +This method returns a `Promise`. The `Formatter` type is a function `(results: AsyncIterator) => AsyncIterator`. It receives lint results then outputs the formatted text. + +This means the `getFormatter()` method wraps the current formatter to align the interface.
+A rough sketch of the `getFormatter()` method. ```js - getFormatter(name) { - const formatter = cliEngine.getFormatter(name) - return Promise.resolve((...args) => { - const text = formatter(...args) - const s = new stream.PassThrough() - process.nextTick(() => { - s.write(text) - s.end() - }) - return s - }) +class ESLint { + async getFormatter(name) { + const format = this.cliEngine.getFormatter(name) + + // Return the wrapper. + return async function* formatter(resultIterator) { + // Collect the results. + const results = [] + for await (const result of resultIterator) { + results.push(result) + } + results.sort(byFilePath) + + // Make `rulesMeta`. + const rules = this.cliEngine.getRules() + const rulesMeta = getRulesMeta(rules) + + // Format the results with the formatter of the current spec. + yield format(results, { rulesMeta }) } + } +} +``` + +
+ +
+Example: Use the formatter. + +```js +const { ESLint } = require("eslint") +const eslint = new ESLint() +const formatter = eslint.getFormatter("stylish") + +// Verify files +const results = eslint.executeOnFiles(patterns) +// Format and write the results +for await (const textPiece of formatter(results)) { + process.stdout.write(textPiece) +} ```
+Once the `getFormatter()` method got this change, we can update the specification of custom formatters without breakage in the future to support streaming. + +#### The other methods + +The following methods return `Promise` which gets fulfilled with each result. + +- `executeOnText()` +- `getConfigForFile()` +- `isPathIgnored()` +- `outputFixes()` + +Once the former three methods got this change, we can support plugins/configs that are ES modules without breakage in the future. And once the `outputFixes()` method got this change, we can write many files more efficiently in the future. + The following methods are as-is because those don't touch both file system and module system. - `addPlugin()` -- `getErrorResults()` - `getRules()` -- `resolveFileGlobPatterns()` +- `getErrorResults()` + +The following methods are removed because those don't fit the current API. + +- `resolveFileGlobPatterns()` ... ESLint doesn't use this logic since `v6.0.0`, but it has stayed there for backward compatibility. Once [RFC 20](https://github.com/eslint/rfcs/tree/master/designs/2019-additional-lint-targets) is implemented, what ESLint iterates and what the glob of this method iterates will be different, then it will confuse users. This is good timing to remove the legacy. + +#### A new `filterErrorResults()` static method + +Existing `getErrorResults()` method doesn't fit the new `executeOnFiles()` method because the result is an async iterator. + +The new `filterErrorResults()` method receives an `AsyncIterator` object and returns an `AsyncIterator` object. It extracts only the lint messages which are `severity === 2`, abandons the other messages. + +
+A rough sketch of the `filterErrorResults()` static method. + +```js +class ESLint { + static async *filterErrorResults(results) { + for await (const result of results) { + const messages = result.messages.filter(m => m.severity === 2) + + if (messages.length === result.messages.length) { + yield result + } + yield { + ...result, + messages, + warningCount: 0, + fixableWarningCount: 0, + } + } + } +} +``` + +
### Deprecate `CLIEngine` class From 628e262aa6df3e2f765d7ec0dc351cb8b4e824dd Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Thu, 3 Oct 2019 17:32:06 +0900 Subject: [PATCH 07/47] update Backwards Compatibility Analysis section --- designs/2019-move-to-async-api/README.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/designs/2019-move-to-async-api/README.md b/designs/2019-move-to-async-api/README.md index a0274a81..a6b332e3 100644 --- a/designs/2019-move-to-async-api/README.md +++ b/designs/2019-move-to-async-api/README.md @@ -209,9 +209,11 @@ People that use `CLIEngine` have to update their application with the new API. I ## Backwards Compatibility Analysis -Deprecating `CLIEngine` is a drastic change. But people can continue to use `CLIEngine` as-is until we decide to remove it. The decision would not be near future. +This is a breaking change. -We can do both adding a new class and the deprecation in a minor release. +Deprecating `CLIEngine` is a drastic change. But people can continue to use `CLIEngine` as-is until we decide to remove it. + +The new API depends on [Asynchronous Iteration](https://github.com/tc39/proposal-async-iteration) syntax. Node.js supports the syntax since `10.0.0`, so we have to drop Node.js `8.x`. Because the `8.x` will be EOL in December 2019 (two months later!), we can work on this soon. ## Alternatives From cade0bddc967a8d410780790493431d8a2f5f5ac Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Thu, 3 Oct 2019 17:52:09 +0900 Subject: [PATCH 08/47] add an example --- designs/2019-move-to-async-api/README.md | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/designs/2019-move-to-async-api/README.md b/designs/2019-move-to-async-api/README.md index a6b332e3..76af4280 100644 --- a/designs/2019-move-to-async-api/README.md +++ b/designs/2019-move-to-async-api/README.md @@ -188,6 +188,28 @@ class ESLint { +
+Example: Use `filterErrorResults()`. + +```js +const { ESLint } = require("eslint") +const eslint = new ESLint() +const formatter = eslint.getFormatter("stylish") + +// Verify files +let results = eslint.executeOnFiles(patterns) +// Filter the results if needed +if (process.argv.includes("--quiet")) { + results = ESLint.filterErrorResults(results) +} +// Format and write the results +for await (const textPiece of formatter(results)) { + process.stdout.write(textPiece) +} +``` + +
+ ### Deprecate `CLIEngine` class This RFC soft-deprecates `CLIEngine` class. From c99ae5677429885c38c6cf012223a9d039966428 Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Thu, 3 Oct 2019 17:54:01 +0900 Subject: [PATCH 09/47] trivial fix --- designs/2019-move-to-async-api/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/designs/2019-move-to-async-api/README.md b/designs/2019-move-to-async-api/README.md index 76af4280..102e4455 100644 --- a/designs/2019-move-to-async-api/README.md +++ b/designs/2019-move-to-async-api/README.md @@ -161,7 +161,7 @@ The following methods are removed because those don't fit the current API. Existing `getErrorResults()` method doesn't fit the new `executeOnFiles()` method because the result is an async iterator. -The new `filterErrorResults()` method receives an `AsyncIterator` object and returns an `AsyncIterator` object. It extracts only the lint messages which are `severity === 2`, abandons the other messages. +The new `filterErrorResults()` method receives an `AsyncIterator` object and returns an `AsyncIterator` object. It extracts only the lint messages which are `severity === 2` and abandons the other messages.
A rough sketch of the `filterErrorResults()` static method. From 730cb5817fb1758fd5371411722a709cc306fc42 Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Thu, 3 Oct 2019 18:05:31 +0900 Subject: [PATCH 10/47] Add a note about streams --- designs/2019-move-to-async-api/README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/designs/2019-move-to-async-api/README.md b/designs/2019-move-to-async-api/README.md index 102e4455..55c81990 100644 --- a/designs/2019-move-to-async-api/README.md +++ b/designs/2019-move-to-async-api/README.md @@ -240,6 +240,7 @@ The new API depends on [Asynchronous Iteration](https://github.com/tc39/proposal ## Alternatives - Adding `engine.executeAsyncOnFiles()`-like methods and we maintain it along with the existing synchronous API. But as what I wrote in the "[Deprecate `CLIEngine` class](#deprecate-cliengine-class)" section, it would be tough. +- Using [Streams](https://nodejs.org/dist/latest-v12.x/docs/api/stream.html) instead of [Asynchronous Iteration](https://github.com/tc39/proposal-async-iteration). We can introduce `ESLint` class in a minor release if we used Streams. But because Node.js 8 will be EOL two months later, we should be able to use Asynchronous Iteration soon. Iterator protocol is smaller spec than streams, and it's easy to use. ## Related Discussions From 2ab427fa9f8960345357bb50ffa3084bfa07f876 Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Thu, 3 Oct 2019 18:29:12 +0900 Subject: [PATCH 11/47] trivial fix --- designs/2019-move-to-async-api/README.md | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/designs/2019-move-to-async-api/README.md b/designs/2019-move-to-async-api/README.md index 55c81990..f02d4629 100644 --- a/designs/2019-move-to-async-api/README.md +++ b/designs/2019-move-to-async-api/README.md @@ -20,13 +20,13 @@ The name of new class, `ESLint`, is our primary API clearly. ## Detailed Design -### Add new `ESLint` class +### ■ Add new `ESLint` class This RFC adds a new class `ESLint`. It has almost the same methods as `CLIEngine`, but the return value of some methods are different. So, for now, `ESLint` class will be a tiny wrapper of `CLIEngine` that modifies the type of the return values. -#### The `executeOnFiles()` method +#### § The `executeOnFiles()` method This method returns a `AsyncIterator` object iterates the lint result of each file in random order. @@ -80,7 +80,7 @@ print(results) Once the `executeOnFiles()` method got this change, we can support "linting in parallel", streaming, and plugins/configs which are ES modules in the future. -#### The `getFormatter()` method +#### § The `getFormatter()` method This method returns a `Promise`. The `Formatter` type is a function `(results: AsyncIterator) => AsyncIterator`. It receives lint results then outputs the formatted text. @@ -136,7 +136,7 @@ for await (const textPiece of formatter(results)) { Once the `getFormatter()` method got this change, we can update the specification of custom formatters without breakage in the future to support streaming. -#### The other methods +#### § The other methods The following methods return `Promise` which gets fulfilled with each result. @@ -157,7 +157,7 @@ The following methods are removed because those don't fit the current API. - `resolveFileGlobPatterns()` ... ESLint doesn't use this logic since `v6.0.0`, but it has stayed there for backward compatibility. Once [RFC 20](https://github.com/eslint/rfcs/tree/master/designs/2019-additional-lint-targets) is implemented, what ESLint iterates and what the glob of this method iterates will be different, then it will confuse users. This is good timing to remove the legacy. -#### A new `filterErrorResults()` static method +#### § A new `filterErrorResults()` static method Existing `getErrorResults()` method doesn't fit the new `executeOnFiles()` method because the result is an async iterator. @@ -210,13 +210,13 @@ for await (const textPiece of formatter(results)) {
-### Deprecate `CLIEngine` class +### ■ Deprecate `CLIEngine` class This RFC soft-deprecates `CLIEngine` class. Because it's tough to maintain two versions (sync and async) of implementation. The two are almost copy-pasted stuff, but hard to share the code. Therefore, this RFC deprecates the sync version to improve our code with the way which needs asynchronous behavior in the future. For example, `CLIEngine` cannot support parallel linting, plugins/configs as ES modules, etc... -### Out of scope +### ■ Out of scope - Not change API for rules. This RFC doesn't change APIs that rule implementation uses. We may be able to support asynchronous stuff in rules in the future, but it's out of this RFC's scope. - Not change internal logics. This RFC just adds the public interface that is asynchronous. It would be a wrapper of `CLIEngine` for now. From ed350229c349ca27c5a8336cd9b1cdce29f84239 Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Thu, 3 Oct 2019 18:38:45 +0900 Subject: [PATCH 12/47] update getErrorResults since it's sandwiched --- designs/2019-move-to-async-api/README.md | 55 +++++++++++------------- 1 file changed, 26 insertions(+), 29 deletions(-) diff --git a/designs/2019-move-to-async-api/README.md b/designs/2019-move-to-async-api/README.md index f02d4629..0b71dd6a 100644 --- a/designs/2019-move-to-async-api/README.md +++ b/designs/2019-move-to-async-api/README.md @@ -136,39 +136,16 @@ for await (const textPiece of formatter(results)) { Once the `getFormatter()` method got this change, we can update the specification of custom formatters without breakage in the future to support streaming. -#### § The other methods - -The following methods return `Promise` which gets fulfilled with each result. - -- `executeOnText()` -- `getConfigForFile()` -- `isPathIgnored()` -- `outputFixes()` - -Once the former three methods got this change, we can support plugins/configs that are ES modules without breakage in the future. And once the `outputFixes()` method got this change, we can write many files more efficiently in the future. - -The following methods are as-is because those don't touch both file system and module system. - -- `addPlugin()` -- `getRules()` -- `getErrorResults()` - -The following methods are removed because those don't fit the current API. - -- `resolveFileGlobPatterns()` ... ESLint doesn't use this logic since `v6.0.0`, but it has stayed there for backward compatibility. Once [RFC 20](https://github.com/eslint/rfcs/tree/master/designs/2019-additional-lint-targets) is implemented, what ESLint iterates and what the glob of this method iterates will be different, then it will confuse users. This is good timing to remove the legacy. - -#### § A new `filterErrorResults()` static method - -Existing `getErrorResults()` method doesn't fit the new `executeOnFiles()` method because the result is an async iterator. +#### § The `getErrorResults()` method -The new `filterErrorResults()` method receives an `AsyncIterator` object and returns an `AsyncIterator` object. It extracts only the lint messages which are `severity === 2` and abandons the other messages. +As related to the above, this method now receives an `AsyncIterator` object and returns an `AsyncIterator` object. Because this method is sandwiched between `executeOnFiles()` and formatters.
-A rough sketch of the `filterErrorResults()` static method. +A rough sketch of the `getErrorResults()` static method. ```js class ESLint { - static async *filterErrorResults(results) { + static async *getErrorResults(results) { for await (const result of results) { const messages = result.messages.filter(m => m.severity === 2) @@ -189,7 +166,7 @@ class ESLint {
-Example: Use `filterErrorResults()`. +Example: Use `getErrorResults()`. ```js const { ESLint } = require("eslint") @@ -200,7 +177,7 @@ const formatter = eslint.getFormatter("stylish") let results = eslint.executeOnFiles(patterns) // Filter the results if needed if (process.argv.includes("--quiet")) { - results = ESLint.filterErrorResults(results) + results = ESLint.getErrorResults(results) } // Format and write the results for await (const textPiece of formatter(results)) { @@ -210,6 +187,26 @@ for await (const textPiece of formatter(results)) {
+#### § The other methods + +The following methods return `Promise` which gets fulfilled with each result. + +- `executeOnText()` +- `getConfigForFile()` +- `isPathIgnored()` +- `outputFixes()` + +Once the former three methods got this change, we can support plugins/configs that are ES modules without breakage in the future. And once the `outputFixes()` method got this change, we can write many files more efficiently in the future. + +The following methods are as-is because those don't touch both file system and module system. + +- `addPlugin()` +- `getRules()` + +The following methods are removed because those don't fit the current API. + +- `resolveFileGlobPatterns()` ... ESLint doesn't use this logic since `v6.0.0`, but it has stayed there for backward compatibility. Once [RFC 20](https://github.com/eslint/rfcs/tree/master/designs/2019-additional-lint-targets) is implemented, what ESLint iterates and what the glob of this method iterates will be different, then it will confuse users. This is good timing to remove the legacy. + ### ■ Deprecate `CLIEngine` class This RFC soft-deprecates `CLIEngine` class. From 13354ce18ce623c246050646ff87c80cb1809538 Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Fri, 4 Oct 2019 21:12:05 +0900 Subject: [PATCH 13/47] add fail-fast note --- designs/2019-move-to-async-api/README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/designs/2019-move-to-async-api/README.md b/designs/2019-move-to-async-api/README.md index 0b71dd6a..792afdb3 100644 --- a/designs/2019-move-to-async-api/README.md +++ b/designs/2019-move-to-async-api/README.md @@ -80,6 +80,8 @@ print(results) Once the `executeOnFiles()` method got this change, we can support "linting in parallel", streaming, and plugins/configs which are ES modules in the future. +⚠️ Because this method updates the cache file, if people call this method in parallel then it causes broken. To prevent the broken, it should throw an error if people called this method while the previous call is still running. + #### § The `getFormatter()` method This method returns a `Promise`. The `Formatter` type is a function `(results: AsyncIterator) => AsyncIterator`. It receives lint results then outputs the formatted text. From 2aa4a5e377cbd0fe54cf6d6d89bcb33c988fd122 Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Tue, 8 Oct 2019 21:52:05 +0900 Subject: [PATCH 14/47] update - change executeOnFiles method to support simple await expression - change executeOnText method to align the return type to executeOnFiles method - add ESLint.compareResultsByFilePath static method --- designs/2019-move-to-async-api/README.md | 183 ++++++++++++++++------- 1 file changed, 125 insertions(+), 58 deletions(-) diff --git a/designs/2019-move-to-async-api/README.md b/designs/2019-move-to-async-api/README.md index 792afdb3..428dc503 100644 --- a/designs/2019-move-to-async-api/README.md +++ b/designs/2019-move-to-async-api/README.md @@ -10,13 +10,9 @@ This RFC adds a new class `ESLint` that has asynchronous API and deprecates `CLI ## Motivation +- We have the functionality that we cannot support with synchronous API. For example, ESLint verifies files in parallel, formatters print progress state, or formatters print results in streaming. - Dynamic `import()` has arrived at Stage 4. The dynamic loading of ES modules requires asynchronous API. Migrating to asynchronous API opens up doors to write plugins/configs with ES modules. -- Linting in parallel requires asynchronous API. We can improve linting logic to run it in parallel. And we can improve config loading logic and file enumeration logic to run it in parallel. (E.g., load `extends` list, traverse child directories.) - -Because the migration of public API needs long time, we should start to migrate our APIs earlier. - -And the name of `CLIEngine`, our primary API, causes confusion to the community. People try `Linter` class at first because the name sounds the main class of ESLint, then they notice it doesn't work as expected. We have a lot of issues that say "please use `CLIEngine` instead." -The name of new class, `ESLint`, is our primary API clearly. +- The name of `CLIEngine`, our primary API, has caused confusion to the community. People try `Linter` class at first because the name sounds the main class of ESLint, then they notice it doesn't work as expected. We have a lot of issues that say "please use `CLIEngine` instead." The name of the new class, `ESLint`, is our primary API clearly. ## Detailed Design @@ -28,17 +24,40 @@ So, for now, `ESLint` class will be a tiny wrapper of `CLIEngine` that modifies #### § The `executeOnFiles()` method -This method returns a `AsyncIterator` object iterates the lint result of each file in random order. +This method returns an `AsyncIterable & Thennable` object. It means that we can use the returned value with `for-await-of` syntax or `await` syntax. + +- If the returned value is used with `for-await-of`, it iterates the lint result of each file in random order. This way yields each lint result immediately. Therefore people can print the results in streaming, or print progress state. +- If the returned value is used with `await`, it returns the array contains all lint results in random order. This way waits for all lint results then returns it. + +Either way, we can support "linting in parallel", loading configs/plugins with `import()`, and other stuff that needs asynchronous logic.
A rough sketch of the `executeOnFiles()` method. +A tiny wrapper of `CLIEngine`. + ```js class ESLint { - async *executeOnFiles(patterns) { - // Verify files and push the results. - for (const result of this.cliEngine.executeOnFiles(patterns).results) { - yield result + executeOnFiles(patterns) { + try { + const { results } = this.cliEngine.executeOnFiles(patterns) + return { + then(onFulfilled) { + return Promise.resolve(results).then(onFulfilled) + }, + async *[Symbol.asyncIterator]() { + yield* results + }, + } + } catch (error) { + return { + then(_, onRejected) { + return Promise.reject(error).then(undefined, onRejected) + }, + [Symbol.asyncIterator]() { + return Promise.reject(error) + }, + } } } } @@ -61,36 +80,84 @@ for await (const result of eslint.executeOnFiles(patterns)) {
-Example: Show the results in the stable order. +Example: Show the results at once. ```js const { ESLint } = require("eslint") const eslint = new ESLint() -const results = [] - -for await (const result of eslint.executeOnFiles(patterns)) { - results.push(result) -} -results.sort(byFilePath) +const results = await eslint.executeOnFiles(patterns) print(results) ```
-Once the `executeOnFiles()` method got this change, we can support "linting in parallel", streaming, and plugins/configs which are ES modules in the future. +
+Example: Show the results in the stable order. + +The [ESLint.compareResultsByFilePath](#-a-new-compareresultsbyfilepath-static-method) static method is useful to sort results. + +```js +const { ESLint } = require("eslint") +const eslint = new ESLint() + +const results = await eslint.executeOnFiles(patterns) +results.sort(ESLint.compareResultsByFilePath) +print(results) +``` + +
⚠️ Because this method updates the cache file, if people call this method in parallel then it causes broken. To prevent the broken, it should throw an error if people called this method while the previous call is still running. +#### § The `executeOnText()` method + +This method returns an `AsyncIterable & Thennable` object (the same type as the `executeOnFiles()` method). + +Because the returned value of `CLIEngine#executeOnText()` method is the same type as the `CLIEngine#executeOnFiles()` method. The `ESLint` class inherits that mannar. + +
+Example: Show the result. + +```js +const { ESLint } = require("eslint") +const eslint = new ESLint() + +const [result] = await eslint.executeOnText(text, filePath) +print(result) +``` + +
+ +
+Example: Using along with the `executeOnFiles()` method. + +```js +const { ESLint } = require("eslint") +const eslint = new ESLint() + +const report = useStdin + ? eslint.executeOnText(text, filePath) + : eslint.executeOnFiles(patterns) + +for await (const result of report) { + print(result) +} +``` + +
+ #### § The `getFormatter()` method -This method returns a `Promise`. The `Formatter` type is a function `(results: AsyncIterator) => AsyncIterator`. It receives lint results then outputs the formatted text. +This method returns a `Promise`. The `Formatter` type is a function `(results: AsyncIterable) => AsyncIterableIterator`. It receives lint results then outputs the formatted text. -This means the `getFormatter()` method wraps the current formatter to align the interface. +This means the `getFormatter()` method wraps the current formatter to adapt the interface.
A rough sketch of the `getFormatter()` method. +The [ESLint.compareResultsByFilePath](#-a-new-compareresultsbyfilepath-static-method) static method is useful to sort results. + ```js class ESLint { async getFormatter(name) { @@ -103,7 +170,7 @@ class ESLint { for await (const result of resultIterator) { results.push(result) } - results.sort(byFilePath) + results.sort(ESLint.compareResultsByFilePath) // Make `rulesMeta`. const rules = this.cliEngine.getRules() @@ -138,37 +205,15 @@ for await (const textPiece of formatter(results)) { Once the `getFormatter()` method got this change, we can update the specification of custom formatters without breakage in the future to support streaming. -#### § The `getErrorResults()` method - -As related to the above, this method now receives an `AsyncIterator` object and returns an `AsyncIterator` object. Because this method is sandwiched between `executeOnFiles()` and formatters. - -
-A rough sketch of the `getErrorResults()` static method. - -```js -class ESLint { - static async *getErrorResults(results) { - for await (const result of results) { - const messages = result.messages.filter(m => m.severity === 2) +#### § The other methods - if (messages.length === result.messages.length) { - yield result - } - yield { - ...result, - messages, - warningCount: 0, - fixableWarningCount: 0, - } - } - } -} -``` +The following methods receive an `AsyncIterable` object as the first argument then return an `AsyncIterableIterator` object because those methods are sandwiched between `executeOnFiles()` and formatters. -
+- `outputFixes()` +- `getErrorResults()`
-Example: Use `getErrorResults()`. +Example: Use `outputFixes()` and `getErrorResults()`. ```js const { ESLint } = require("eslint") @@ -177,6 +222,11 @@ const formatter = eslint.getFormatter("stylish") // Verify files let results = eslint.executeOnFiles(patterns) +// Update the files of the results if needed +// (This must be done before `--quiet` for backward compatibility.) +if (process.argv.includes("--fix")) { + results = ESLint.outputFixes(results) +} // Filter the results if needed if (process.argv.includes("--quiet")) { results = ESLint.getErrorResults(results) @@ -189,26 +239,43 @@ for await (const textPiece of formatter(results)) {
-#### § The other methods - -The following methods return `Promise` which gets fulfilled with each result. +The following methods return `Promise` which gets fulfilled with each result in order to support plugins/configs that are ES modules without breakage in the future. -- `executeOnText()` - `getConfigForFile()` - `isPathIgnored()` -- `outputFixes()` - -Once the former three methods got this change, we can support plugins/configs that are ES modules without breakage in the future. And once the `outputFixes()` method got this change, we can write many files more efficiently in the future. The following methods are as-is because those don't touch both file system and module system. - `addPlugin()` - `getRules()` -The following methods are removed because those don't fit the current API. +The following method is removed because it doesn't fit the current API. - `resolveFileGlobPatterns()` ... ESLint doesn't use this logic since `v6.0.0`, but it has stayed there for backward compatibility. Once [RFC 20](https://github.com/eslint/rfcs/tree/master/designs/2019-additional-lint-targets) is implemented, what ESLint iterates and what the glob of this method iterates will be different, then it will confuse users. This is good timing to remove the legacy. +#### § A new `compareResultsByFilePath` static method + +This method receives two lint results then returns `+1`, `-1`, or `0`. This method is intended to use in order to sort results. + +
+A rough sketch of the `compareResultsByFilePath()` method. + +```js +class ESLint { + static compareResultsByFilePath(a, b) { + if (a.filePath < b.filePath) { + return -1 + } + if (a.filePath > b.filePath) { + return 1 + } + return 0 + } +} +``` + +
+ ### ■ Deprecate `CLIEngine` class This RFC soft-deprecates `CLIEngine` class. @@ -239,7 +306,7 @@ The new API depends on [Asynchronous Iteration](https://github.com/tc39/proposal ## Alternatives - Adding `engine.executeAsyncOnFiles()`-like methods and we maintain it along with the existing synchronous API. But as what I wrote in the "[Deprecate `CLIEngine` class](#deprecate-cliengine-class)" section, it would be tough. -- Using [Streams](https://nodejs.org/dist/latest-v12.x/docs/api/stream.html) instead of [Asynchronous Iteration](https://github.com/tc39/proposal-async-iteration). We can introduce `ESLint` class in a minor release if we used Streams. But because Node.js 8 will be EOL two months later, we should be able to use Asynchronous Iteration soon. Iterator protocol is smaller spec than streams, and it's easy to use. +- Using [Streams](https://nodejs.org/dist/latest-v12.x/docs/api/stream.html) instead of [Asynchronous Iteration](https://github.com/tc39/proposal-async-iteration). We can introduce `ESLint` class in a minor release if we used Streams. But because Node.js 8 will be EOL two months later, we should be able to use Asynchronous Iteration soon. And Streams don't support `for-await-of` syntax until `v11.14.0`. Iterator protocol is smaller spec than streams, and it's easy to use. ## Related Discussions From 8892dc734ec74182cd8d379be0e27fd0a04efbc2 Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Sun, 13 Oct 2019 15:44:48 +0900 Subject: [PATCH 15/47] refactor and add about usedDeprecatedRules --- designs/2019-move-to-async-api/README.md | 116 +++++++++++------------ 1 file changed, 53 insertions(+), 63 deletions(-) diff --git a/designs/2019-move-to-async-api/README.md b/designs/2019-move-to-async-api/README.md index 428dc503..55295040 100644 --- a/designs/2019-move-to-async-api/README.md +++ b/designs/2019-move-to-async-api/README.md @@ -24,97 +24,89 @@ So, for now, `ESLint` class will be a tiny wrapper of `CLIEngine` that modifies #### § The `executeOnFiles()` method -This method returns an `AsyncIterable & Thennable` object. It means that we can use the returned value with `for-await-of` syntax or `await` syntax. +This method returns an object that has two properties two methods `then()` and `[Symbol.asyncIterator]()`, and we can use the returned object with `await` expression and `for-await-of` statement. -- If the returned value is used with `for-await-of`, it iterates the lint result of each file in random order. This way yields each lint result immediately. Therefore people can print the results in streaming, or print progress state. -- If the returned value is used with `await`, it returns the array contains all lint results in random order. This way waits for all lint results then returns it. +- If you used the returned object with `for-await-of` statement, it iterates the lint result of each file in random order. This way yields each lint result immediately. Therefore you can print the results in streaming, or print progress state. ESLint may spend time to lint files (for example, ESLint needs about 20 seconds to lint our codebase), to print progress state will be useful. -Either way, we can support "linting in parallel", loading configs/plugins with `import()`, and other stuff that needs asynchronous logic. + ```js + const { ESLint } = require("eslint") + const eslint = new ESLint() -
-A rough sketch of the `executeOnFiles()` method. + for await (const result of eslint.executeOnFiles(patterns)) { + print(result) + } + ``` -A tiny wrapper of `CLIEngine`. +- If you used the returned object with `await` expression, it returns the array that contains all lint results in random order. This way waits for all lint results then returns it. -```js -class ESLint { - executeOnFiles(patterns) { - try { - const { results } = this.cliEngine.executeOnFiles(patterns) - return { - then(onFulfilled) { - return Promise.resolve(results).then(onFulfilled) - }, - async *[Symbol.asyncIterator]() { - yield* results - }, - } - } catch (error) { - return { - then(_, onRejected) { - return Promise.reject(error).then(undefined, onRejected) - }, - [Symbol.asyncIterator]() { - return Promise.reject(error) - }, - } - } - } -} -``` + ```js + const { ESLint } = require("eslint") + const eslint = new ESLint() -
+ const results = await eslint.executeOnFiles(patterns) + // Optionally you can sort the results. + // results.sort(ESLint.compareResultsByFilePath) + print(results) + ``` -
-Example: Show the results step by step. +Either way, we can support "linting in parallel", loading configs/plugins with `import()`, and other stuff that needs asynchronous logic. + +##### Move the `usedDeprecatedRules` property + +The returned object of `CLIEngine#executeOnFiles()` has the `usedDeprecatedRules` property that includes the deprecated rule IDs which the linting used. But the location doesn't fit the use with `for-await-of` statement. Therefore, this RFC mvoes the `usedDeprecatedRules` property to each lint result. ```js const { ESLint } = require("eslint") const eslint = new ESLint() for await (const result of eslint.executeOnFiles(patterns)) { - print(result) + console.log(result.usedDeprecatedRules) } ``` -
+As a side-effect, formatters gets the capability to print the used deprecated rules. (!?) -
-Example: Show the results at once. +##### Fail-fast on parallel calling -```js -const { ESLint } = require("eslint") -const eslint = new ESLint() +Because this method updates the cache file, if people call this method in parallel then it causes broken. To prevent the broken, it should throw an error if people called this method while the previous call is still running. -const results = await eslint.executeOnFiles(patterns) -print(results) -``` - -
+##### Implementation
-Example: Show the results in the stable order. +A rough sketch of the `executeOnFiles()` method. -The [ESLint.compareResultsByFilePath](#-a-new-compareresultsbyfilepath-static-method) static method is useful to sort results. +A tiny wrapper of `CLIEngine`. ```js -const { ESLint } = require("eslint") -const eslint = new ESLint() +class ESLint { + executeOnFiles(patterns) { + let promise + try { + const report = this.cliEngine.executeOnFiles(patterns) + promise = Promise.resolve(report.results) + } catch (error) { + promise = Promise.reject(error) + } -const results = await eslint.executeOnFiles(patterns) -results.sort(ESLint.compareResultsByFilePath) -print(results) + return { + then(onFulfilled, onRejected) { + return promise.then(onFulfilled, onRejected) + }, + async *[Symbol.asyncIterator]() { + yield* await promise + }, + } + } +} ```
-⚠️ Because this method updates the cache file, if people call this method in parallel then it causes broken. To prevent the broken, it should throw an error if people called this method while the previous call is still running. - #### § The `executeOnText()` method -This method returns an `AsyncIterable & Thennable` object (the same type as the `executeOnFiles()` method). +This method returns the same type of an object as the `executeOnFiles()` method. -Because the returned value of `CLIEngine#executeOnText()` method is the same type as the `CLIEngine#executeOnFiles()` method. The `ESLint` class inherits that mannar. +Because the returned object of `CLIEngine#executeOnText()` method is the same type as the `CLIEngine#executeOnFiles()` method. The `ESLint` class inherits that mannar.
Example: Show the result. @@ -156,8 +148,6 @@ This means the `getFormatter()` method wraps the current formatter to adapt the
A rough sketch of the `getFormatter()` method. -The [ESLint.compareResultsByFilePath](#-a-new-compareresultsbyfilepath-static-method) static method is useful to sort results. - ```js class ESLint { async getFormatter(name) { @@ -165,7 +155,7 @@ class ESLint { // Return the wrapper. return async function* formatter(resultIterator) { - // Collect the results. + // Collect and sort the results. const results = [] for await (const result of resultIterator) { results.push(result) @@ -223,7 +213,7 @@ const formatter = eslint.getFormatter("stylish") // Verify files let results = eslint.executeOnFiles(patterns) // Update the files of the results if needed -// (This must be done before `--quiet` for backward compatibility.) +// (This must be done before `--quiet`) if (process.argv.includes("--fix")) { results = ESLint.outputFixes(results) } From 7a94b03564b48edc977be8679c4fc086b97c6767 Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Sun, 13 Oct 2019 15:53:10 +0900 Subject: [PATCH 16/47] fix typo --- designs/2019-move-to-async-api/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/designs/2019-move-to-async-api/README.md b/designs/2019-move-to-async-api/README.md index 55295040..070b72dc 100644 --- a/designs/2019-move-to-async-api/README.md +++ b/designs/2019-move-to-async-api/README.md @@ -24,7 +24,7 @@ So, for now, `ESLint` class will be a tiny wrapper of `CLIEngine` that modifies #### § The `executeOnFiles()` method -This method returns an object that has two properties two methods `then()` and `[Symbol.asyncIterator]()`, and we can use the returned object with `await` expression and `for-await-of` statement. +This method returns an object that has two methods `then()` and `[Symbol.asyncIterator]()`, so we can use the returned object with `await` expression and `for-await-of` statement. - If you used the returned object with `for-await-of` statement, it iterates the lint result of each file in random order. This way yields each lint result immediately. Therefore you can print the results in streaming, or print progress state. ESLint may spend time to lint files (for example, ESLint needs about 20 seconds to lint our codebase), to print progress state will be useful. From 7b5e401b1713291a8127618c9dae8cad5cfda386 Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Mon, 14 Oct 2019 09:27:59 +0900 Subject: [PATCH 17/47] fix typo --- designs/2019-move-to-async-api/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/designs/2019-move-to-async-api/README.md b/designs/2019-move-to-async-api/README.md index 070b72dc..8c9361a8 100644 --- a/designs/2019-move-to-async-api/README.md +++ b/designs/2019-move-to-async-api/README.md @@ -53,7 +53,7 @@ Either way, we can support "linting in parallel", loading configs/plugins with ` ##### Move the `usedDeprecatedRules` property -The returned object of `CLIEngine#executeOnFiles()` has the `usedDeprecatedRules` property that includes the deprecated rule IDs which the linting used. But the location doesn't fit the use with `for-await-of` statement. Therefore, this RFC mvoes the `usedDeprecatedRules` property to each lint result. +The returned object of `CLIEngine#executeOnFiles()` has the `usedDeprecatedRules` property that includes the deprecated rule IDs which the linting used. But the location doesn't fit the use with `for-await-of` statement. Therefore, this RFC moves the `usedDeprecatedRules` property to each lint result. ```js const { ESLint } = require("eslint") From 29812287b19663d1535cc05205f36de2fe68ae07 Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Mon, 14 Oct 2019 10:04:11 +0900 Subject: [PATCH 18/47] add about aborting --- designs/2019-move-to-async-api/README.md | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/designs/2019-move-to-async-api/README.md b/designs/2019-move-to-async-api/README.md index 8c9361a8..07f4ba9a 100644 --- a/designs/2019-move-to-async-api/README.md +++ b/designs/2019-move-to-async-api/README.md @@ -70,6 +70,25 @@ As a side-effect, formatters gets the capability to print the used deprecated ru Because this method updates the cache file, if people call this method in parallel then it causes broken. To prevent the broken, it should throw an error if people called this method while the previous call is still running. +##### Abort linting + +The iterator has optional [`return()` method](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Generator/return). The `for-of`/`for-await-of` syntax calls the `return()` method automatically if the execution escaped from the loop by `braek`, `return`, or `throw`. In short, the `return()` method will be called when aborted. + +Therefore, ESLint aborts linting when the `return()` method is called. This will mean that the `return()` method terminates all workers once we implement parallel linting. + +```js +const { ESLint } = require("eslint") +const eslint = new ESLint() + +for await (const result of eslint.executeOnFiles(patterns)) { + if (Math.random() < 0.5) { + break // abort linting. + } +} +``` + +And it throws an error if you reuse the aborted results. + ##### Implementation
From 72e69af02205d03ef0e1657979fe908d1ddfab87 Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Mon, 14 Oct 2019 18:23:02 +0900 Subject: [PATCH 19/47] add constructor and remove addPlugin --- designs/2019-move-to-async-api/README.md | 101 ++++++++++++++++++++++- 1 file changed, 98 insertions(+), 3 deletions(-) diff --git a/designs/2019-move-to-async-api/README.md b/designs/2019-move-to-async-api/README.md index 07f4ba9a..1a6b13c0 100644 --- a/designs/2019-move-to-async-api/README.md +++ b/designs/2019-move-to-async-api/README.md @@ -22,6 +22,100 @@ This RFC adds a new class `ESLint`. It has almost the same methods as `CLIEngine So, for now, `ESLint` class will be a tiny wrapper of `CLIEngine` that modifies the type of the return values. +#### § Constructor + +The constructor has the most same arguments as `CLIEngine`, but there are small differences. + +- It throws fatal errors if the options contain unknown properties or an option is invalid type ([eslint/eslint#10272](https://github.com/eslint/eslint/issues/10272)). +- It disallows the deprecated `cacheFile` option. +- It has a new `pluginImplementations` option as the successor of `addPlugin()` method. This is an object that keys are plugin IDs and each value is the plugin object. + +##### Implementation + +
+A rough sketch of the constructor. + +```js +class ESLint { + constructor({ + allowInlineConfig = true, + baseConfig = null, + cache = false, + cacheLocation = ".eslintcache", + configFile = null, + cwd = process.cwd(), + envs = [], + extensions = [".js"], + fix = false, + fixTypes = ["problem", "suggestion", "layout"], + globInputPaths = true, + globals = [], + ignore = true, + ignorePath = null, + ignorePattern = [], + parser = "espree", + parserOptions = null, + pluginImplementations = null, + plugins = [], + reportUnusedDisableDirectives = false, + resolvePluginsRelativeTo = cwd, + rulePaths = [], + rules = null, + useEslintrc = true, + ...unknownOptions + } = {}) { + // Throws on unknown options + { + const keys = Object.keys(unknownOptions) + if (keys.length >= 1) { + //... + } + } + // Throws on the invalid value of options + if (typeof allowInlineConfig !== "boolean") { + // ... + } + // ... + + // Initialize CLIEngine because this is a tiny wrapper. + const engine = (this._cliEngine = new CLIEngine({ + allowInlineConfig, + baseConfig, + cache, + cacheLocation, + configFile, + cwd, + envs, + extensions, + fix, + fixTypes, + globInputPaths, + globals, + ignore, + ignorePath, + ignorePattern, + parser, + parserOptions, + plugins, + reportUnusedDisableDirectives, + resolvePluginsRelativeTo, + rulePaths, + rules, + useEslintrc, + })) + + // Apply `pluginImplementations` option. + if (pluginImplementations) { + for (const [id, definition] of Object.entries(pluginImplementations)) { + engine.addPlugin(id, definition) + } + } + } +} +``` + +
+ #### § The `executeOnFiles()` method This method returns an object that has two methods `then()` and `[Symbol.asyncIterator]()`, so we can use the returned object with `await` expression and `for-await-of` statement. @@ -253,13 +347,13 @@ The following methods return `Promise` which gets fulfilled with each result in - `getConfigForFile()` - `isPathIgnored()` -The following methods are as-is because those don't touch both file system and module system. +The following method is as-is because those don't touch both file system and module system. -- `addPlugin()` - `getRules()` -The following method is removed because it doesn't fit the current API. +The following methods are removed because those don't fit the current API. +- `addPlugin()` ... This method has caused to confuse people. We have introduced this method to add plugin implementations and expected people to use this method to test plugins, but people have thought that this method loads a new plugin for the following linting. And this method is only one that mutates the state of `CLIEngine` objects (except cache). This RFC moves this functionality to the constructor options. - `resolveFileGlobPatterns()` ... ESLint doesn't use this logic since `v6.0.0`, but it has stayed there for backward compatibility. Once [RFC 20](https://github.com/eslint/rfcs/tree/master/designs/2019-additional-lint-targets) is implemented, what ESLint iterates and what the glob of this method iterates will be different, then it will confuse users. This is good timing to remove the legacy. #### § A new `compareResultsByFilePath` static method @@ -320,6 +414,7 @@ The new API depends on [Asynchronous Iteration](https://github.com/tc39/proposal ## Related Discussions - https://github.com/eslint/eslint/issues/3565 - Lint multiple files in parallel +- https://github.com/eslint/eslint/issues/10272 - Validate options passed to CLIEngine API - https://github.com/eslint/eslint/issues/12319 - `ERR_REQUIRE_ESM` when requiring `.eslintrc.js` - https://github.com/eslint/rfcs/pull/4 - New: added draft for async/parallel proposal - https://github.com/eslint/rfcs/pull/11 - New: Lint files in parallel From 86689ec197b427a1854bfcadabf1271ca7871aa2 Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Tue, 15 Oct 2019 20:48:32 +0900 Subject: [PATCH 20/47] Update motivation section Co-Authored-By: Pelle Wessman --- designs/2019-move-to-async-api/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/designs/2019-move-to-async-api/README.md b/designs/2019-move-to-async-api/README.md index 1a6b13c0..28cdc4d5 100644 --- a/designs/2019-move-to-async-api/README.md +++ b/designs/2019-move-to-async-api/README.md @@ -10,7 +10,7 @@ This RFC adds a new class `ESLint` that has asynchronous API and deprecates `CLI ## Motivation -- We have the functionality that we cannot support with synchronous API. For example, ESLint verifies files in parallel, formatters print progress state, or formatters print results in streaming. +- We have functionality that cannot be supported with the current synchronous API. For example, ESLint verifying files in parallel, formatters printing progress state, formatters printing results in streams etc. A move to an asynchronous API would be beneficial and a new `ESLint` class can be created with an async API in mind from the start. - Dynamic `import()` has arrived at Stage 4. The dynamic loading of ES modules requires asynchronous API. Migrating to asynchronous API opens up doors to write plugins/configs with ES modules. - The name of `CLIEngine`, our primary API, has caused confusion to the community. People try `Linter` class at first because the name sounds the main class of ESLint, then they notice it doesn't work as expected. We have a lot of issues that say "please use `CLIEngine` instead." The name of the new class, `ESLint`, is our primary API clearly. From a76d6c0d49dadcec3c30a4f10b0af3d2a049fb0e Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Tue, 15 Oct 2019 20:50:55 +0900 Subject: [PATCH 21/47] Update motivation section Co-Authored-By: Pelle Wessman --- designs/2019-move-to-async-api/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/designs/2019-move-to-async-api/README.md b/designs/2019-move-to-async-api/README.md index 28cdc4d5..632fb4e1 100644 --- a/designs/2019-move-to-async-api/README.md +++ b/designs/2019-move-to-async-api/README.md @@ -12,7 +12,7 @@ This RFC adds a new class `ESLint` that has asynchronous API and deprecates `CLI - We have functionality that cannot be supported with the current synchronous API. For example, ESLint verifying files in parallel, formatters printing progress state, formatters printing results in streams etc. A move to an asynchronous API would be beneficial and a new `ESLint` class can be created with an async API in mind from the start. - Dynamic `import()` has arrived at Stage 4. The dynamic loading of ES modules requires asynchronous API. Migrating to asynchronous API opens up doors to write plugins/configs with ES modules. -- The name of `CLIEngine`, our primary API, has caused confusion to the community. People try `Linter` class at first because the name sounds the main class of ESLint, then they notice it doesn't work as expected. We have a lot of issues that say "please use `CLIEngine` instead." The name of the new class, `ESLint`, is our primary API clearly. +- The name of `CLIEngine`, our primary API, has caused confusion in the community and is sub-optimal. We have a lot of issues that say "please use `CLIEngine` instead.". A new class, `ESLint`, while fixing other issues, will also make our primary API more clear. ## Detailed Design From face206bf5c80180a63f6fcedfa24985a1b1313a Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Tue, 15 Oct 2019 20:52:54 +0900 Subject: [PATCH 22/47] Update design overview Co-Authored-By: Pelle Wessman --- designs/2019-move-to-async-api/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/designs/2019-move-to-async-api/README.md b/designs/2019-move-to-async-api/README.md index 632fb4e1..0fba983b 100644 --- a/designs/2019-move-to-async-api/README.md +++ b/designs/2019-move-to-async-api/README.md @@ -20,7 +20,7 @@ This RFC adds a new class `ESLint` that has asynchronous API and deprecates `CLI This RFC adds a new class `ESLint`. It has almost the same methods as `CLIEngine`, but the return value of some methods are different. -So, for now, `ESLint` class will be a tiny wrapper of `CLIEngine` that modifies the type of the return values. +Initially the `ESLint` class will be a wrapper around `CLIEngine`, modifying return types. Later it can take on a more independent shape as `CLIEngine` gets more deprecated. #### § Constructor From 65b67f51b27906552cef6405f2e6fd01fa56cf38 Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Tue, 15 Oct 2019 20:53:56 +0900 Subject: [PATCH 23/47] Update constructor description Co-Authored-By: Pelle Wessman --- designs/2019-move-to-async-api/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/designs/2019-move-to-async-api/README.md b/designs/2019-move-to-async-api/README.md index 0fba983b..0890f04b 100644 --- a/designs/2019-move-to-async-api/README.md +++ b/designs/2019-move-to-async-api/README.md @@ -24,7 +24,7 @@ Initially the `ESLint` class will be a wrapper around `CLIEngine`, modifying ret #### § Constructor -The constructor has the most same arguments as `CLIEngine`, but there are small differences. +The constructor has mostly the same options as `CLIEngine`, but with some small differences: - It throws fatal errors if the options contain unknown properties or an option is invalid type ([eslint/eslint#10272](https://github.com/eslint/eslint/issues/10272)). - It disallows the deprecated `cacheFile` option. From cedb6e67ebbc7e2e0f7f5b03c8228309178aa1c7 Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Tue, 15 Oct 2019 22:00:23 +0900 Subject: [PATCH 24/47] Update fail-fast on parallel calling section Co-Authored-By: Pelle Wessman --- designs/2019-move-to-async-api/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/designs/2019-move-to-async-api/README.md b/designs/2019-move-to-async-api/README.md index 0890f04b..82f3ba23 100644 --- a/designs/2019-move-to-async-api/README.md +++ b/designs/2019-move-to-async-api/README.md @@ -162,7 +162,7 @@ As a side-effect, formatters gets the capability to print the used deprecated ru ##### Fail-fast on parallel calling -Because this method updates the cache file, if people call this method in parallel then it causes broken. To prevent the broken, it should throw an error if people called this method while the previous call is still running. +Because this method updates the cache file, it will break of called multiple times in parallel. To prevent that, it should throw an error if the method is called while a previous call is still running. ##### Abort linting From 8f58df938971a9d4c6542e3b280b5172d00ecaf0 Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Tue, 15 Oct 2019 22:04:54 +0900 Subject: [PATCH 25/47] Update designs/2019-move-to-async-api/README.md Co-Authored-By: Pelle Wessman --- designs/2019-move-to-async-api/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/designs/2019-move-to-async-api/README.md b/designs/2019-move-to-async-api/README.md index 82f3ba23..1327eeb8 100644 --- a/designs/2019-move-to-async-api/README.md +++ b/designs/2019-move-to-async-api/README.md @@ -168,7 +168,7 @@ Because this method updates the cache file, it will break of called multiple tim The iterator has optional [`return()` method](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Generator/return). The `for-of`/`for-await-of` syntax calls the `return()` method automatically if the execution escaped from the loop by `braek`, `return`, or `throw`. In short, the `return()` method will be called when aborted. -Therefore, ESLint aborts linting when the `return()` method is called. This will mean that the `return()` method terminates all workers once we implement parallel linting. +ESLint aborts all linting when the `return()` method is called. This will eg. terminate all workers if parallel linting is implemented. ```js const { ESLint } = require("eslint") From 227fe54ff7f3482aff7b66040430d63bc0e37042 Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Wed, 16 Oct 2019 08:23:41 +0900 Subject: [PATCH 26/47] Update designs/2019-move-to-async-api/README.md Co-Authored-By: Pelle Wessman --- designs/2019-move-to-async-api/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/designs/2019-move-to-async-api/README.md b/designs/2019-move-to-async-api/README.md index 1327eeb8..751b0ae4 100644 --- a/designs/2019-move-to-async-api/README.md +++ b/designs/2019-move-to-async-api/README.md @@ -347,7 +347,7 @@ The following methods return `Promise` which gets fulfilled with each result in - `getConfigForFile()` - `isPathIgnored()` -The following method is as-is because those don't touch both file system and module system. +The following method is as-is because they touch neither file system or module system. - `getRules()` From 4c5cdff0bc77d3e501cd26af134545aaad879c39 Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Thu, 17 Oct 2019 11:49:39 +0900 Subject: [PATCH 27/47] update RFC for more clarification and... - add `errorsOnly` constructor option as the successor of `getErrorResults()`. - rename `outputFixes()` --- designs/2019-move-to-async-api/README.md | 110 +++++++++++++---------- 1 file changed, 64 insertions(+), 46 deletions(-) diff --git a/designs/2019-move-to-async-api/README.md b/designs/2019-move-to-async-api/README.md index 751b0ae4..f3fe548f 100644 --- a/designs/2019-move-to-async-api/README.md +++ b/designs/2019-move-to-async-api/README.md @@ -2,33 +2,34 @@ - RFC PR: https://github.com/eslint/rfcs/pull/40 - Authors: Toru Nagashima ([@mysticatea](https://github.com/mysticatea)) -# Moving to Asynchronous API +# `ESLint` Class Replacing `CLIEngine` ## Summary -This RFC adds a new class `ESLint` that has asynchronous API and deprecates `CLIEngine`. +This RFC adds a new class `ESLint` that provides asynchronous API and deprecates `CLIEngine`. ## Motivation - We have functionality that cannot be supported with the current synchronous API. For example, ESLint verifying files in parallel, formatters printing progress state, formatters printing results in streams etc. A move to an asynchronous API would be beneficial and a new `ESLint` class can be created with an async API in mind from the start. -- Dynamic `import()` has arrived at Stage 4. The dynamic loading of ES modules requires asynchronous API. Migrating to asynchronous API opens up doors to write plugins/configs with ES modules. +- Node.js will support [ES modules](https://nodejs.org/api/esm.html) stably on `13.0.0` at last. Node.js doesn't provide any way that loads ES modules synchronously from CJS. This means that ESLint (CJS) cannot load configs/plugins that are written as ES modules synchronously. Migrating to asynchronous API opens up doors to support those. - The name of `CLIEngine`, our primary API, has caused confusion in the community and is sub-optimal. We have a lot of issues that say "please use `CLIEngine` instead.". A new class, `ESLint`, while fixing other issues, will also make our primary API more clear. ## Detailed Design -### ■ Add new `ESLint` class +### Add new `ESLint` class This RFC adds a new class `ESLint`. It has almost the same methods as `CLIEngine`, but the return value of some methods are different. Initially the `ESLint` class will be a wrapper around `CLIEngine`, modifying return types. Later it can take on a more independent shape as `CLIEngine` gets more deprecated. -#### § Constructor +#### ● Constructor -The constructor has mostly the same options as `CLIEngine`, but with some small differences: +The constructor has mostly the same options as `CLIEngine`, but with some differences: - It throws fatal errors if the options contain unknown properties or an option is invalid type ([eslint/eslint#10272](https://github.com/eslint/eslint/issues/10272)). - It disallows the deprecated `cacheFile` option. -- It has a new `pluginImplementations` option as the successor of `addPlugin()` method. This is an object that keys are plugin IDs and each value is the plugin object. +- It has a new `errorsOnly` option as the successor of `getErrorResults()` method. This option is a boolean value and defaults to `false`. If `true` is present then ESLint disables the rules which are configured with the warning severity. This option corresponds to [`--quiet` CLI option](https://eslint.org/docs/user-guide/command-line-interface#--quiet). See also "[The other methods](#-the-other-methods)" section. +- It has a new `pluginImplementations` option as the successor of `addPlugin()` method. This is an object that keys are plugin IDs and each value is the plugin object. See also "[The other methods](#-the-other-methods)" section. ##### Implementation @@ -45,6 +46,7 @@ class ESLint { configFile = null, cwd = process.cwd(), envs = [], + errorsOnly = false, extensions = [".js"], fix = false, fixTypes = ["problem", "suggestion", "layout"], @@ -65,17 +67,17 @@ class ESLint { ...unknownOptions } = {}) { // Throws on unknown options - { - const keys = Object.keys(unknownOptions) - if (keys.length >= 1) { - //... - } + if (Object.keys(unknownOptions).length >= 1) { + //... } // Throws on the invalid value of options if (typeof allowInlineConfig !== "boolean") { // ... } - // ... + if (typeof baseConfig !== "object") { + // ... + } + // and other options... // Initialize CLIEngine because this is a tiny wrapper. const engine = (this._cliEngine = new CLIEngine({ @@ -116,11 +118,11 @@ class ESLint {
-#### § The `executeOnFiles()` method +#### ● The `executeOnFiles()` method This method returns an object that has two methods `then()` and `[Symbol.asyncIterator]()`, so we can use the returned object with `await` expression and `for-await-of` statement. -- If you used the returned object with `for-await-of` statement, it iterates the lint result of each file in random order. This way yields each lint result immediately. Therefore you can print the results in streaming, or print progress state. ESLint may spend time to lint files (for example, ESLint needs about 20 seconds to lint our codebase), to print progress state will be useful. +- If you used the returned object with `for-await-of` statement, it iterates the lint result of each file in random order. This way yields each lint result immediately. Therefore you can print the results in streaming, or print progress state. ESLint may spend time to lint files (for example, ESLint needs about 20 seconds to lint [our codebase](https://github.com/eslint/eslint)), to print progress state will be useful. ```js const { ESLint } = require("eslint") @@ -143,11 +145,31 @@ This method returns an object that has two methods `then()` and `[Symbol.asyncIt print(results) ``` -Either way, we can support "linting in parallel", loading configs/plugins with `import()`, and other stuff that needs asynchronous logic. +Either way, we can support [linting in parallel](https://github.com/eslint/rfcs/pull/42), loading configs/plugins with `import()`, and other stuff that needs asynchronous logic. + +##### Iterate only one time + +We can iterate the returned object of this method only one time similar to generators. + +```js +const { ESLint } = require("eslint") +const eslint = new ESLint() + +const resultGenerator = eslint.executeOnFiles(patterns) +for await (const result of resultGenerator) { + print(result) +} +// ↓ Throw "This generator has been consumed already" +for await (const result of resultGenerator) { + print(result) +} +``` ##### Move the `usedDeprecatedRules` property -The returned object of `CLIEngine#executeOnFiles()` has the `usedDeprecatedRules` property that includes the deprecated rule IDs which the linting used. But the location doesn't fit the use with `for-await-of` statement. Therefore, this RFC moves the `usedDeprecatedRules` property to each lint result. +The returned object of `CLIEngine#executeOnFiles()` has the `usedDeprecatedRules` property that includes the deprecated rule IDs which the linting used. + +But the location doesn't fit asynchronous because the used deprecated rule list is not determined until the iteration finished. Therefore, this RFC moves the `usedDeprecatedRules` property to each lint result. ```js const { ESLint } = require("eslint") @@ -158,7 +180,7 @@ for await (const result of eslint.executeOnFiles(patterns)) { } ``` -As a side-effect, formatters gets the capability to print the used deprecated rules. (!?) +As a side-effect, formatters gets the capability to print the used deprecated rules. Previously, ESLint has not passed the returned object to formatters, so the formatters could not print used deprecated rules. After this RFC, each lint result has the `usedDeprecatedRules` property and the formatters receive those. ##### Fail-fast on parallel calling @@ -166,9 +188,9 @@ Because this method updates the cache file, it will break of called multiple tim ##### Abort linting -The iterator has optional [`return()` method](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Generator/return). The `for-of`/`for-await-of` syntax calls the `return()` method automatically if the execution escaped from the loop by `braek`, `return`, or `throw`. In short, the `return()` method will be called when aborted. +The iterator protocol has optional [`return()` method](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Generator/return) that forces to finish the iterator. The `for-of`/`for-await-of` syntax calls the `return()` method automatically if the loop is stopped through a `braek`, `return`, or `throw`. -ESLint aborts all linting when the `return()` method is called. This will eg. terminate all workers if parallel linting is implemented. +ESLint aborts linting when the `return()` method is called. For example, it will terminate all workers if [RFC42](https://github.com/eslint/rfcs/pull/42) is implemented. ```js const { ESLint } = require("eslint") @@ -181,8 +203,6 @@ for await (const result of eslint.executeOnFiles(patterns)) { } ``` -And it throws an error if you reuse the aborted results. - ##### Implementation
@@ -213,9 +233,11 @@ class ESLint { } ``` +But once [RFC42](https://github.com/eslint/rfcs/pull/42) is implemented, the returned object will be an instance of [`LintResultGenerator`](https://github.com/eslint/eslint/blob/836c0e48704d70bc1a5cbdbf0211368b0ada942d/lib/eslint/lint-result-generator.js#L136) class. The class implements async iterable protocol, async iterator protocol, and thenable. +
-#### § The `executeOnText()` method +#### ● The `executeOnText()` method This method returns the same type of an object as the `executeOnFiles()` method. @@ -252,12 +274,14 @@ for await (const result of report) {
-#### § The `getFormatter()` method +#### ● The `getFormatter()` method This method returns a `Promise`. The `Formatter` type is a function `(results: AsyncIterable) => AsyncIterableIterator`. It receives lint results then outputs the formatted text. This means the `getFormatter()` method wraps the current formatter to adapt the interface. +Once the `getFormatter()` method got this change, in the future, we can update the specification of custom formatters without breakage to support streaming. +
A rough sketch of the `getFormatter()` method. @@ -306,17 +330,14 @@ for await (const textPiece of formatter(results)) {
-Once the `getFormatter()` method got this change, we can update the specification of custom formatters without breakage in the future to support streaming. - -#### § The other methods +#### ● The `outputFixesInIteration()` method -The following methods receive an `AsyncIterable` object as the first argument then return an `AsyncIterableIterator` object because those methods are sandwiched between `executeOnFiles()` and formatters. +The original `CLIEngine.outputFixes()` static method writes the fix results to the source code files. -- `outputFixes()` -- `getErrorResults()` +The goal of this method is same as the `CLIEngine.outputFixes()` method, but we cannot share async iterators with this method and formatters, so this method receives an `AsyncIterable` object as the first argument then return an `AsyncIterableIterator` object. This method is sandwiched between `executeOnFiles()` and formatters.
-Example: Use `outputFixes()` and `getErrorResults()`. +Example: Use `outputFixesInIteration()`. ```js const { ESLint } = require("eslint") @@ -326,13 +347,8 @@ const formatter = eslint.getFormatter("stylish") // Verify files let results = eslint.executeOnFiles(patterns) // Update the files of the results if needed -// (This must be done before `--quiet`) if (process.argv.includes("--fix")) { - results = ESLint.outputFixes(results) -} -// Filter the results if needed -if (process.argv.includes("--quiet")) { - results = ESLint.getErrorResults(results) + results = ESLint.outputFixesInIteration(results) } // Format and write the results for await (const textPiece of formatter(results)) { @@ -342,21 +358,21 @@ for await (const textPiece of formatter(results)) {
+#### ● The other methods + The following methods return `Promise` which gets fulfilled with each result in order to support plugins/configs that are ES modules without breakage in the future. - `getConfigForFile()` -- `isPathIgnored()` - -The following method is as-is because they touch neither file system or module system. - - `getRules()` +- `isPathIgnored()` -The following methods are removed because those don't fit the current API. +The following methods are removed because those don't fit the new API. -- `addPlugin()` ... This method has caused to confuse people. We have introduced this method to add plugin implementations and expected people to use this method to test plugins, but people have thought that this method loads a new plugin for the following linting. And this method is only one that mutates the state of `CLIEngine` objects (except cache). This RFC moves this functionality to the constructor options. -- `resolveFileGlobPatterns()` ... ESLint doesn't use this logic since `v6.0.0`, but it has stayed there for backward compatibility. Once [RFC 20](https://github.com/eslint/rfcs/tree/master/designs/2019-additional-lint-targets) is implemented, what ESLint iterates and what the glob of this method iterates will be different, then it will confuse users. This is good timing to remove the legacy. +- `addPlugin()` ... This method has caused to confuse people. We have introduced this method to add plugin implementations and expected people to use this method to test plugins, but people have thought that this method loads a new plugin for the following linting. And this method is only one that mutates the state of `CLIEngine` objects (except cache) and messes all caches. Therefore, this RFC moves this functionality to a constructor option. See also "[Constructor](#-constructor)" section. +- `getErrorResults()` ... This method is a utility to filter warning messages and is used for `--quiet` CLI option. But disabling the rules which are configured with the warning severity is more efficient than the current way. Therefore, this RFC moves this functionality to a constructor option. See also "[Constructor](#-constructor)" section. +- `resolveFileGlobPatterns()` ... ESLint doesn't use this logic since `v6.0.0`, but it has stayed there for backward compatibility. Once [RFC20](https://github.com/eslint/rfcs/tree/master/designs/2019-additional-lint-targets) is implemented, what ESLint iterates and what the glob of this method iterates will be different, then it will confuse users. This is good timing to remove the legacy. -#### § A new `compareResultsByFilePath` static method +#### ● A new `compareResultsByFilePath` static method This method receives two lint results then returns `+1`, `-1`, or `0`. This method is intended to use in order to sort results. @@ -383,7 +399,7 @@ class ESLint { This RFC soft-deprecates `CLIEngine` class. -Because it's tough to maintain two versions (sync and async) of implementation. The two are almost copy-pasted stuff, but hard to share the code. Therefore, this RFC deprecates the sync version to improve our code with the way which needs asynchronous behavior in the future. For example, `CLIEngine` cannot support parallel linting, plugins/configs as ES modules, etc... +Because it's tough to maintain two versions (sync and async) of implementation. The two are almost copy-pasted stuff, but hard to share the code. Therefore, this RFC deprecates the sync version to improve our code with the way which needs asynchronous behavior in the future. For example, `CLIEngine` cannot support [linting in parallel](https://github.com/eslint/rfcs/pull/42), plugins/configs as ES modules, etc... ### ■ Out of scope @@ -413,8 +429,10 @@ The new API depends on [Asynchronous Iteration](https://github.com/tc39/proposal ## Related Discussions +- https://github.com/eslint/eslint/issues/1098 - Show results for each file as eslint is running - https://github.com/eslint/eslint/issues/3565 - Lint multiple files in parallel - https://github.com/eslint/eslint/issues/10272 - Validate options passed to CLIEngine API - https://github.com/eslint/eslint/issues/12319 - `ERR_REQUIRE_ESM` when requiring `.eslintrc.js` - https://github.com/eslint/rfcs/pull/4 - New: added draft for async/parallel proposal - https://github.com/eslint/rfcs/pull/11 - New: Lint files in parallel +- https://github.com/eslint/rfcs/pull/42 - New: Lint files in parallel if many files exist From 00be95b618535723ab054119494f9a251aa28a95 Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Thu, 17 Oct 2019 11:58:10 +0900 Subject: [PATCH 28/47] add a related discussion --- designs/2019-move-to-async-api/README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/designs/2019-move-to-async-api/README.md b/designs/2019-move-to-async-api/README.md index f3fe548f..482dc3f2 100644 --- a/designs/2019-move-to-async-api/README.md +++ b/designs/2019-move-to-async-api/README.md @@ -432,6 +432,7 @@ The new API depends on [Asynchronous Iteration](https://github.com/tc39/proposal - https://github.com/eslint/eslint/issues/1098 - Show results for each file as eslint is running - https://github.com/eslint/eslint/issues/3565 - Lint multiple files in parallel - https://github.com/eslint/eslint/issues/10272 - Validate options passed to CLIEngine API +- https://github.com/eslint/eslint/issues/10606 - Make CLIEngine and supporting functions async via Promises - https://github.com/eslint/eslint/issues/12319 - `ERR_REQUIRE_ESM` when requiring `.eslintrc.js` - https://github.com/eslint/rfcs/pull/4 - New: added draft for async/parallel proposal - https://github.com/eslint/rfcs/pull/11 - New: Lint files in parallel From 756e929f74058be9e163ad5b28bf7f73e6c1da80 Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Mon, 21 Oct 2019 15:38:07 +0900 Subject: [PATCH 29/47] add about cache when abort --- designs/2019-move-to-async-api/README.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/designs/2019-move-to-async-api/README.md b/designs/2019-move-to-async-api/README.md index 482dc3f2..463f253f 100644 --- a/designs/2019-move-to-async-api/README.md +++ b/designs/2019-move-to-async-api/README.md @@ -190,7 +190,10 @@ Because this method updates the cache file, it will break of called multiple tim The iterator protocol has optional [`return()` method](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Generator/return) that forces to finish the iterator. The `for-of`/`for-await-of` syntax calls the `return()` method automatically if the loop is stopped through a `braek`, `return`, or `throw`. -ESLint aborts linting when the `return()` method is called. For example, it will terminate all workers if [RFC42](https://github.com/eslint/rfcs/pull/42) is implemented. +ESLint aborts linting when the `return()` method is called. The abort does: + +- ESLint updates the cache file with the current state when the `return()` method is first called. Therefore, the next time, ESLint can use the cache of the already linted files and lint only the canceled files. +- ESLint will terminate all workers if [RFC42](https://github.com/eslint/rfcs/pull/42) is implemented. ```js const { ESLint } = require("eslint") From 975fa02bc105aa1d9387042450575a5cc497089b Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Mon, 21 Oct 2019 19:46:17 +0900 Subject: [PATCH 30/47] add TOC of methods --- designs/2019-move-to-async-api/README.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/designs/2019-move-to-async-api/README.md b/designs/2019-move-to-async-api/README.md index 463f253f..72a67270 100644 --- a/designs/2019-move-to-async-api/README.md +++ b/designs/2019-move-to-async-api/README.md @@ -20,6 +20,16 @@ This RFC adds a new class `ESLint` that provides asynchronous API and deprecates This RFC adds a new class `ESLint`. It has almost the same methods as `CLIEngine`, but the return value of some methods are different. +- [constructor()](#-constructor) +- [executeOnFiles()](#-the-executeonfiles-method) +- [executeOnText()](#-the-executeontext-method) +- [getFormatter()](#-the-getformatter-method) +- [static outputFixesInIteration()](#-the-outputfixesiniteration-method) (rename) +- [getConfigForFile()](#-the-other-methods) +- [getRules()](#-the-other-methods) +- [isPathIgnored()](#-the-other-methods) +- [static compareResultsByFilePath()](#-new-methods) (new) + Initially the `ESLint` class will be a wrapper around `CLIEngine`, modifying return types. Later it can take on a more independent shape as `CLIEngine` gets more deprecated. #### ● Constructor From f22894b7d9cff4619afdc8299183d04140724982 Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Mon, 21 Oct 2019 19:47:29 +0900 Subject: [PATCH 31/47] update about disallow execution in parallel --- designs/2019-move-to-async-api/README.md | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/designs/2019-move-to-async-api/README.md b/designs/2019-move-to-async-api/README.md index 72a67270..867aefbd 100644 --- a/designs/2019-move-to-async-api/README.md +++ b/designs/2019-move-to-async-api/README.md @@ -157,6 +157,8 @@ This method returns an object that has two methods `then()` and `[Symbol.asyncIt Either way, we can support [linting in parallel](https://github.com/eslint/rfcs/pull/42), loading configs/plugins with `import()`, and other stuff that needs asynchronous logic. +This method must not throw any errors synchronously. + ##### Iterate only one time We can iterate the returned object of this method only one time similar to generators. @@ -192,17 +194,18 @@ for await (const result of eslint.executeOnFiles(patterns)) { As a side-effect, formatters gets the capability to print the used deprecated rules. Previously, ESLint has not passed the returned object to formatters, so the formatters could not print used deprecated rules. After this RFC, each lint result has the `usedDeprecatedRules` property and the formatters receive those. -##### Fail-fast on parallel calling +##### Disallow execution in parallel -Because this method updates the cache file, it will break of called multiple times in parallel. To prevent that, it should throw an error if the method is called while a previous call is still running. +Because this method updates the cache file, it will break the cache file if called multiple times in parallel. To prevent that, every call of `executeOnFiles()` must wait for the previous call finishes. ##### Abort linting The iterator protocol has optional [`return()` method](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Generator/return) that forces to finish the iterator. The `for-of`/`for-await-of` syntax calls the `return()` method automatically if the loop is stopped through a `braek`, `return`, or `throw`. -ESLint aborts linting when the `return()` method is called. The abort does: +ESLint aborts linting when the `return()` method is called. The first `return()` method call does: -- ESLint updates the cache file with the current state when the `return()` method is first called. Therefore, the next time, ESLint can use the cache of the already linted files and lint only the canceled files. +- ESLint cancels the linting of all pending files. +- ESLint updates the cache file with the current state. Therefore, the next time, ESLint can use the cache of the already linted files and lint only the canceled files. - ESLint will terminate all workers if [RFC42](https://github.com/eslint/rfcs/pull/42) is implemented. ```js @@ -216,6 +219,8 @@ for await (const result of eslint.executeOnFiles(patterns)) { } ``` +The second and later calls do nothing. + ##### Implementation
From d45e86e78683574ddc3e4bf41b5be14893beade7 Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Mon, 21 Oct 2019 19:47:56 +0900 Subject: [PATCH 32/47] rewrite "Alternatives" section --- designs/2019-move-to-async-api/README.md | 52 +++++++++++++++++++++++- 1 file changed, 50 insertions(+), 2 deletions(-) diff --git a/designs/2019-move-to-async-api/README.md b/designs/2019-move-to-async-api/README.md index 867aefbd..8b871722 100644 --- a/designs/2019-move-to-async-api/README.md +++ b/designs/2019-move-to-async-api/README.md @@ -442,8 +442,56 @@ The new API depends on [Asynchronous Iteration](https://github.com/tc39/proposal ## Alternatives -- Adding `engine.executeAsyncOnFiles()`-like methods and we maintain it along with the existing synchronous API. But as what I wrote in the "[Deprecate `CLIEngine` class](#deprecate-cliengine-class)" section, it would be tough. -- Using [Streams](https://nodejs.org/dist/latest-v12.x/docs/api/stream.html) instead of [Asynchronous Iteration](https://github.com/tc39/proposal-async-iteration). We can introduce `ESLint` class in a minor release if we used Streams. But because Node.js 8 will be EOL two months later, we should be able to use Asynchronous Iteration soon. And Streams don't support `for-await-of` syntax until `v11.14.0`. Iterator protocol is smaller spec than streams, and it's easy to use. +### Alternatives for the new class + +Adding `CLIEngine#executeOnFilesAsync()` method is an alternative. + +**Pros:** + +- It's smaller change than adding `ESLint` class. + +**Cons:** + +- We have to maintain both synchronous and asynchronous APIs. It's kind of hard works. + - We can deprecate the synchronous version, but in that case, I feel odd because `executeOnFiles()` is the essential name of `executeOnFilesAsync()`. +- We need the asynchronous version of the other APIs in the future. The `getFormatterAsync()` is needed for formatters with progress. The asynchronous version of the other methods is needed for ES modules. + - `CLIEngine` will get huge because we cannot share the most code between asynchronous API and synchronous API. + - API users need the number of API migrations. +- It causes confusion for API users. As Node.js built-in libraries are so, I guess that people expect the two (`executeOnFiles()` and `executeOnFilesAsync()`) to have exactly the same features. However, it's impossible. We have a bundle of the features that we cannot support on synchronous API, such as linting in parallel, formatters with progress, ES modules, etc. + +Therefore, I think that introducing the new class that has only asynchronous API makes more sense. This RFC solves: + +- We can freeze the code of the synchronous version of our API by deprecating `CLIEngine`. This reduces the maintenance cost of duplicate codes. +- We can reduce the number of API migrations for API users. +- We can reduce the confusion of similar but different methods. +- And as a bonus, we can reduce the confusion of the name of `CLIEngine`. + +### Alternatives for [Asynchronous Iteration](https://github.com/tc39/proposal-async-iteration) + +Using [streams](https://nodejs.org/dist/latest-v12.x/docs/api/stream.html) instead is an alternative. + +**Pros:** + +- We can introduce `ESLint` class in a minor release. (To use Async Iteration, we have to wait for [RFC44](https://github.com/eslint/rfcs/pull/44)). + +**Cons:** + +- Streams are problematic a bit about error propagation. + - [straem.pipeline()](https://nodejs.org/api/stream.html#stream_stream_pipeline_streams_callback) function reduces this pain, but [it doesn't cover all cases](https://github.com/nodejs/node/issues/26311). +- We have to wait for Node.js `v11.14.0` to use streams with `for-await-of` syntax. + +Because Node.js 8 will be EOL two months later, we should be able to use Asynchronous Iteration soon. And Iterator protocol is smaller spec than streams, and it's easy to use. + +### Alternatives for disallow execution in parallel + +- Throwing if the previous call has not finished yet (fail-fast). +- Aborting the previous call then run (steal ownership of the cache file). + +are alternatives. + +Both cases stop the previous or current call. It may be surprising users. + +The access of the cache file finishes regardless of the progress of the iterator that the method returned. It means that API users cannot know when the file access finished. On the other hand, because `ESLint` objects know when the file access finished, it can execute the next call at the proper timing. ## Related Discussions From c5bdf22d627b291e9eaa2602ff209cd7f68012ae Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Mon, 21 Oct 2019 21:25:59 +0900 Subject: [PATCH 33/47] revert about errorsOnly and... - remove `then()` support - add ESLint.collectResults() method --- designs/2019-move-to-async-api/README.md | 268 +++++++++++++---------- 1 file changed, 147 insertions(+), 121 deletions(-) diff --git a/designs/2019-move-to-async-api/README.md b/designs/2019-move-to-async-api/README.md index 8b871722..a732bbf6 100644 --- a/designs/2019-move-to-async-api/README.md +++ b/designs/2019-move-to-async-api/README.md @@ -25,9 +25,13 @@ This RFC adds a new class `ESLint`. It has almost the same methods as `CLIEngine - [executeOnText()](#-the-executeontext-method) - [getFormatter()](#-the-getformatter-method) - [static outputFixesInIteration()](#-the-outputfixesiniteration-method) (rename) +- [static extractErrorResults()](#-the-extracterrorresults-method) (rename) - [getConfigForFile()](#-the-other-methods) - [getRules()](#-the-other-methods) - [isPathIgnored()](#-the-other-methods) +- ~~addPlugin()~~ (delete) +- ~~resolveFileGlobPatterns()~~ (delete) +- [static collectResults()](#-new-methods) (new) - [static compareResultsByFilePath()](#-new-methods) (new) Initially the `ESLint` class will be a wrapper around `CLIEngine`, modifying return types. Later it can take on a more independent shape as `CLIEngine` gets more deprecated. @@ -38,11 +42,8 @@ The constructor has mostly the same options as `CLIEngine`, but with some differ - It throws fatal errors if the options contain unknown properties or an option is invalid type ([eslint/eslint#10272](https://github.com/eslint/eslint/issues/10272)). - It disallows the deprecated `cacheFile` option. -- It has a new `errorsOnly` option as the successor of `getErrorResults()` method. This option is a boolean value and defaults to `false`. If `true` is present then ESLint disables the rules which are configured with the warning severity. This option corresponds to [`--quiet` CLI option](https://eslint.org/docs/user-guide/command-line-interface#--quiet). See also "[The other methods](#-the-other-methods)" section. - It has a new `pluginImplementations` option as the successor of `addPlugin()` method. This is an object that keys are plugin IDs and each value is the plugin object. See also "[The other methods](#-the-other-methods)" section. -##### Implementation -
A rough sketch of the constructor. @@ -56,7 +57,6 @@ class ESLint { configFile = null, cwd = process.cwd(), envs = [], - errorsOnly = false, extensions = [".js"], fix = false, fixTypes = ["problem", "suggestion", "layout"], @@ -130,34 +130,58 @@ class ESLint { #### ● The `executeOnFiles()` method -This method returns an object that has two methods `then()` and `[Symbol.asyncIterator]()`, so we can use the returned object with `await` expression and `for-await-of` statement. +This method returns an object that implements [AsyncIterable] and [AsyncIterator], as similar to async generators. Therefore we can use the returned object with `for-await-of` statement. -- If you used the returned object with `for-await-of` statement, it iterates the lint result of each file in random order. This way yields each lint result immediately. Therefore you can print the results in streaming, or print progress state. ESLint may spend time to lint files (for example, ESLint needs about 20 seconds to lint [our codebase](https://github.com/eslint/eslint)), to print progress state will be useful. +```js +const { ESLint } = require("eslint") +const eslint = new ESLint() - ```js - const { ESLint } = require("eslint") - const eslint = new ESLint() +for await (const result of eslint.executeOnFiles(patterns)) { + print(result) +} +``` + +The returned object yields the lint result of each file immediately when it has finished linting each file. Therefore, ESLint doesn't guarantee the order of the iteration. The order is random. + +This method must not throw any errors synchronously. Errors may happen in iteration asynchronously. + +
+A rough sketch of the `executeOnFiles()` method. + +A tiny wrapper of `CLIEngine`. - for await (const result of eslint.executeOnFiles(patterns)) { - print(result) +```js +class ESLint { + async *executeOnFiles(patterns) { + yield* this._cliEngine.executeOnFiles(patterns).results } - ``` +} +``` -- If you used the returned object with `await` expression, it returns the array that contains all lint results in random order. This way waits for all lint results then returns it. +But once [RFC42] is implemented, the returned object will be an instance of [`LintResultGenerator`](https://github.com/eslint/eslint/blob/836c0e48704d70bc1a5cbdbf0211368b0ada942d/lib/eslint/lint-result-generator.js#L136) class. - ```js - const { ESLint } = require("eslint") - const eslint = new ESLint() +
- const results = await eslint.executeOnFiles(patterns) - // Optionally you can sort the results. - // results.sort(ESLint.compareResultsByFilePath) - print(results) - ``` +If you want to use the returned object with `await` expression, you can use a small utility to convert an async iterable object to an array. -Either way, we can support [linting in parallel](https://github.com/eslint/rfcs/pull/42), loading configs/plugins with `import()`, and other stuff that needs asynchronous logic. +```js +const { ESLint } = require("eslint") +const eslint = new ESLint() + +// Convert the results to an array. +const results = await ESLint.collectResults(eslint.executeOnFiles(patterns)) + +// Optionally you can sort the results. +results.sort(ESLint.compareResultsByFilePath) + +print(results) +``` -This method must not throw any errors synchronously. +Once we got this change, we can realize the following things: + +- [RFC42] We can implement linting in parallel by worker threads. It will reduce spending time of linting much. +- [RFC45] We can implement to print the results in streaming or to print progress state, without more breaking changes. Because ESLint may spend time to lint files (for example, ESLint needs about 20 seconds to lint [our codebase](https://github.com/eslint/eslint)), to print progress state will be useful. +- (no RFC yet) We can support ES modules for shareable configs, plugins, and custom parsers. ##### Iterate only one time @@ -200,13 +224,13 @@ Because this method updates the cache file, it will break the cache file if call ##### Abort linting -The iterator protocol has optional [`return()` method](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Generator/return) that forces to finish the iterator. The `for-of`/`for-await-of` syntax calls the `return()` method automatically if the loop is stopped through a `braek`, `return`, or `throw`. +The iterator interface has optional [`return()` method](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Generator/return) that forces to finish the iterator. The `for-of`/`for-await-of` syntax calls the `return()` method automatically if the loop is stopped through a `braek`, `return`, or `throw`. ESLint aborts linting when the `return()` method is called. The first `return()` method call does: - ESLint cancels the linting of all pending files. - ESLint updates the cache file with the current state. Therefore, the next time, ESLint can use the cache of the already linted files and lint only the canceled files. -- ESLint will terminate all workers if [RFC42](https://github.com/eslint/rfcs/pull/42) is implemented. +- ESLint will terminate all workers if [RFC42] is implemented. ```js const { ESLint } = require("eslint") @@ -221,61 +245,24 @@ for await (const result of eslint.executeOnFiles(patterns)) { The second and later calls do nothing. -##### Implementation - -
-A rough sketch of the `executeOnFiles()` method. - -A tiny wrapper of `CLIEngine`. - -```js -class ESLint { - executeOnFiles(patterns) { - let promise - try { - const report = this.cliEngine.executeOnFiles(patterns) - promise = Promise.resolve(report.results) - } catch (error) { - promise = Promise.reject(error) - } - - return { - then(onFulfilled, onRejected) { - return promise.then(onFulfilled, onRejected) - }, - async *[Symbol.asyncIterator]() { - yield* await promise - }, - } - } -} -``` - -But once [RFC42](https://github.com/eslint/rfcs/pull/42) is implemented, the returned object will be an instance of [`LintResultGenerator`](https://github.com/eslint/eslint/blob/836c0e48704d70bc1a5cbdbf0211368b0ada942d/lib/eslint/lint-result-generator.js#L136) class. The class implements async iterable protocol, async iterator protocol, and thenable. - -
- #### ● The `executeOnText()` method This method returns the same type of an object as the `executeOnFiles()` method. Because the returned object of `CLIEngine#executeOnText()` method is the same type as the `CLIEngine#executeOnFiles()` method. The `ESLint` class inherits that mannar. -
-Example: Show the result. - ```js const { ESLint } = require("eslint") const eslint = new ESLint() -const [result] = await eslint.executeOnText(text, filePath) +const [result] = await ESLint.collectResults( + eslint.executeOnText(text, filePath), +) + print(result) ``` -
- -
-Example: Using along with the `executeOnFiles()` method. +Example: Using along with the `executeOnFiles()` method. ```js const { ESLint } = require("eslint") @@ -290,15 +277,24 @@ for await (const result of report) { } ``` -
- #### ● The `getFormatter()` method -This method returns a `Promise`. The `Formatter` type is a function `(results: AsyncIterable) => AsyncIterableIterator`. It receives lint results then outputs the formatted text. +This method returns a `Promise`. The `Formatter` type is a function `(results: AsyncIterable) => AsyncIterable`. It receives lint results then outputs the formatted text. -This means the `getFormatter()` method wraps the current formatter to adapt the interface. +```js +const { ESLint } = require("eslint") +const eslint = new ESLint() +const formatter = eslint.getFormatter("stylish") + +// Verify files +const results = eslint.executeOnFiles(patterns) +// Format and write the results +for await (const textPiece of formatter(results)) { + process.stdout.write(textPiece) +} +``` -Once the `getFormatter()` method got this change, in the future, we can update the specification of custom formatters without breakage to support streaming. +This means the `getFormatter()` method wraps the current formatter to adapt the interface.
A rough sketch of the `getFormatter()` method. @@ -306,19 +302,16 @@ Once the `getFormatter()` method got this change, in the future, we can update t ```js class ESLint { async getFormatter(name) { - const format = this.cliEngine.getFormatter(name) + const format = this._cliEngine.getFormatter(name) // Return the wrapper. return async function* formatter(resultIterator) { // Collect and sort the results. - const results = [] - for await (const result of resultIterator) { - results.push(result) - } + const results = await ESLint.collectResults(resultIterator) results.sort(ESLint.compareResultsByFilePath) // Make `rulesMeta`. - const rules = this.cliEngine.getRules() + const rules = this._cliEngine.getRules() const rulesMeta = getRulesMeta(rules) // Format the results with the formatter of the current spec. @@ -330,8 +323,16 @@ class ESLint {
-
-Example: Use the formatter. +Once we got this change, we can realize the following things: + +- [RFC45] We can implement to print the results in streaming or to print progress state, without more breaking changes. +- (no RFC yet) We can support ES modules for custom formatters. + +#### ● The `outputFixesInIteration()` method + +The original `CLIEngine.outputFixes()` static method writes the fix results to the source code files. + +The goal of this method is same as the `CLIEngine.outputFixes()` method, but we cannot share async iterators with this method and formatters, so this method receives an `AsyncIterable` object as the first argument then return an `AsyncIterable` object. This method is sandwiched between `executeOnFiles()` and formatters. ```js const { ESLint } = require("eslint") @@ -339,23 +340,22 @@ const eslint = new ESLint() const formatter = eslint.getFormatter("stylish") // Verify files -const results = eslint.executeOnFiles(patterns) +let results = eslint.executeOnFiles(patterns) +// Update the files of the results if needed +if (process.argv.includes("--fix")) { + results = ESLint.outputFixesInIteration(results) +} // Format and write the results for await (const textPiece of formatter(results)) { process.stdout.write(textPiece) } ``` -
+#### ● The `extractErrorResults()` method -#### ● The `outputFixesInIteration()` method +The original `CLIEngine.getErrorResults()` static method receives an array of lint results, then extracts only the messages, which are error severity, then returns the results that contain only those. -The original `CLIEngine.outputFixes()` static method writes the fix results to the source code files. - -The goal of this method is same as the `CLIEngine.outputFixes()` method, but we cannot share async iterators with this method and formatters, so this method receives an `AsyncIterable` object as the first argument then return an `AsyncIterableIterator` object. This method is sandwiched between `executeOnFiles()` and formatters. - -
-Example: Use `outputFixesInIteration()`. +This method just changed the arrays to async iterables because this method is sandwiched between `executeOnFiles()` and formatters. ```js const { ESLint } = require("eslint") @@ -364,9 +364,9 @@ const formatter = eslint.getFormatter("stylish") // Verify files let results = eslint.executeOnFiles(patterns) -// Update the files of the results if needed -if (process.argv.includes("--fix")) { - results = ESLint.outputFixesInIteration(results) +// Extract only error results +if (process.argv.includes("--quiet")) { + results = ESLint.extractErrorResults(results) } // Format and write the results for await (const textPiece of formatter(results)) { @@ -374,11 +374,9 @@ for await (const textPiece of formatter(results)) { } ``` -
- #### ● The other methods -The following methods return `Promise` which gets fulfilled with each result in order to support plugins/configs that are ES modules without breakage in the future. +The following methods return `Promise` which gets fulfilled with each result. Once we got this change, we can support ES modules for shareable configs, plugins, and custom parsers without more breaking changes. - `getConfigForFile()` - `getRules()` @@ -386,38 +384,57 @@ The following methods return `Promise` which gets fulfilled with each result in The following methods are removed because those don't fit the new API. -- `addPlugin()` ... This method has caused to confuse people. We have introduced this method to add plugin implementations and expected people to use this method to test plugins, but people have thought that this method loads a new plugin for the following linting. And this method is only one that mutates the state of `CLIEngine` objects (except cache) and messes all caches. Therefore, this RFC moves this functionality to a constructor option. See also "[Constructor](#-constructor)" section. -- `getErrorResults()` ... This method is a utility to filter warning messages and is used for `--quiet` CLI option. But disabling the rules which are configured with the warning severity is more efficient than the current way. Therefore, this RFC moves this functionality to a constructor option. See also "[Constructor](#-constructor)" section. -- `resolveFileGlobPatterns()` ... ESLint doesn't use this logic since `v6.0.0`, but it has stayed there for backward compatibility. Once [RFC20](https://github.com/eslint/rfcs/tree/master/designs/2019-additional-lint-targets) is implemented, what ESLint iterates and what the glob of this method iterates will be different, then it will confuse users. This is good timing to remove the legacy. +- `addPlugin()` ... This method has caused to confuse people. We have introduced this method to add plugin implementations and expected people to use this method to test plugins. But people have often thought that this method loads a new plugin for the following linting so they can use plugins rules without `plugins` setting. And this method is only one that mutates the state of `CLIEngine` objects and messes all caches. Therefore, this RFC moves this functionality to a constructor option. See also "[Constructor](#-constructor)" section. +- `resolveFileGlobPatterns()` ... ESLint doesn't use this logic since `v6.0.0`, but it has stayed there for backward compatibility. Once [RFC20] is implemented, what ESLint iterates and what the glob of this method iterates will be different, then it will confuse users. This is good timing to remove the legacy. -#### ● A new `compareResultsByFilePath` static method +#### ● New methods -This method receives two lint results then returns `+1`, `-1`, or `0`. This method is intended to use in order to sort results. +- `collectResults()` ... This method receives an async iterable object then returns an array that contains the iterated values. -
-A rough sketch of the `compareResultsByFilePath()` method. +
+ A rough sketch of the `collectResults()` method. -```js -class ESLint { - static compareResultsByFilePath(a, b) { - if (a.filePath < b.filePath) { - return -1 + ```js + class ESLint { + static collectResults(resultIterator) { + const results = [] + for await (const result of resultIterator) { + results.push(result) + } + return results } - if (a.filePath > b.filePath) { - return 1 + } + ``` + +
+ +- `compareResultsByFilePath()` ... This method receives two lint results then returns `+1`, `-1`, or `0`. This method is intended to use in order to sort results. + +
+ A rough sketch of the `compareResultsByFilePath()` method. + + ```js + class ESLint { + static compareResultsByFilePath(a, b) { + if (a.filePath < b.filePath) { + return -1 + } + if (a.filePath > b.filePath) { + return 1 + } + return 0 } - return 0 } -} -``` + ``` -
+
### ■ Deprecate `CLIEngine` class -This RFC soft-deprecates `CLIEngine` class. +This RFC soft-deprecates `CLIEngine` class. Because: -Because it's tough to maintain two versions (sync and async) of implementation. The two are almost copy-pasted stuff, but hard to share the code. Therefore, this RFC deprecates the sync version to improve our code with the way which needs asynchronous behavior in the future. For example, `CLIEngine` cannot support [linting in parallel](https://github.com/eslint/rfcs/pull/42), plugins/configs as ES modules, etc... +- It's tough to maintain two versions (sync and async) of implementation. The two are almost copy-pasted stuff, but hard to share the code. We can freeze the synchronous version of code by deprecation. +- In the future, `CLIEngine` get not-supported features such as [RFC42], [RFC45], ES modules, etc because of synchronous API. This difference may be surprising for API users, but we can explain that as "Because `CLIEngine` has been deprecated, we don't add any new features into that class." ### ■ Out of scope @@ -434,11 +451,9 @@ People that use `CLIEngine` have to update their application with the new API. I ## Backwards Compatibility Analysis -This is a breaking change. - -Deprecating `CLIEngine` is a drastic change. But people can continue to use `CLIEngine` as-is until we decide to remove it. +If we assume [RFC44] will be merged, this RFC is a drastic change, but not a breaking change until we decide to remove `CLIEngine` class. -The new API depends on [Asynchronous Iteration](https://github.com/tc39/proposal-async-iteration) syntax. Node.js supports the syntax since `10.0.0`, so we have to drop Node.js `8.x`. Because the `8.x` will be EOL in December 2019 (two months later!), we can work on this soon. +This RFC just adds `ESLint` class and deprecates `CLIEngine` class. We can do both in a minor release. ## Alternatives @@ -472,7 +487,7 @@ Using [streams](https://nodejs.org/dist/latest-v12.x/docs/api/stream.html) inste **Pros:** -- We can introduce `ESLint` class in a minor release. (To use Async Iteration, we have to wait for [RFC44](https://github.com/eslint/rfcs/pull/44)). +- We can introduce `ESLint` class in a minor release. (To use Async Iteration, we have to wait for [RFC44]). **Cons:** @@ -480,7 +495,7 @@ Using [streams](https://nodejs.org/dist/latest-v12.x/docs/api/stream.html) inste - [straem.pipeline()](https://nodejs.org/api/stream.html#stream_stream_pipeline_streams_callback) function reduces this pain, but [it doesn't cover all cases](https://github.com/nodejs/node/issues/26311). - We have to wait for Node.js `v11.14.0` to use streams with `for-await-of` syntax. -Because Node.js 8 will be EOL two months later, we should be able to use Asynchronous Iteration soon. And Iterator protocol is smaller spec than streams, and it's easy to use. +Because Node.js 8 will be EOL two months later, we should be able to use Asynchronous Iteration soon. And Iterator interface is smaller spec than streams, and it's easy to use. ### Alternatives for disallow execution in parallel @@ -503,3 +518,14 @@ The access of the cache file finishes regardless of the progress of the iterator - https://github.com/eslint/rfcs/pull/4 - New: added draft for async/parallel proposal - https://github.com/eslint/rfcs/pull/11 - New: Lint files in parallel - https://github.com/eslint/rfcs/pull/42 - New: Lint files in parallel if many files exist +- https://github.com/eslint/rfcs/pull/44 - New: Drop supports for Node.js 8.x and 11.x +- https://github.com/eslint/rfcs/pull/45 - New: Formatter v2 + +[asynciterable]: https://tc39.es/ecma262/#sec-asynciterable-interface +[asynciterator]: https://tc39.es/ecma262/#sec-asynciterator-interface +[rfc04]: https://github.com/eslint/rfcs/pull/4 +[rfc11]: https://github.com/eslint/rfcs/pull/11 +[rfc20]: https://github.com/eslint/rfcs/tree/master/designs/2019-additional-lint-targets +[rfc42]: https://github.com/eslint/rfcs/pull/42 +[rfc44]: https://github.com/eslint/rfcs/pull/44 +[rfc45]: https://github.com/eslint/rfcs/pull/45 From 6ecf034f91d4096097ca492b0ba5f328218fb84e Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Mon, 21 Oct 2019 21:46:18 +0900 Subject: [PATCH 34/47] small fix --- designs/2019-move-to-async-api/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/designs/2019-move-to-async-api/README.md b/designs/2019-move-to-async-api/README.md index a732bbf6..e137dd24 100644 --- a/designs/2019-move-to-async-api/README.md +++ b/designs/2019-move-to-async-api/README.md @@ -29,7 +29,7 @@ This RFC adds a new class `ESLint`. It has almost the same methods as `CLIEngine - [getConfigForFile()](#-the-other-methods) - [getRules()](#-the-other-methods) - [isPathIgnored()](#-the-other-methods) -- ~~addPlugin()~~ (delete) +- ~~addPlugin()~~ (move to a constructor option) - ~~resolveFileGlobPatterns()~~ (delete) - [static collectResults()](#-new-methods) (new) - [static compareResultsByFilePath()](#-new-methods) (new) From 98243b8706e44762746b58ae5b38aadf68e6dc37 Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Thu, 14 Nov 2019 17:23:18 +0900 Subject: [PATCH 35/47] remove collectResults method --- designs/2019-move-to-async-api/README.md | 43 ++++++------------------ 1 file changed, 11 insertions(+), 32 deletions(-) diff --git a/designs/2019-move-to-async-api/README.md b/designs/2019-move-to-async-api/README.md index e137dd24..48fda684 100644 --- a/designs/2019-move-to-async-api/README.md +++ b/designs/2019-move-to-async-api/README.md @@ -31,7 +31,6 @@ This RFC adds a new class `ESLint`. It has almost the same methods as `CLIEngine - [isPathIgnored()](#-the-other-methods) - ~~addPlugin()~~ (move to a constructor option) - ~~resolveFileGlobPatterns()~~ (delete) -- [static collectResults()](#-new-methods) (new) - [static compareResultsByFilePath()](#-new-methods) (new) Initially the `ESLint` class will be a wrapper around `CLIEngine`, modifying return types. Later it can take on a more independent shape as `CLIEngine` gets more deprecated. @@ -165,11 +164,12 @@ But once [RFC42] is implemented, the returned object will be an instance of [`Li If you want to use the returned object with `await` expression, you can use a small utility to convert an async iterable object to an array. ```js +const toArray = require("@async-generators/to-array").default const { ESLint } = require("eslint") const eslint = new ESLint() // Convert the results to an array. -const results = await ESLint.collectResults(eslint.executeOnFiles(patterns)) +const results = await toArray(eslint.executeOnFiles(patterns)) // Optionally you can sort the results. results.sort(ESLint.compareResultsByFilePath) @@ -255,11 +255,9 @@ Because the returned object of `CLIEngine#executeOnText()` method is the same ty const { ESLint } = require("eslint") const eslint = new ESLint() -const [result] = await ESLint.collectResults( - eslint.executeOnText(text, filePath), -) - -print(result) +for await (const result of eslint.executeOnText(text, filePath)) { + print(result) +} ``` Example: Using along with the `executeOnFiles()` method. @@ -290,7 +288,7 @@ const formatter = eslint.getFormatter("stylish") const results = eslint.executeOnFiles(patterns) // Format and write the results for await (const textPiece of formatter(results)) { - process.stdout.write(textPiece) + process.stdout.write(textPiece) } ``` @@ -307,7 +305,7 @@ class ESLint { // Return the wrapper. return async function* formatter(resultIterator) { // Collect and sort the results. - const results = await ESLint.collectResults(resultIterator) + const results = await toArray(resultIterator) results.sort(ESLint.compareResultsByFilePath) // Make `rulesMeta`. @@ -343,11 +341,11 @@ const formatter = eslint.getFormatter("stylish") let results = eslint.executeOnFiles(patterns) // Update the files of the results if needed if (process.argv.includes("--fix")) { - results = ESLint.outputFixesInIteration(results) + results = ESLint.outputFixesInIteration(results) } // Format and write the results for await (const textPiece of formatter(results)) { - process.stdout.write(textPiece) + process.stdout.write(textPiece) } ``` @@ -366,11 +364,11 @@ const formatter = eslint.getFormatter("stylish") let results = eslint.executeOnFiles(patterns) // Extract only error results if (process.argv.includes("--quiet")) { - results = ESLint.extractErrorResults(results) + results = ESLint.extractErrorResults(results) } // Format and write the results for await (const textPiece of formatter(results)) { - process.stdout.write(textPiece) + process.stdout.write(textPiece) } ``` @@ -389,25 +387,6 @@ The following methods are removed because those don't fit the new API. #### ● New methods -- `collectResults()` ... This method receives an async iterable object then returns an array that contains the iterated values. - -
- A rough sketch of the `collectResults()` method. - - ```js - class ESLint { - static collectResults(resultIterator) { - const results = [] - for await (const result of resultIterator) { - results.push(result) - } - return results - } - } - ``` - -
- - `compareResultsByFilePath()` ... This method receives two lint results then returns `+1`, `-1`, or `0`. This method is intended to use in order to sort results.
From 13650ffa0daa7f4db44532bd0b976345d8279a57 Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Thu, 14 Nov 2019 17:31:14 +0900 Subject: [PATCH 36/47] remove getRules() --- designs/2019-move-to-async-api/README.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/designs/2019-move-to-async-api/README.md b/designs/2019-move-to-async-api/README.md index 48fda684..ca1a735e 100644 --- a/designs/2019-move-to-async-api/README.md +++ b/designs/2019-move-to-async-api/README.md @@ -27,9 +27,9 @@ This RFC adds a new class `ESLint`. It has almost the same methods as `CLIEngine - [static outputFixesInIteration()](#-the-outputfixesiniteration-method) (rename) - [static extractErrorResults()](#-the-extracterrorresults-method) (rename) - [getConfigForFile()](#-the-other-methods) -- [getRules()](#-the-other-methods) - [isPathIgnored()](#-the-other-methods) - ~~addPlugin()~~ (move to a constructor option) +- ~~getRules()~~ (delete) - ~~resolveFileGlobPatterns()~~ (delete) - [static compareResultsByFilePath()](#-new-methods) (new) @@ -377,12 +377,12 @@ for await (const textPiece of formatter(results)) { The following methods return `Promise` which gets fulfilled with each result. Once we got this change, we can support ES modules for shareable configs, plugins, and custom parsers without more breaking changes. - `getConfigForFile()` -- `getRules()` - `isPathIgnored()` The following methods are removed because those don't fit the new API. - `addPlugin()` ... This method has caused to confuse people. We have introduced this method to add plugin implementations and expected people to use this method to test plugins. But people have often thought that this method loads a new plugin for the following linting so they can use plugins rules without `plugins` setting. And this method is only one that mutates the state of `CLIEngine` objects and messes all caches. Therefore, this RFC moves this functionality to a constructor option. See also "[Constructor](#-constructor)" section. +- `getRules()` ... This method returns the map that contains core rules and the rules of the plugin that the previous `executeOnFiles()` method call used. This behavior is surprised and forces us to store the config objects that the previous `executeOnFiles()` method call used. This proposal removes this method and a separated RFC (maybe [RFC47]) will add the successor. - `resolveFileGlobPatterns()` ... ESLint doesn't use this logic since `v6.0.0`, but it has stayed there for backward compatibility. Once [RFC20] is implemented, what ESLint iterates and what the glob of this method iterates will be different, then it will confuse users. This is good timing to remove the legacy. #### ● New methods @@ -508,3 +508,4 @@ The access of the cache file finishes regardless of the progress of the iterator [rfc42]: https://github.com/eslint/rfcs/pull/42 [rfc44]: https://github.com/eslint/rfcs/pull/44 [rfc45]: https://github.com/eslint/rfcs/pull/45 +[rfc47]: https://github.com/eslint/rfcs/pull/47 From a860d00336a25410db26d1d51a1ee64fd6ba231c Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Thu, 14 Nov 2019 17:34:06 +0900 Subject: [PATCH 37/47] update the sentence about ES modules --- designs/2019-move-to-async-api/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/designs/2019-move-to-async-api/README.md b/designs/2019-move-to-async-api/README.md index ca1a735e..d59e2b0d 100644 --- a/designs/2019-move-to-async-api/README.md +++ b/designs/2019-move-to-async-api/README.md @@ -11,7 +11,7 @@ This RFC adds a new class `ESLint` that provides asynchronous API and deprecates ## Motivation - We have functionality that cannot be supported with the current synchronous API. For example, ESLint verifying files in parallel, formatters printing progress state, formatters printing results in streams etc. A move to an asynchronous API would be beneficial and a new `ESLint` class can be created with an async API in mind from the start. -- Node.js will support [ES modules](https://nodejs.org/api/esm.html) stably on `13.0.0` at last. Node.js doesn't provide any way that loads ES modules synchronously from CJS. This means that ESLint (CJS) cannot load configs/plugins that are written as ES modules synchronously. Migrating to asynchronous API opens up doors to support those. +- Node.js has supported [ES modules](https://nodejs.org/api/esm.html) stably since `13.2.0`. Because Node.js doesn't provide any way that loads ES modules synchronously from CJS, ESLint cannot load configs/plugins that are written as ES modules. And migrating to asynchronous API opens up doors to support those. - The name of `CLIEngine`, our primary API, has caused confusion in the community and is sub-optimal. We have a lot of issues that say "please use `CLIEngine` instead.". A new class, `ESLint`, while fixing other issues, will also make our primary API more clear. ## Detailed Design From eeac990c9bce2a914ccea589afc543e1c458ea5e Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Thu, 14 Nov 2019 17:40:00 +0900 Subject: [PATCH 38/47] rename for lintFiles() and lintText() --- designs/2019-move-to-async-api/README.md | 52 +++++++++++++----------- 1 file changed, 28 insertions(+), 24 deletions(-) diff --git a/designs/2019-move-to-async-api/README.md b/designs/2019-move-to-async-api/README.md index d59e2b0d..861851be 100644 --- a/designs/2019-move-to-async-api/README.md +++ b/designs/2019-move-to-async-api/README.md @@ -21,11 +21,11 @@ This RFC adds a new class `ESLint` that provides asynchronous API and deprecates This RFC adds a new class `ESLint`. It has almost the same methods as `CLIEngine`, but the return value of some methods are different. - [constructor()](#-constructor) -- [executeOnFiles()](#-the-executeonfiles-method) -- [executeOnText()](#-the-executeontext-method) +- [lintFiles()](#-the-lintfiles-method) (rename from `executeOnFiles()`) +- [lintText()](#-the-linttext-method) (rename from `executeOnText()`) - [getFormatter()](#-the-getformatter-method) -- [static outputFixesInIteration()](#-the-outputfixesiniteration-method) (rename) -- [static extractErrorResults()](#-the-extracterrorresults-method) (rename) +- [static outputFixesInIteration()](#-the-outputfixesiniteration-method) (rename from `outputFixes()`) +- [static extractErrorResults()](#-the-extracterrorresults-method) (rename from `getErrorResults()`) - [getConfigForFile()](#-the-other-methods) - [isPathIgnored()](#-the-other-methods) - ~~addPlugin()~~ (move to a constructor option) @@ -127,7 +127,9 @@ class ESLint {
-#### ● The `executeOnFiles()` method +#### ● The `lintFiles()` method + +This method corresponds to `CLIEngine#executeOnFiles()`. This method returns an object that implements [AsyncIterable] and [AsyncIterator], as similar to async generators. Therefore we can use the returned object with `for-await-of` statement. @@ -135,7 +137,7 @@ This method returns an object that implements [AsyncIterable] and [AsyncIterator const { ESLint } = require("eslint") const eslint = new ESLint() -for await (const result of eslint.executeOnFiles(patterns)) { +for await (const result of eslint.lintFiles(patterns)) { print(result) } ``` @@ -145,13 +147,13 @@ The returned object yields the lint result of each file immediately when it has This method must not throw any errors synchronously. Errors may happen in iteration asynchronously.
-A rough sketch of the `executeOnFiles()` method. +A rough sketch of the `lintFiles()` method. A tiny wrapper of `CLIEngine`. ```js class ESLint { - async *executeOnFiles(patterns) { + async *lintFiles(patterns) { yield* this._cliEngine.executeOnFiles(patterns).results } } @@ -169,7 +171,7 @@ const { ESLint } = require("eslint") const eslint = new ESLint() // Convert the results to an array. -const results = await toArray(eslint.executeOnFiles(patterns)) +const results = await toArray(eslint.lintFiles(patterns)) // Optionally you can sort the results. results.sort(ESLint.compareResultsByFilePath) @@ -191,7 +193,7 @@ We can iterate the returned object of this method only one time similar to gener const { ESLint } = require("eslint") const eslint = new ESLint() -const resultGenerator = eslint.executeOnFiles(patterns) +const resultGenerator = eslint.lintFiles(patterns) for await (const result of resultGenerator) { print(result) } @@ -211,7 +213,7 @@ But the location doesn't fit asynchronous because the used deprecated rule list const { ESLint } = require("eslint") const eslint = new ESLint() -for await (const result of eslint.executeOnFiles(patterns)) { +for await (const result of eslint.lintFiles(patterns)) { console.log(result.usedDeprecatedRules) } ``` @@ -220,7 +222,7 @@ As a side-effect, formatters gets the capability to print the used deprecated ru ##### Disallow execution in parallel -Because this method updates the cache file, it will break the cache file if called multiple times in parallel. To prevent that, every call of `executeOnFiles()` must wait for the previous call finishes. +Because this method updates the cache file, it will break the cache file if called multiple times in parallel. To prevent that, every call of `lintFiles()` must wait for the previous call finishes. ##### Abort linting @@ -236,7 +238,7 @@ ESLint aborts linting when the `return()` method is called. The first `return()` const { ESLint } = require("eslint") const eslint = new ESLint() -for await (const result of eslint.executeOnFiles(patterns)) { +for await (const result of eslint.lintFiles(patterns)) { if (Math.random() < 0.5) { break // abort linting. } @@ -245,9 +247,11 @@ for await (const result of eslint.executeOnFiles(patterns)) { The second and later calls do nothing. -#### ● The `executeOnText()` method +#### ● The `lintText()` method + +This method corresponds to `CLIEngine#executeOnText()`. -This method returns the same type of an object as the `executeOnFiles()` method. +This method returns the same type of an object as the `lintFiles()` method. Because the returned object of `CLIEngine#executeOnText()` method is the same type as the `CLIEngine#executeOnFiles()` method. The `ESLint` class inherits that mannar. @@ -255,20 +259,20 @@ Because the returned object of `CLIEngine#executeOnText()` method is the same ty const { ESLint } = require("eslint") const eslint = new ESLint() -for await (const result of eslint.executeOnText(text, filePath)) { +for await (const result of eslint.lintText(text, filePath)) { print(result) } ``` -Example: Using along with the `executeOnFiles()` method. +Example: Using along with the `lintFiles()` method. ```js const { ESLint } = require("eslint") const eslint = new ESLint() const report = useStdin - ? eslint.executeOnText(text, filePath) - : eslint.executeOnFiles(patterns) + ? eslint.lintText(text, filePath) + : eslint.lintFiles(patterns) for await (const result of report) { print(result) @@ -285,7 +289,7 @@ const eslint = new ESLint() const formatter = eslint.getFormatter("stylish") // Verify files -const results = eslint.executeOnFiles(patterns) +const results = eslint.lintFiles(patterns) // Format and write the results for await (const textPiece of formatter(results)) { process.stdout.write(textPiece) @@ -330,7 +334,7 @@ Once we got this change, we can realize the following things: The original `CLIEngine.outputFixes()` static method writes the fix results to the source code files. -The goal of this method is same as the `CLIEngine.outputFixes()` method, but we cannot share async iterators with this method and formatters, so this method receives an `AsyncIterable` object as the first argument then return an `AsyncIterable` object. This method is sandwiched between `executeOnFiles()` and formatters. +The goal of this method is same as the `CLIEngine.outputFixes()` method, but we cannot share async iterators with this method and formatters, so this method receives an `AsyncIterable` object as the first argument then return an `AsyncIterable` object. This method is sandwiched between `lintFiles()` and formatters. ```js const { ESLint } = require("eslint") @@ -338,7 +342,7 @@ const eslint = new ESLint() const formatter = eslint.getFormatter("stylish") // Verify files -let results = eslint.executeOnFiles(patterns) +let results = eslint.lintFiles(patterns) // Update the files of the results if needed if (process.argv.includes("--fix")) { results = ESLint.outputFixesInIteration(results) @@ -353,7 +357,7 @@ for await (const textPiece of formatter(results)) { The original `CLIEngine.getErrorResults()` static method receives an array of lint results, then extracts only the messages, which are error severity, then returns the results that contain only those. -This method just changed the arrays to async iterables because this method is sandwiched between `executeOnFiles()` and formatters. +This method just changed the arrays to async iterables because this method is sandwiched between `lintFiles()` and formatters. ```js const { ESLint } = require("eslint") @@ -361,7 +365,7 @@ const eslint = new ESLint() const formatter = eslint.getFormatter("stylish") // Verify files -let results = eslint.executeOnFiles(patterns) +let results = eslint.lintFiles(patterns) // Extract only error results if (process.argv.includes("--quiet")) { results = ESLint.extractErrorResults(results) From 6a36a141b4121500284ba9a079bdd3ee2f7f8506 Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Thu, 14 Nov 2019 18:00:16 +0900 Subject: [PATCH 39/47] replace pluginImplementations option --- designs/2019-move-to-async-api/README.md | 31 ++++++++++++++++++------ 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/designs/2019-move-to-async-api/README.md b/designs/2019-move-to-async-api/README.md index 861851be..8a1dd0c8 100644 --- a/designs/2019-move-to-async-api/README.md +++ b/designs/2019-move-to-async-api/README.md @@ -41,7 +41,7 @@ The constructor has mostly the same options as `CLIEngine`, but with some differ - It throws fatal errors if the options contain unknown properties or an option is invalid type ([eslint/eslint#10272](https://github.com/eslint/eslint/issues/10272)). - It disallows the deprecated `cacheFile` option. -- It has a new `pluginImplementations` option as the successor of `addPlugin()` method. This is an object that keys are plugin IDs and each value is the plugin object. See also "[The other methods](#-the-other-methods)" section. +- The array of the `plugins` option can contain objects `{ id: string; definition: Object }` along with strings. If the objects are present, the `id` property is the plugin ID and the `definition` property is the definition of the plugin. This is the successor of `addPlugin()` method. See also "[The other methods](#-the-other-methods)" section.
A rough sketch of the constructor. @@ -66,7 +66,6 @@ class ESLint { ignorePattern = [], parser = "espree", parserOptions = null, - pluginImplementations = null, plugins = [], reportUnusedDisableDirectives = false, resolvePluginsRelativeTo = cwd, @@ -107,7 +106,7 @@ class ESLint { ignorePattern, parser, parserOptions, - plugins, + plugins: plugins.map(p => (typeof p === "string" ? p : p.id)), reportUnusedDisableDirectives, resolvePluginsRelativeTo, rulePaths, @@ -115,10 +114,12 @@ class ESLint { useEslintrc, })) - // Apply `pluginImplementations` option. - if (pluginImplementations) { - for (const [id, definition] of Object.entries(pluginImplementations)) { - engine.addPlugin(id, definition) + // Add the definitions of the `plugins` option. + if (plugins) { + for (const plugin of plugins) { + if (typeof plugin === "object" && plugin !== null) { + engine.addPlugin(plugin.id, plugin.definition) + } } } } @@ -127,6 +128,22 @@ class ESLint {
+
+For `plugins` example. + +```js +const { ESLint } = require("eslint") +const eslint = new ESLint({ + plugins: [ + "foo", + "eslint-plugin-bar", + { id: "abc", definition: require("./path/to/a-plugin") }, + ], +}) +``` + +
+ #### ● The `lintFiles()` method This method corresponds to `CLIEngine#executeOnFiles()`. From ab978fa291a999b7f4f616ee1fe3f9d2a1626720 Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Thu, 14 Nov 2019 18:20:07 +0900 Subject: [PATCH 40/47] update about cache handling --- designs/2019-move-to-async-api/README.md | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/designs/2019-move-to-async-api/README.md b/designs/2019-move-to-async-api/README.md index 8a1dd0c8..cda7315e 100644 --- a/designs/2019-move-to-async-api/README.md +++ b/designs/2019-move-to-async-api/README.md @@ -237,9 +237,13 @@ for await (const result of eslint.lintFiles(patterns)) { As a side-effect, formatters gets the capability to print the used deprecated rules. Previously, ESLint has not passed the returned object to formatters, so the formatters could not print used deprecated rules. After this RFC, each lint result has the `usedDeprecatedRules` property and the formatters receive those. -##### Disallow execution in parallel +##### Write cache safely (best effort) -Because this method updates the cache file, it will break the cache file if called multiple times in parallel. To prevent that, every call of `lintFiles()` must wait for the previous call finishes. +Because this method updates the cache file, it will break the cache file if called multiple times in parallel. To prevent that, this method doesn't write the cache file if the cache file has been updated since this method read. + +This method does this check with the best effort (e.g., check `mtime` of the cache file) because Node.js doesn't provide the way that reads/writes a file exclusively. + +If the cache file was broken, this method should ignore the cache file and does lint. ##### Abort linting @@ -502,12 +506,14 @@ Because Node.js 8 will be EOL two months later, we should be able to use Asynchr - Throwing if the previous call has not finished yet (fail-fast). - Aborting the previous call then run (steal ownership of the cache file). -are alternatives. +are alternatives. Both cases stop the previous or current call. It may be surprising users. -Both cases stop the previous or current call. It may be surprising users. +- Waiting the previous call internally. The access of the cache file finishes regardless of the progress of the iterator that the method returned. It means that API users cannot know when the file access finished. On the other hand, because `ESLint` objects know when the file access finished, it can execute the next call at the proper timing. +However, the `ESLint` objects cannot know the existence of other threads and processes that write the cache file. The current way has a smaller risk than the alternatives. + ## Related Discussions - https://github.com/eslint/eslint/issues/1098 - Show results for each file as eslint is running From 920c881137c02b45ac00e25a512dd6bc8969a60e Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Fri, 22 Nov 2019 10:44:40 +0900 Subject: [PATCH 41/47] rename the new class --- designs/2019-move-to-async-api/README.md | 114 +++++++++++------------ 1 file changed, 57 insertions(+), 57 deletions(-) diff --git a/designs/2019-move-to-async-api/README.md b/designs/2019-move-to-async-api/README.md index cda7315e..2f4e1ff9 100644 --- a/designs/2019-move-to-async-api/README.md +++ b/designs/2019-move-to-async-api/README.md @@ -2,23 +2,23 @@ - RFC PR: https://github.com/eslint/rfcs/pull/40 - Authors: Toru Nagashima ([@mysticatea](https://github.com/mysticatea)) -# `ESLint` Class Replacing `CLIEngine` +# `LinterShell` Class Replacing `CLIEngine` ## Summary -This RFC adds a new class `ESLint` that provides asynchronous API and deprecates `CLIEngine`. +This RFC adds a new class `LinterShell` that provides asynchronous API and deprecates `CLIEngine`. ## Motivation -- We have functionality that cannot be supported with the current synchronous API. For example, ESLint verifying files in parallel, formatters printing progress state, formatters printing results in streams etc. A move to an asynchronous API would be beneficial and a new `ESLint` class can be created with an async API in mind from the start. +- We have functionality that cannot be supported with the current synchronous API. For example, ESLint verifying files in parallel, formatters printing progress state, formatters printing results in streams etc. A move to an asynchronous API would be beneficial and a new `LinterShell` class can be created with an async API in mind from the start. - Node.js has supported [ES modules](https://nodejs.org/api/esm.html) stably since `13.2.0`. Because Node.js doesn't provide any way that loads ES modules synchronously from CJS, ESLint cannot load configs/plugins that are written as ES modules. And migrating to asynchronous API opens up doors to support those. -- The name of `CLIEngine`, our primary API, has caused confusion in the community and is sub-optimal. We have a lot of issues that say "please use `CLIEngine` instead.". A new class, `ESLint`, while fixing other issues, will also make our primary API more clear. +- The name of `CLIEngine`, our primary API, has caused confusion in the community and is sub-optimal. We have a lot of issues that say "please use `CLIEngine` instead.". A new class, `LinterShell`, while fixing other issues, will also make our primary API more clear. ## Detailed Design -### Add new `ESLint` class +### Add new `LinterShell` class -This RFC adds a new class `ESLint`. It has almost the same methods as `CLIEngine`, but the return value of some methods are different. +This RFC adds a new class `LinterShell`. It has almost the same methods as `CLIEngine`, but the return value of some methods are different. - [constructor()](#-constructor) - [lintFiles()](#-the-lintfiles-method) (rename from `executeOnFiles()`) @@ -33,7 +33,7 @@ This RFC adds a new class `ESLint`. It has almost the same methods as `CLIEngine - ~~resolveFileGlobPatterns()~~ (delete) - [static compareResultsByFilePath()](#-new-methods) (new) -Initially the `ESLint` class will be a wrapper around `CLIEngine`, modifying return types. Later it can take on a more independent shape as `CLIEngine` gets more deprecated. +Initially the `LinterShell` class will be a wrapper around `CLIEngine`, modifying return types. Later it can take on a more independent shape as `CLIEngine` gets more deprecated. #### ● Constructor @@ -47,7 +47,7 @@ The constructor has mostly the same options as `CLIEngine`, but with some differ A rough sketch of the constructor. ```js -class ESLint { +class LinterShell { constructor({ allowInlineConfig = true, baseConfig = null, @@ -132,8 +132,8 @@ class ESLint { For `plugins` example. ```js -const { ESLint } = require("eslint") -const eslint = new ESLint({ +const { LinterShell } = require("eslint") +const linter = new LinterShell({ plugins: [ "foo", "eslint-plugin-bar", @@ -151,10 +151,10 @@ This method corresponds to `CLIEngine#executeOnFiles()`. This method returns an object that implements [AsyncIterable] and [AsyncIterator], as similar to async generators. Therefore we can use the returned object with `for-await-of` statement. ```js -const { ESLint } = require("eslint") -const eslint = new ESLint() +const { LinterShell } = require("eslint") +const linter = new LinterShell() -for await (const result of eslint.lintFiles(patterns)) { +for await (const result of linter.lintFiles(patterns)) { print(result) } ``` @@ -169,7 +169,7 @@ This method must not throw any errors synchronously. Errors may happen in iterat A tiny wrapper of `CLIEngine`. ```js -class ESLint { +class LinterShell { async *lintFiles(patterns) { yield* this._cliEngine.executeOnFiles(patterns).results } @@ -184,14 +184,14 @@ If you want to use the returned object with `await` expression, you can use a sm ```js const toArray = require("@async-generators/to-array").default -const { ESLint } = require("eslint") -const eslint = new ESLint() +const { LinterShell } = require("eslint") +const linter = new LinterShell() // Convert the results to an array. -const results = await toArray(eslint.lintFiles(patterns)) +const results = await toArray(linter.lintFiles(patterns)) // Optionally you can sort the results. -results.sort(ESLint.compareResultsByFilePath) +results.sort(LinterShell.compareResultsByFilePath) print(results) ``` @@ -207,10 +207,10 @@ Once we got this change, we can realize the following things: We can iterate the returned object of this method only one time similar to generators. ```js -const { ESLint } = require("eslint") -const eslint = new ESLint() +const { LinterShell } = require("eslint") +const linter = new LinterShell() -const resultGenerator = eslint.lintFiles(patterns) +const resultGenerator = linter.lintFiles(patterns) for await (const result of resultGenerator) { print(result) } @@ -227,10 +227,10 @@ The returned object of `CLIEngine#executeOnFiles()` has the `usedDeprecatedRules But the location doesn't fit asynchronous because the used deprecated rule list is not determined until the iteration finished. Therefore, this RFC moves the `usedDeprecatedRules` property to each lint result. ```js -const { ESLint } = require("eslint") -const eslint = new ESLint() +const { LinterShell } = require("eslint") +const linter = new LinterShell() -for await (const result of eslint.lintFiles(patterns)) { +for await (const result of linter.lintFiles(patterns)) { console.log(result.usedDeprecatedRules) } ``` @@ -256,10 +256,10 @@ ESLint aborts linting when the `return()` method is called. The first `return()` - ESLint will terminate all workers if [RFC42] is implemented. ```js -const { ESLint } = require("eslint") -const eslint = new ESLint() +const { LinterShell } = require("eslint") +const linter = new LinterShell() -for await (const result of eslint.lintFiles(patterns)) { +for await (const result of linter.lintFiles(patterns)) { if (Math.random() < 0.5) { break // abort linting. } @@ -274,13 +274,13 @@ This method corresponds to `CLIEngine#executeOnText()`. This method returns the same type of an object as the `lintFiles()` method. -Because the returned object of `CLIEngine#executeOnText()` method is the same type as the `CLIEngine#executeOnFiles()` method. The `ESLint` class inherits that mannar. +Because the returned object of `CLIEngine#executeOnText()` method is the same type as the `CLIEngine#executeOnFiles()` method. The `LinterShell` class inherits that mannar. ```js -const { ESLint } = require("eslint") -const eslint = new ESLint() +const { LinterShell } = require("eslint") +const linter = new LinterShell() -for await (const result of eslint.lintText(text, filePath)) { +for await (const result of linter.lintText(text, filePath)) { print(result) } ``` @@ -288,12 +288,12 @@ for await (const result of eslint.lintText(text, filePath)) { Example: Using along with the `lintFiles()` method. ```js -const { ESLint } = require("eslint") -const eslint = new ESLint() +const { LinterShell } = require("eslint") +const linter = new LinterShell() const report = useStdin - ? eslint.lintText(text, filePath) - : eslint.lintFiles(patterns) + ? linter.lintText(text, filePath) + : linter.lintFiles(patterns) for await (const result of report) { print(result) @@ -305,12 +305,12 @@ for await (const result of report) { This method returns a `Promise`. The `Formatter` type is a function `(results: AsyncIterable) => AsyncIterable`. It receives lint results then outputs the formatted text. ```js -const { ESLint } = require("eslint") -const eslint = new ESLint() -const formatter = eslint.getFormatter("stylish") +const { LinterShell } = require("eslint") +const linter = new LinterShell() +const formatter = linter.getFormatter("stylish") // Verify files -const results = eslint.lintFiles(patterns) +const results = linter.lintFiles(patterns) // Format and write the results for await (const textPiece of formatter(results)) { process.stdout.write(textPiece) @@ -323,7 +323,7 @@ This means the `getFormatter()` method wraps the current formatter to adapt the A rough sketch of the `getFormatter()` method. ```js -class ESLint { +class LinterShell { async getFormatter(name) { const format = this._cliEngine.getFormatter(name) @@ -331,7 +331,7 @@ class ESLint { return async function* formatter(resultIterator) { // Collect and sort the results. const results = await toArray(resultIterator) - results.sort(ESLint.compareResultsByFilePath) + results.sort(LinterShell.compareResultsByFilePath) // Make `rulesMeta`. const rules = this._cliEngine.getRules() @@ -358,15 +358,15 @@ The original `CLIEngine.outputFixes()` static method writes the fix results to t The goal of this method is same as the `CLIEngine.outputFixes()` method, but we cannot share async iterators with this method and formatters, so this method receives an `AsyncIterable` object as the first argument then return an `AsyncIterable` object. This method is sandwiched between `lintFiles()` and formatters. ```js -const { ESLint } = require("eslint") -const eslint = new ESLint() -const formatter = eslint.getFormatter("stylish") +const { LinterShell } = require("eslint") +const linter = new LinterShell() +const formatter = linter.getFormatter("stylish") // Verify files -let results = eslint.lintFiles(patterns) +let results = linter.lintFiles(patterns) // Update the files of the results if needed if (process.argv.includes("--fix")) { - results = ESLint.outputFixesInIteration(results) + results = LinterShell.outputFixesInIteration(results) } // Format and write the results for await (const textPiece of formatter(results)) { @@ -381,15 +381,15 @@ The original `CLIEngine.getErrorResults()` static method receives an array of li This method just changed the arrays to async iterables because this method is sandwiched between `lintFiles()` and formatters. ```js -const { ESLint } = require("eslint") -const eslint = new ESLint() -const formatter = eslint.getFormatter("stylish") +const { LinterShell } = require("eslint") +const linter = new LinterShell() +const formatter = linter.getFormatter("stylish") // Verify files -let results = eslint.lintFiles(patterns) +let results = linter.lintFiles(patterns) // Extract only error results if (process.argv.includes("--quiet")) { - results = ESLint.extractErrorResults(results) + results = LinterShell.extractErrorResults(results) } // Format and write the results for await (const textPiece of formatter(results)) { @@ -418,7 +418,7 @@ The following methods are removed because those don't fit the new API. A rough sketch of the `compareResultsByFilePath()` method. ```js - class ESLint { + class LinterShell { static compareResultsByFilePath(a, b) { if (a.filePath < b.filePath) { return -1 @@ -457,7 +457,7 @@ People that use `CLIEngine` have to update their application with the new API. I If we assume [RFC44] will be merged, this RFC is a drastic change, but not a breaking change until we decide to remove `CLIEngine` class. -This RFC just adds `ESLint` class and deprecates `CLIEngine` class. We can do both in a minor release. +This RFC just adds `LinterShell` class and deprecates `CLIEngine` class. We can do both in a minor release. ## Alternatives @@ -467,7 +467,7 @@ Adding `CLIEngine#executeOnFilesAsync()` method is an alternative. **Pros:** -- It's smaller change than adding `ESLint` class. +- It's smaller change than adding `LinterShell` class. **Cons:** @@ -491,7 +491,7 @@ Using [streams](https://nodejs.org/dist/latest-v12.x/docs/api/stream.html) inste **Pros:** -- We can introduce `ESLint` class in a minor release. (To use Async Iteration, we have to wait for [RFC44]). +- We can introduce `LinterShell` class in a minor release. (To use Async Iteration, we have to wait for [RFC44]). **Cons:** @@ -510,9 +510,9 @@ are alternatives. Both cases stop the previous or current call. It may be surpri - Waiting the previous call internally. -The access of the cache file finishes regardless of the progress of the iterator that the method returned. It means that API users cannot know when the file access finished. On the other hand, because `ESLint` objects know when the file access finished, it can execute the next call at the proper timing. +The access of the cache file finishes regardless of the progress of the iterator that the method returned. It means that API users cannot know when the file access finished. On the other hand, because `LinterShell` objects know when the file access finished, it can execute the next call at the proper timing. -However, the `ESLint` objects cannot know the existence of other threads and processes that write the cache file. The current way has a smaller risk than the alternatives. +However, the `LinterShell` objects cannot know the existence of other threads and processes that write the cache file. The current way has a smaller risk than the alternatives. ## Related Discussions From 8547265da4c4a3122fecdd58fe0796b2c2dbe032 Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Sun, 1 Dec 2019 04:09:12 +0900 Subject: [PATCH 42/47] remove about to print progress --- designs/2019-move-to-async-api/README.md | 255 +++++++++-------------- 1 file changed, 93 insertions(+), 162 deletions(-) diff --git a/designs/2019-move-to-async-api/README.md b/designs/2019-move-to-async-api/README.md index 2f4e1ff9..b92f6cdc 100644 --- a/designs/2019-move-to-async-api/README.md +++ b/designs/2019-move-to-async-api/README.md @@ -24,10 +24,10 @@ This RFC adds a new class `LinterShell`. It has almost the same methods as `CLIE - [lintFiles()](#-the-lintfiles-method) (rename from `executeOnFiles()`) - [lintText()](#-the-linttext-method) (rename from `executeOnText()`) - [getFormatter()](#-the-getformatter-method) -- [static outputFixesInIteration()](#-the-outputfixesiniteration-method) (rename from `outputFixes()`) -- [static extractErrorResults()](#-the-extracterrorresults-method) (rename from `getErrorResults()`) - [getConfigForFile()](#-the-other-methods) - [isPathIgnored()](#-the-other-methods) +- [static outputFixes()](#-the-other-methods) +- [static getErrorResults()](#-the-other-methods) - ~~addPlugin()~~ (move to a constructor option) - ~~getRules()~~ (delete) - ~~resolveFileGlobPatterns()~~ (delete) @@ -146,22 +146,32 @@ const linter = new LinterShell({ #### ● The `lintFiles()` method -This method corresponds to `CLIEngine#executeOnFiles()`. +##### Parameters + +| Name | Type | Description | +| :--------- | :--------- | :---------------------------------- | +| `patterns` | `string[]` | The glob patterns for target files. | + +##### Return Value -This method returns an object that implements [AsyncIterable] and [AsyncIterator], as similar to async generators. Therefore we can use the returned object with `for-await-of` statement. +This method returns a Promise object that will be fulfilled with the array of lint results. + +##### Description + +This method corresponds to `CLIEngine#executeOnFiles()`. ```js const { LinterShell } = require("eslint") const linter = new LinterShell() -for await (const result of linter.lintFiles(patterns)) { +for (const result of await linter.lintFiles(patterns)) { print(result) } ``` -The returned object yields the lint result of each file immediately when it has finished linting each file. Therefore, ESLint doesn't guarantee the order of the iteration. The order is random. +ESLint doesn't guarantee the order of the lint results in the array. If we implemented parallel linting, the result of the file that finished linting earlier will be in the front of the array. For backward compatibility, the wrapper of formatters sorts the results then gives the formatters the sorted results. See also [getFormatter()](#-the-getformatter-method). And we provide [compareResultsByFilePath()](#-new-methods) method to sort the results in the same order as `CLIEngine`. -This method must not throw any errors synchronously. Errors may happen in iteration asynchronously. +This method must not throw any errors synchronously. If an error happened, the returned promise goes rejected.
A rough sketch of the `lintFiles()` method. @@ -170,72 +180,35 @@ A tiny wrapper of `CLIEngine`. ```js class LinterShell { - async *lintFiles(patterns) { - yield* this._cliEngine.executeOnFiles(patterns).results + async lintFiles(patterns) { + return this._cliEngine.executeOnFiles(patterns).results } } ``` -But once [RFC42] is implemented, the returned object will be an instance of [`LintResultGenerator`](https://github.com/eslint/eslint/blob/836c0e48704d70bc1a5cbdbf0211368b0ada942d/lib/eslint/lint-result-generator.js#L136) class. -
-If you want to use the returned object with `await` expression, you can use a small utility to convert an async iterable object to an array. - -```js -const toArray = require("@async-generators/to-array").default -const { LinterShell } = require("eslint") -const linter = new LinterShell() - -// Convert the results to an array. -const results = await toArray(linter.lintFiles(patterns)) - -// Optionally you can sort the results. -results.sort(LinterShell.compareResultsByFilePath) - -print(results) -``` - Once we got this change, we can realize the following things: - [RFC42] We can implement linting in parallel by worker threads. It will reduce spending time of linting much. -- [RFC45] We can implement to print the results in streaming or to print progress state, without more breaking changes. Because ESLint may spend time to lint files (for example, ESLint needs about 20 seconds to lint [our codebase](https://github.com/eslint/eslint)), to print progress state will be useful. - (no RFC yet) We can support ES modules for shareable configs, plugins, and custom parsers. -##### Iterate only one time - -We can iterate the returned object of this method only one time similar to generators. - -```js -const { LinterShell } = require("eslint") -const linter = new LinterShell() - -const resultGenerator = linter.lintFiles(patterns) -for await (const result of resultGenerator) { - print(result) -} -// ↓ Throw "This generator has been consumed already" -for await (const result of resultGenerator) { - print(result) -} -``` - ##### Move the `usedDeprecatedRules` property The returned object of `CLIEngine#executeOnFiles()` has the `usedDeprecatedRules` property that includes the deprecated rule IDs which the linting used. -But the location doesn't fit asynchronous because the used deprecated rule list is not determined until the iteration finished. Therefore, this RFC moves the `usedDeprecatedRules` property to each lint result. +But the location is problematic because it requires the plugin uniqueness in spanning all files. Therefore, this RFC moves the `usedDeprecatedRules` property to each lint result. ```js const { LinterShell } = require("eslint") const linter = new LinterShell() -for await (const result of linter.lintFiles(patterns)) { +for (const result of await linter.lintFiles(patterns)) { console.log(result.usedDeprecatedRules) } ``` -As a side-effect, formatters gets the capability to print the used deprecated rules. Previously, ESLint has not passed the returned object to formatters, so the formatters could not print used deprecated rules. After this RFC, each lint result has the `usedDeprecatedRules` property and the formatters receive those. +As a side-effect, formatters gets the capability to print the used deprecated rules. Previously, ESLint has not passed the returned object to formatters, so the formatters could not print used deprecated rules. After this proposal, each lint result has the `usedDeprecatedRules` property and the formatters receive those. ##### Write cache safely (best effort) @@ -247,40 +220,34 @@ If the cache file was broken, this method should ignore the cache file and does ##### Abort linting -The iterator interface has optional [`return()` method](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Generator/return) that forces to finish the iterator. The `for-of`/`for-await-of` syntax calls the `return()` method automatically if the loop is stopped through a `braek`, `return`, or `throw`. +This proposal doesn't provide the way to abort linting because we cannot abort linting at this time (the method does linting synchronously internally). We can discuss it along with parallel linting (I'm guessing we can use [AbortSignal] that is the Web Standard. Passing it as `options.signal`). -ESLint aborts linting when the `return()` method is called. The first `return()` method call does: +#### ● The `lintText()` method -- ESLint cancels the linting of all pending files. -- ESLint updates the cache file with the current state. Therefore, the next time, ESLint can use the cache of the already linted files and lint only the canceled files. -- ESLint will terminate all workers if [RFC42] is implemented. +##### Parameters -```js -const { LinterShell } = require("eslint") -const linter = new LinterShell() +| Name | Type | Description | +| :-------------------- | :-------- | :--------------------------------------------------------------- | +| `code` | `string` | The source code to lint. | +| `options` | `Object` | Optional. The options. | +| `options.filePath` | `string` | Optional. The path to the file of the source code. | +| `options.warnIgnored` | `boolean` | Optional. If `true`, it warns the `filePath` is an ignored path. | -for await (const result of linter.lintFiles(patterns)) { - if (Math.random() < 0.5) { - break // abort linting. - } -} -``` +##### Return Value -The second and later calls do nothing. +This method returns a Promise object that will be fulfilled with the array of lint results. -#### ● The `lintText()` method +##### Description This method corresponds to `CLIEngine#executeOnText()`. -This method returns the same type of an object as the `lintFiles()` method. - -Because the returned object of `CLIEngine#executeOnText()` method is the same type as the `CLIEngine#executeOnFiles()` method. The `LinterShell` class inherits that mannar. +Because the returned object of `CLIEngine#executeOnText()` method is the same type as the `CLIEngine#executeOnFiles()` method, the `LinterShell` class inherits that mannar. Therefore, the returned value is an array that contains one result. ```js const { LinterShell } = require("eslint") const linter = new LinterShell() -for await (const result of linter.lintText(text, filePath)) { +for (const result of await linter.lintText(text, filePath)) { print(result) } ``` @@ -291,112 +258,83 @@ Example: Using along with the `lintFiles()` method. const { LinterShell } = require("eslint") const linter = new LinterShell() -const report = useStdin +const results = await (useStdin ? linter.lintText(text, filePath) - : linter.lintFiles(patterns) + : linter.lintFiles(patterns)) -for await (const result of report) { +for (const result of results) { print(result) } ``` #### ● The `getFormatter()` method -This method returns a `Promise`. The `Formatter` type is a function `(results: AsyncIterable) => AsyncIterable`. It receives lint results then outputs the formatted text. +##### Parameters + +| Name | Type | Description | +| :----- | :------- | :------------------------ | +| `name` | `string` | The formatter ID to load. | + +##### Return Value + +This method returns a Promise object that will be fulfilled with the wrapper of the loaded formatter. The wrapper is the object that has `format` method. + +| Name | Type | Description | +| :------- | :---------------------------------- | :------------------------------------------------- | +| `format` | `(results: LintResult[]) => string` | The function that converts lint results to output. | + +##### Description + +This method corresponds to `CLIEngine#getFormatter()`. + +But as different from that, it returns a wrapper object instead of the loaded formatter. Because we have experienced withdrawn some features because of breaking `getFormatter()` API in the past. The wrapper glues `getFormatter()` API and formatters. ```js const { LinterShell } = require("eslint") const linter = new LinterShell() -const formatter = linter.getFormatter("stylish") +const formatter = await linter.getFormatter("stylish") // Verify files -const results = linter.lintFiles(patterns) +const results = await linter.lintFiles(patterns) // Format and write the results -for await (const textPiece of formatter(results)) { - process.stdout.write(textPiece) -} +process.stdout.write(formatter.format(results)) ``` -This means the `getFormatter()` method wraps the current formatter to adapt the interface. +Currently, the wrapper does: -
-A rough sketch of the `getFormatter()` method. +1. sort given lint results. +1. create `rulesMeta`. ```js class LinterShell { async getFormatter(name) { const format = this._cliEngine.getFormatter(name) - // Return the wrapper. - return async function* formatter(resultIterator) { - // Collect and sort the results. - const results = await toArray(resultIterator) - results.sort(LinterShell.compareResultsByFilePath) - - // Make `rulesMeta`. - const rules = this._cliEngine.getRules() - const rulesMeta = getRulesMeta(rules) - - // Format the results with the formatter of the current spec. - yield format(results, { rulesMeta }) + return { + format(results) { + let rulesMeta = null + + results.sort(LinterShell.compareResultsByFilePath) + + return format(results, { + get rulesMeta() { + if (!rulesMeta) { + rulesMeta = createRulesMeta(this._cliEngine.getRules()) + } + return rulesMeta + }, + }) + }, } } } ``` -
- Once we got this change, we can realize the following things: -- [RFC45] We can implement to print the results in streaming or to print progress state, without more breaking changes. +- (no RFC yet) We can support to print progress state. - (no RFC yet) We can support ES modules for custom formatters. -#### ● The `outputFixesInIteration()` method - -The original `CLIEngine.outputFixes()` static method writes the fix results to the source code files. - -The goal of this method is same as the `CLIEngine.outputFixes()` method, but we cannot share async iterators with this method and formatters, so this method receives an `AsyncIterable` object as the first argument then return an `AsyncIterable` object. This method is sandwiched between `lintFiles()` and formatters. - -```js -const { LinterShell } = require("eslint") -const linter = new LinterShell() -const formatter = linter.getFormatter("stylish") - -// Verify files -let results = linter.lintFiles(patterns) -// Update the files of the results if needed -if (process.argv.includes("--fix")) { - results = LinterShell.outputFixesInIteration(results) -} -// Format and write the results -for await (const textPiece of formatter(results)) { - process.stdout.write(textPiece) -} -``` - -#### ● The `extractErrorResults()` method - -The original `CLIEngine.getErrorResults()` static method receives an array of lint results, then extracts only the messages, which are error severity, then returns the results that contain only those. - -This method just changed the arrays to async iterables because this method is sandwiched between `lintFiles()` and formatters. - -```js -const { LinterShell } = require("eslint") -const linter = new LinterShell() -const formatter = linter.getFormatter("stylish") - -// Verify files -let results = linter.lintFiles(patterns) -// Extract only error results -if (process.argv.includes("--quiet")) { - results = LinterShell.extractErrorResults(results) -} -// Format and write the results -for await (const textPiece of formatter(results)) { - process.stdout.write(textPiece) -} -``` - #### ● The other methods The following methods return `Promise` which gets fulfilled with each result. Once we got this change, we can support ES modules for shareable configs, plugins, and custom parsers without more breaking changes. @@ -404,6 +342,14 @@ The following methods return `Promise` which gets fulfilled with each result. On - `getConfigForFile()` - `isPathIgnored()` +The following methods return a `Promise`. Once we got this change, we can use asynchronous `fs` API to write files. + +- `static outputFixes()` + +The following methods are as-is. + +- `static getErrorResults()` + The following methods are removed because those don't fit the new API. - `addPlugin()` ... This method has caused to confuse people. We have introduced this method to add plugin implementations and expected people to use this method to test plugins. But people have often thought that this method loads a new plugin for the following linting so they can use plugins rules without `plugins` setting. And this method is only one that mutates the state of `CLIEngine` objects and messes all caches. Therefore, this RFC moves this functionality to a constructor option. See also "[Constructor](#-constructor)" section. @@ -412,7 +358,7 @@ The following methods are removed because those don't fit the new API. #### ● New methods -- `compareResultsByFilePath()` ... This method receives two lint results then returns `+1`, `-1`, or `0`. This method is intended to use in order to sort results. +- `static compareResultsByFilePath()` ... This method receives two lint results then returns `+1`, `-1`, or `0`. This method is intended to use in order to sort results.
A rough sketch of the `compareResultsByFilePath()` method. @@ -447,6 +393,7 @@ This RFC soft-deprecates `CLIEngine` class. Because: ## Documentation +- It needs an entry in the migration guide. - The "[Node.js API](https://eslint.org/docs/developer-guide/nodejs-api)" page should describe the new public API and deprecation of `CLIEngine` class. ## Drawbacks @@ -455,7 +402,7 @@ People that use `CLIEngine` have to update their application with the new API. I ## Backwards Compatibility Analysis -If we assume [RFC44] will be merged, this RFC is a drastic change, but not a breaking change until we decide to remove `CLIEngine` class. +This RFC is a drastic change, but not a breaking change until we decide to remove `CLIEngine` class. This RFC just adds `LinterShell` class and deprecates `CLIEngine` class. We can do both in a minor release. @@ -485,22 +432,6 @@ Therefore, I think that introducing the new class that has only asynchronous API - We can reduce the confusion of similar but different methods. - And as a bonus, we can reduce the confusion of the name of `CLIEngine`. -### Alternatives for [Asynchronous Iteration](https://github.com/tc39/proposal-async-iteration) - -Using [streams](https://nodejs.org/dist/latest-v12.x/docs/api/stream.html) instead is an alternative. - -**Pros:** - -- We can introduce `LinterShell` class in a minor release. (To use Async Iteration, we have to wait for [RFC44]). - -**Cons:** - -- Streams are problematic a bit about error propagation. - - [straem.pipeline()](https://nodejs.org/api/stream.html#stream_stream_pipeline_streams_callback) function reduces this pain, but [it doesn't cover all cases](https://github.com/nodejs/node/issues/26311). -- We have to wait for Node.js `v11.14.0` to use streams with `for-await-of` syntax. - -Because Node.js 8 will be EOL two months later, we should be able to use Asynchronous Iteration soon. And Iterator interface is smaller spec than streams, and it's easy to use. - ### Alternatives for disallow execution in parallel - Throwing if the previous call has not finished yet (fail-fast). @@ -527,8 +458,8 @@ However, the `LinterShell` objects cannot know the existence of other threads an - https://github.com/eslint/rfcs/pull/44 - New: Drop supports for Node.js 8.x and 11.x - https://github.com/eslint/rfcs/pull/45 - New: Formatter v2 -[asynciterable]: https://tc39.es/ecma262/#sec-asynciterable-interface -[asynciterator]: https://tc39.es/ecma262/#sec-asynciterator-interface +[abortcontroller]: https://dom.spec.whatwg.org/#interface-abortcontroller +[abortsignal]: https://dom.spec.whatwg.org/#interface-abortsignal [rfc04]: https://github.com/eslint/rfcs/pull/4 [rfc11]: https://github.com/eslint/rfcs/pull/11 [rfc20]: https://github.com/eslint/rfcs/tree/master/designs/2019-additional-lint-targets From c24f337c43689c93fd5ec01c40e5bee6941eff58 Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Fri, 6 Dec 2019 08:55:41 +0900 Subject: [PATCH 43/47] Update designs/2019-move-to-async-api/README.md Co-Authored-By: Kevin Partington --- designs/2019-move-to-async-api/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/designs/2019-move-to-async-api/README.md b/designs/2019-move-to-async-api/README.md index b92f6cdc..a8886223 100644 --- a/designs/2019-move-to-async-api/README.md +++ b/designs/2019-move-to-async-api/README.md @@ -241,7 +241,7 @@ This method returns a Promise object that will be fulfilled with the array of li This method corresponds to `CLIEngine#executeOnText()`. -Because the returned object of `CLIEngine#executeOnText()` method is the same type as the `CLIEngine#executeOnFiles()` method, the `LinterShell` class inherits that mannar. Therefore, the returned value is an array that contains one result. +Because the returned object of `CLIEngine#executeOnText()` method is the same type as the `CLIEngine#executeOnFiles()` method, the `LinterShell` class also returns the same type. Therefore, the returned value is an array that contains one result. ```js const { LinterShell } = require("eslint") From 9d4affc527effb44bc69559e150c2406299af6d5 Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Sun, 8 Dec 2019 12:58:14 +0900 Subject: [PATCH 44/47] Update designs/2019-move-to-async-api/README.md Co-Authored-By: Brandon Mills --- designs/2019-move-to-async-api/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/designs/2019-move-to-async-api/README.md b/designs/2019-move-to-async-api/README.md index a8886223..dd1703d8 100644 --- a/designs/2019-move-to-async-api/README.md +++ b/designs/2019-move-to-async-api/README.md @@ -10,7 +10,7 @@ This RFC adds a new class `LinterShell` that provides asynchronous API and depre ## Motivation -- We have functionality that cannot be supported with the current synchronous API. For example, ESLint verifying files in parallel, formatters printing progress state, formatters printing results in streams etc. A move to an asynchronous API would be beneficial and a new `LinterShell` class can be created with an async API in mind from the start. +- We have functionality that cannot be supported with the current synchronous API. For example, ESLint verifying files in parallel. A move to an asynchronous API would be beneficial and a new `LinterShell` class can be created with an async API in mind from the start. - Node.js has supported [ES modules](https://nodejs.org/api/esm.html) stably since `13.2.0`. Because Node.js doesn't provide any way that loads ES modules synchronously from CJS, ESLint cannot load configs/plugins that are written as ES modules. And migrating to asynchronous API opens up doors to support those. - The name of `CLIEngine`, our primary API, has caused confusion in the community and is sub-optimal. We have a lot of issues that say "please use `CLIEngine` instead.". A new class, `LinterShell`, while fixing other issues, will also make our primary API more clear. From 5032c3cadba23d3f1cf4e9edd3464add266bf2c8 Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Sun, 8 Dec 2019 12:58:27 +0900 Subject: [PATCH 45/47] Update designs/2019-move-to-async-api/README.md Co-Authored-By: Brandon Mills --- designs/2019-move-to-async-api/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/designs/2019-move-to-async-api/README.md b/designs/2019-move-to-async-api/README.md index dd1703d8..59e3e84a 100644 --- a/designs/2019-move-to-async-api/README.md +++ b/designs/2019-move-to-async-api/README.md @@ -384,7 +384,7 @@ The following methods are removed because those don't fit the new API. This RFC soft-deprecates `CLIEngine` class. Because: - It's tough to maintain two versions (sync and async) of implementation. The two are almost copy-pasted stuff, but hard to share the code. We can freeze the synchronous version of code by deprecation. -- In the future, `CLIEngine` get not-supported features such as [RFC42], [RFC45], ES modules, etc because of synchronous API. This difference may be surprising for API users, but we can explain that as "Because `CLIEngine` has been deprecated, we don't add any new features into that class." +- In the future, `CLIEngine` get not-supported features such as [RFC42], ES modules, etc because of synchronous API. This difference may be surprising for API users, but we can explain that as "Because `CLIEngine` has been deprecated, we don't add any new features into that class." ### ■ Out of scope From 671cf6afbc572f3ce3b24599669d3252daa76390 Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Sun, 8 Dec 2019 13:16:49 +0900 Subject: [PATCH 46/47] remove about printing progress --- designs/2019-move-to-async-api/README.md | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/designs/2019-move-to-async-api/README.md b/designs/2019-move-to-async-api/README.md index 59e3e84a..c86e21ac 100644 --- a/designs/2019-move-to-async-api/README.md +++ b/designs/2019-move-to-async-api/README.md @@ -332,7 +332,6 @@ class LinterShell { Once we got this change, we can realize the following things: -- (no RFC yet) We can support to print progress state. - (no RFC yet) We can support ES modules for custom formatters. #### ● The other methods @@ -441,9 +440,9 @@ are alternatives. Both cases stop the previous or current call. It may be surpri - Waiting the previous call internally. -The access of the cache file finishes regardless of the progress of the iterator that the method returned. It means that API users cannot know when the file access finished. On the other hand, because `LinterShell` objects know when the file access finished, it can execute the next call at the proper timing. +It will work fine in a thread. However, the `LinterShell` objects cannot know the existence of other threads and processes that write the cache file. -However, the `LinterShell` objects cannot know the existence of other threads and processes that write the cache file. The current way has a smaller risk than the alternatives. +The current way has a smaller risk than the alternatives. ## Related Discussions @@ -464,6 +463,4 @@ However, the `LinterShell` objects cannot know the existence of other threads an [rfc11]: https://github.com/eslint/rfcs/pull/11 [rfc20]: https://github.com/eslint/rfcs/tree/master/designs/2019-additional-lint-targets [rfc42]: https://github.com/eslint/rfcs/pull/42 -[rfc44]: https://github.com/eslint/rfcs/pull/44 -[rfc45]: https://github.com/eslint/rfcs/pull/45 [rfc47]: https://github.com/eslint/rfcs/pull/47 From ef4d198898ef41e1c4859cd09dd876e8a9192075 Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Sun, 8 Dec 2019 16:54:49 +0900 Subject: [PATCH 47/47] ESLint class again --- designs/2019-move-to-async-api/README.md | 78 +++++++++++++++--------- 1 file changed, 50 insertions(+), 28 deletions(-) diff --git a/designs/2019-move-to-async-api/README.md b/designs/2019-move-to-async-api/README.md index c86e21ac..3b2df6b9 100644 --- a/designs/2019-move-to-async-api/README.md +++ b/designs/2019-move-to-async-api/README.md @@ -2,23 +2,23 @@ - RFC PR: https://github.com/eslint/rfcs/pull/40 - Authors: Toru Nagashima ([@mysticatea](https://github.com/mysticatea)) -# `LinterShell` Class Replacing `CLIEngine` +# `ESLint` Class Replacing `CLIEngine` ## Summary -This RFC adds a new class `LinterShell` that provides asynchronous API and deprecates `CLIEngine`. +This RFC adds a new class `ESLint` that provides asynchronous API and deprecates `CLIEngine`. ## Motivation -- We have functionality that cannot be supported with the current synchronous API. For example, ESLint verifying files in parallel. A move to an asynchronous API would be beneficial and a new `LinterShell` class can be created with an async API in mind from the start. +- We have functionality that cannot be supported with the current synchronous API. For example, ESLint verifying files in parallel. A move to an asynchronous API would be beneficial and a new `ESLint` class can be created with an async API in mind from the start. - Node.js has supported [ES modules](https://nodejs.org/api/esm.html) stably since `13.2.0`. Because Node.js doesn't provide any way that loads ES modules synchronously from CJS, ESLint cannot load configs/plugins that are written as ES modules. And migrating to asynchronous API opens up doors to support those. -- The name of `CLIEngine`, our primary API, has caused confusion in the community and is sub-optimal. We have a lot of issues that say "please use `CLIEngine` instead.". A new class, `LinterShell`, while fixing other issues, will also make our primary API more clear. +- The name of `CLIEngine`, our primary API, has caused confusion in the community and is sub-optimal. We have a lot of issues that say "please use `CLIEngine` instead.". A new class, `ESLint`, while fixing other issues, will also make our primary API more clear. ## Detailed Design -### Add new `LinterShell` class +### Add new `ESLint` class -This RFC adds a new class `LinterShell`. It has almost the same methods as `CLIEngine`, but the return value of some methods are different. +This RFC adds a new class `ESLint`. It has almost the same methods as `CLIEngine`, but the return value of some methods are different. - [constructor()](#-constructor) - [lintFiles()](#-the-lintfiles-method) (rename from `executeOnFiles()`) @@ -33,7 +33,18 @@ This RFC adds a new class `LinterShell`. It has almost the same methods as `CLIE - ~~resolveFileGlobPatterns()~~ (delete) - [static compareResultsByFilePath()](#-new-methods) (new) -Initially the `LinterShell` class will be a wrapper around `CLIEngine`, modifying return types. Later it can take on a more independent shape as `CLIEngine` gets more deprecated. +Initially the `ESLint` class will be a wrapper around `CLIEngine`, modifying return types. Later it can take on a more independent shape as `CLIEngine` gets more deprecated. + +#### The class name `ESLint` + +This name means that it's the [facade](https://en.wikipedia.org/wiki/Facade_pattern) class of ESLint Node.js API. + +Currently, we have two classes `CLIEngine` and `Linter`. The `Linter` class is the most general name in our API, but it doesn't support some important parts that [our documentation](https://eslint.org/docs/user-guide/configuring) describes. I.e., it doesn't support config files and some configurations such as `extends`, `plugins`, `processor`, and `overrides`. Therefore, some users tried `Linter` class first, but they noticed that it doesn't work as expected. We have a ton of issues that we said: "use CLIEngine instead." + +This naming solves this problem. We can expect new API users are to try the `ESLint` class first without hesitation. + +On the other hand, there is a concern about this: "the `ESLint` class doesn't work on browsers. Users may try the `ESLint` class to use on browsers but notice it doesn't work." ([thread](https://github.com/eslint/rfcs/pull/40#discussion_r345384576)) +However, this concern is not a high priority because we don't support browsers officially. A more important matter is that the class which has the most general name supports features [our documentation](https://eslint.org/docs/user-guide/configuring) describes. #### ● Constructor @@ -47,7 +58,7 @@ The constructor has mostly the same options as `CLIEngine`, but with some differ A rough sketch of the constructor. ```js -class LinterShell { +class ESLint { constructor({ allowInlineConfig = true, baseConfig = null, @@ -132,8 +143,8 @@ class LinterShell { For `plugins` example. ```js -const { LinterShell } = require("eslint") -const linter = new LinterShell({ +const { ESLint } = require("eslint") +const linter = new ESLint({ plugins: [ "foo", "eslint-plugin-bar", @@ -161,8 +172,8 @@ This method returns a Promise object that will be fulfilled with the array of li This method corresponds to `CLIEngine#executeOnFiles()`. ```js -const { LinterShell } = require("eslint") -const linter = new LinterShell() +const { ESLint } = require("eslint") +const linter = new ESLint() for (const result of await linter.lintFiles(patterns)) { print(result) @@ -179,7 +190,7 @@ This method must not throw any errors synchronously. If an error happened, the r A tiny wrapper of `CLIEngine`. ```js -class LinterShell { +class ESLint { async lintFiles(patterns) { return this._cliEngine.executeOnFiles(patterns).results } @@ -200,8 +211,8 @@ The returned object of `CLIEngine#executeOnFiles()` has the `usedDeprecatedRules But the location is problematic because it requires the plugin uniqueness in spanning all files. Therefore, this RFC moves the `usedDeprecatedRules` property to each lint result. ```js -const { LinterShell } = require("eslint") -const linter = new LinterShell() +const { ESLint } = require("eslint") +const linter = new ESLint() for (const result of await linter.lintFiles(patterns)) { console.log(result.usedDeprecatedRules) @@ -241,11 +252,11 @@ This method returns a Promise object that will be fulfilled with the array of li This method corresponds to `CLIEngine#executeOnText()`. -Because the returned object of `CLIEngine#executeOnText()` method is the same type as the `CLIEngine#executeOnFiles()` method, the `LinterShell` class also returns the same type. Therefore, the returned value is an array that contains one result. +Because the returned object of `CLIEngine#executeOnText()` method is the same type as the `CLIEngine#executeOnFiles()` method, the `ESLint` class also returns the same type. Therefore, the returned value is an array that contains one result. ```js -const { LinterShell } = require("eslint") -const linter = new LinterShell() +const { ESLint } = require("eslint") +const linter = new ESLint() for (const result of await linter.lintText(text, filePath)) { print(result) @@ -255,8 +266,8 @@ for (const result of await linter.lintText(text, filePath)) { Example: Using along with the `lintFiles()` method. ```js -const { LinterShell } = require("eslint") -const linter = new LinterShell() +const { ESLint } = require("eslint") +const linter = new ESLint() const results = await (useStdin ? linter.lintText(text, filePath) @@ -290,8 +301,8 @@ This method corresponds to `CLIEngine#getFormatter()`. But as different from that, it returns a wrapper object instead of the loaded formatter. Because we have experienced withdrawn some features because of breaking `getFormatter()` API in the past. The wrapper glues `getFormatter()` API and formatters. ```js -const { LinterShell } = require("eslint") -const linter = new LinterShell() +const { ESLint } = require("eslint") +const linter = new ESLint() const formatter = await linter.getFormatter("stylish") // Verify files @@ -306,7 +317,7 @@ Currently, the wrapper does: 1. create `rulesMeta`. ```js -class LinterShell { +class ESLint { async getFormatter(name) { const format = this._cliEngine.getFormatter(name) @@ -314,7 +325,7 @@ class LinterShell { format(results) { let rulesMeta = null - results.sort(LinterShell.compareResultsByFilePath) + results.sort(ESLint.compareResultsByFilePath) return format(results, { get rulesMeta() { @@ -363,7 +374,7 @@ The following methods are removed because those don't fit the new API. A rough sketch of the `compareResultsByFilePath()` method. ```js - class LinterShell { + class ESLint { static compareResultsByFilePath(a, b) { if (a.filePath < b.filePath) { return -1 @@ -403,17 +414,28 @@ People that use `CLIEngine` have to update their application with the new API. I This RFC is a drastic change, but not a breaking change until we decide to remove `CLIEngine` class. -This RFC just adds `LinterShell` class and deprecates `CLIEngine` class. We can do both in a minor release. +This RFC just adds `ESLint` class and deprecates `CLIEngine` class. We can do both in a minor release. ## Alternatives +### Alternatives for the class name + +#### `LinterShell` + +- `CLIEngine` ⇒ `LinterShell` +- `Linter` ⇒ `LinterKernel` + +This name is inspired by shell and kernel. This name means that the `LinterShell` wraps `LinterKernel` and provides additional features. Most API users would use `LinterShell` rather than `LinterKernel`. + +However, this proposal focuses on adding a successor class of `CLIEngine`, so I'm not sure it's good if we rename `Linter` as well. + ### Alternatives for the new class Adding `CLIEngine#executeOnFilesAsync()` method is an alternative. **Pros:** -- It's smaller change than adding `LinterShell` class. +- It's smaller change than adding `ESLint` class. **Cons:** @@ -440,7 +462,7 @@ are alternatives. Both cases stop the previous or current call. It may be surpri - Waiting the previous call internally. -It will work fine in a thread. However, the `LinterShell` objects cannot know the existence of other threads and processes that write the cache file. +It will work fine in a thread. However, the `ESLint` objects cannot know the existence of other threads and processes that write the cache file. The current way has a smaller risk than the alternatives.