diff --git a/designs/2019-eslintrc-improvements/README.md b/designs/2019-eslintrc-improvements/README.md new file mode 100644 index 00000000..54210d46 --- /dev/null +++ b/designs/2019-eslintrc-improvements/README.md @@ -0,0 +1,356 @@ +- Start Date: 2019-02-25 +- RFC PR: https://github.com/eslint/rfcs/pull/13 +- Authors: Toru Nagashima (@mysticatea) + +# Config File Improvements + +## Summary + +This proposal improves our configuration files. This changes the architecture of configuration files to maintain our codebase easier and make enhancing easier. + +This RFC fixes two bugs I found while I make a PoC. I guess we don't want to make surprised behaviors on purpose. + +- ([link](#fix-error-in-unused-deps)) Even if unused dependencies have some errors, ESLint doesn't throw it. (fixes eslint/eslint#11396) +- ([link](#fix-overrides-order)) The configuration of overrides in shareable configs no longer overwrites user settings in .eslintrc files. (see details) + +This RFC includes four enhancements. + +- [Additional Lint Targets](major-01-additional-lint-targets.md) (major) +- [Array Config](minor-01-array-config.md) (minor) +- [`extends` in `overrides`](minor-02-extends-in-overrides.md) (minor) +- [Plugin Naming](minor-03-plugin-renaming.md) (minor) + +I made the enhancements in my PoC in order to confirm this architecture change is effective to maintain our codebase easier and make enhancing easier. Therefore, the enhancements are not required for this proposal. + +However, I'd like to add the enhancements with this because those will solve some important pain of the ecosystem. + +## Motivation + +- The codebase about configuration files is complicated. It has made us hard to maintain and enhance the configuration system. +- The current process determines target files at first. Next, it finds configuration files on the directory where each target file exists and that ancestors. This "files-then-config" order prevents adding some enhancements such as `--ext` functionality to our configuration file system. +- Several good ideas have been born in [#9] and [#14]. We can simplify the logic about configuration files by the ideas. + +Therefore, the goal of this RFC is to simplify our codebase by some architecture changes in order to maintain our codebase easier and make enhancing easier. + +Then some enhancements that the simplification gives would solve the important pains of the ecosystem. + +## Detailed Design + +(✨ in this section means the enhancement point for additional enhancement proposals.) + +> Proof of Concept (implementation): https://github.com/eslint/eslint/tree/proof-of-concept/config-array-in-eslintrc + +1. It simplifies the internal structure of configuration files by an array. +1. The config object owns loaded plugins and parsers rather than registers those to somewhere. +1. It changes the processing order to "config-then-files" from "files-then-config". +1. It restructures the files about CLIEngine and lookup. + +### 1. It simplifies the internal structure of configuration files by an array. + +✨ When it reads configuration files, it flattens the `extends` property and `overrides` property of the configuration. This is the following order: + +1. The loaded configurations of `extends` property. +1. The implicit configuration for file extension processors of `plugins` property. +1. The configuration except `extends` property and `overrides` property. +1. The configurations of `overrides` property. + +> [lib/lookup/config-array-factory.js#L635-L680](https://github.com/eslint/eslint/blob/153640180a8944af3a1c488462ed30d0c215f5ed/lib/_lookup/config-array-factory.js#L635-L680) in PoC. + +For some details: + +- If it cannot load a configuration of `extends` property, it throws an error immediately. +- If a loaded configuration of `extends` property has `extends` property or `overrides` property, it flattens those recursively. + +
+ ℹ️ User-facing change:
+ The configuration of overrides in shareable configs no longer overwrites user settings in .eslintrc files. (see details) +
+ +- If the configuration has `plugins` property and the plugins have file extension processors, it yields config array element for those to apply the file extension processors to matched files automatically. + + > [lib/lookup/config-array-factory.js#L890-L901](https://github.com/eslint/eslint/blob/153640180a8944af3a1c488462ed30d0c215f5ed/lib/_lookup/config-array-factory.js#L890-L901) in PoC. + +- ✨ If a configuration of `overrides` property has `extends` property or `overrides` property, it throws an error. + +- For duplicated settings, a later element in the array has precedence over an earlier element in the array. + +
+💡 Example: +
+{
+    "extends": ["eslint:recommended", "plugin:node/recommended"],
+    "rules": { ... },
+    "overrides": [
+        {
+            "files": ["*.ts"],
+            "rules": { ... },
+        }
+    ]
+}
+
+is flattend to: +
+[
+    // extends
+    {
+        "name": ".eslintrc.json » eslint:recommended",
+        "filePath": "node_modules/eslint/conf/eslint-recommended.js",
+        "rules": { ... }
+    },
+    {
+        "name": ".eslintrc.json » plugin:node/recommended",
+        "filePath": "node_modules/eslint-plugin-node/lib/index.js",
+        "env": { ... },
+        "parserOptions": { ... },
+        "plugins": { ... },
+        "rules": { ... }
+    },
+    // main
+    {
+        "name": ".eslintrc.json",
+        "filePath": ".eslintrc.json",
+        "rules": { ... }
+    },
+    // overrides
+    {
+        "name": ".eslintrc.json#overrides[0]",
+        "filePath": ".eslintrc.json",
+        "matchFile": { "includes": ["*.ts"], "excludes": null },
+        "rules": { ... }
+    }
+]
+
+
+ +### 2. The config object owns loaded plugins and parsers rather than registers those to somewhere. + +The loading logic of configuration files is complicated because it has complicated relationships between the config, plugins, parsers, and environments. + +![Current relationship graph](diagrams/current-deps.svg) + +The main reason is registration. The loading logic has side-effects that register loaded plugins to the plugin manager, and plugins have side-effects that register rules and environments to other managers. + +The codebase gets pretty simple by the removal of the registration. Instead, the internal structure of configuration owns loaded plugins and parsers ✨. + +![New relationship graph](diagrams/new-deps.svg) + +Surprisingly, now the return value of `ConfigArrayFactory.loadFile(filePath)` has all needed information to check files. Previously, we also needed information that was registered somewhere. + +
+💡 Example: +
+[
+    {
+        "name": ".eslintrc.json » plugin:@typescript-eslint/recommended",
+        "filePath": "node_modules/@typescript-eslint/eslint-plugin/dist/index.js",
+        // Config array element owns the loaded parser.
+        "parser": {
+            "definition": { ... }, // the parser implementation.
+            "id": "@typescript-eslint/parser",
+            "filePath": "node_modules/@typescript-eslint/parser/dist/index.js",
+            "importerPath": "node_modules/@typescript-eslint/eslint-plugin/dist/index.js"
+        },
+        "parserOptions": {
+            "sourceType": "module"
+        },
+        "plugins": {
+            // Config array element owns the loaded plugins.
+            "@typescript-eslint": {
+                "definition": { ... }, // the plugin implementation.
+                "id": "@typescript-eslint",
+                "filePath": "node_modules/@typescript-eslint/eslint-plugin/dist/index.js",
+                "importerPath": "node_modules/@typescript-eslint/eslint-plugin/dist/index.js"
+            }
+        },
+    }
+]
+
+
+ +If arbitrary errors happen while loading a plugin or a parser, the config array stores the error information rather than throws it. Because the plugin or the parser might not be used finally. +When `ConfigArray#extractConfig(filePath)` method extracted configuration for a file, if the final configuration contains the error, it throws the error. Here, "extract" means merge the elements in the config array as filtering it by `files` property and `excludedFiles` property. + +
+ℹ️ User-facing change:
+Even if unused dependencies have some errors, ESLint doesn't throw it. (fixes eslint/eslint#11396) +
+ +
+💡 Example: +
+[
+    {
+        "name": ".eslintrc.json » plugin:@typescript-eslint/recommended",
+        "filePath": "node_modules/@typescript-eslint/eslint-plugin/dist/index.js",
+        // Config array element owns the loaded plugins.
+        "parser": {
+            "error": Error, // an error object (maybe "Module Not Found").
+            "id": "@typescript-eslint/parser",
+            "importerPath": "node_modules/@typescript-eslint/eslint-plugin/dist/index.js"
+        },
+        "parserOptions": {
+            "sourceType": "module"
+        },
+        "plugins": {
+                // Config array element owns the loaded plugins.
+            "@typescript-eslint": {
+                "definition": { ... },
+                "id": "@typescript-eslint",
+                "filePath": "node_modules/@typescript-eslint/eslint-plugin/dist/index.js",
+                "importerPath": "node_modules/@typescript-eslint/eslint-plugin/dist/index.js"
+            }
+        },
+    }
+]
+
+
+ +If `Linter#verify` received a `ConfigArray` object, it requires `options.filename` as well. The `Linter` object calls `ConfigArray#extractConfig(filePath)` method and set needed parser, rules, and environments up. If the `options.filename` was `/path/to/.js`, it gives each rule only `` part. + +
+📝 Note:
+

Now we can implement #3 in Linter#verify method because the ConfigArray object has the complete information to handle virtual files. So we get two pros. +

    +
  • We don't need to access to the file system for each virtual file. +
  • We don't need to clone the logic of Linter#verifyAndFix method. +
+
+ +### 3. It changes the processing order to "config-then-files" from "files-then-config". + +Currently, first it finds target files by globs, next it finds configs for each target file. Therefore, we could not change target files by configuration files because of this ordering. + +This proposal changes that ordering. + +1. When the file enumerator entered into a directory, + 1. It finds `.eslintrc.*` file on the directory. + - If found, it concatenates the found configuration to the parent configuration (just `Array#concat`). + 1. It enumerates files on the directory. + - ✨ If a file is a regular file and matched to the current criteria, yields the pair of the file and the current configuration. + - If a file is a directory, enters into the directory (go step 1). + +> [lib/lookup/file-enumerator.js#L303-L360](https://github.com/eslint/eslint/blob/153640180a8944af3a1c488462ed30d0c215f5ed/lib/_lookup/file-enumerator.js#L303-L360) in PoC. + +
+📝 Note:
+As a side effect, the file enumerator reuses configuration instances naturally without a special cache logic. +
+ +As the result, we can change target files by settings of configuration files. + +### 4. It restructures the files about CLIEngine and lookup. + +It's premature optimization if we moved a logic only one functionality is using to the shared utility directory. The shared utility directory is similar to global variables, so it makes hard to know who use the utilities. + +This proposal moves some utility files to the directory of lookup functionality. + +We should be able to understand the lookup logic only the files in [lib/lookup](https://github.com/eslint/eslint/tree/proof-of-concept/config-array-in-eslintrc/lib/_lookup). + +## Documentation + +This core proposal needs migration guide because of a breaking change by a bug fix. + +- If people was affected by the `overrides` order change, they have to modify their config file. + +Additional enhancements need documents as well: + +- [Additional Lint Targets](major-01-additional-lint-targets.md#documentation) +- [Array Config](minor-01-array-config.md#documentation) +- [`extends` in `overrides`](minor-02-extends-in-overrides.md#documentation) +- [Plugin Naming](minor-03-plugin-renaming.md#documentation) + +## Drawbacks + +This is a large change. Currently we have multiple pull requests around configuration files, this will conflict with those pull requests severely. And this will be a large change for tests. Our tests really depend on internal structures seriously. + +## Backwards Compatibility Analysis + +Most cases work fine as is. + +### ✅ CLIEngine#addPlugin + +It can work fine as is. +`CLIEngine` has a `Map` for the added plugin, and it gives `FileEnumerator` (then `ConfigArrayFactory.load*` methods) the map, and the loading logic uses the map to load plugins. + +### ✅ Linter + +It can work fine as is. + +Only if a given `config` has `extractConfig` method, `Linter` does the setup process for the config. Otherwise, it works the same as currently. + +See [this paragraph](#linter-change) also. + +### ✅ LintResultCache + +It can work fine as is. + +The `ConfigArray` has all information to distinguish if the config was changed since the last time. + +### ⚠️ Fix a surprised behavior of `overrides`. + +Currently, `overrides` is applied after all `overrides` properties are merged. This means that the `overrides` in a shareable config priors to the setting in your `.eslintrc`. + +```yml +extends: + - foo +rules: + eqeqeq: error # silently ignored if the "foo" has the "eqeqeq" setting in "overrides". +``` + +After this proposal, the setting in your `.eslintrc` priors to the setting in shareable configs always. +This is a breaking change, but I think this is a bug fix. + +### ⚠️ About additional enhancements + +An additional enhancement have a breaking change. + +- [Additional Lint Targets](major-01-additional-lint-targets.md#backwards-compatibility-analysis) + +## Alternatives + +- [#9] is the alternative. But double duplicate features cause confusion for the ecosystem. For newcomers, a mix of articles about two config systems makes hard to understand ESLint. For non-English users, the official document is far. + +## Open Questions + +- + +## Frequently Asked Questions + +- + +## Related Discussions + +- [#14] +- [#9] +- [#7] +- [#5] +- [#3] +- [eslint/eslint#3458] +- [eslint/eslint#6732] +- [eslint/eslint#8813] +- [eslint/eslint#9505] +- [eslint/eslint#9897] +- [eslint/eslint#10125] +- [eslint/eslint#10643] +- [eslint/eslint#10891] +- [eslint/eslint#11223] +- [eslint/eslint#11396] + +Especially, this proposal is inspired by the discussion on [#9] and [#14]. + +[#14]: https://github.com/eslint/rfcs/pull/14 +[#9]: https://github.com/eslint/rfcs/pull/9 +[#7]: https://github.com/eslint/rfcs/pull/7 +[#5]: https://github.com/eslint/rfcs/pull/5 +[#3]: https://github.com/eslint/rfcs/pull/3 +[eslint/eslint#3458]: https://github.com/eslint/eslint/issues/3458 +[eslint/eslint#6732]: https://github.com/eslint/eslint/issues/6732 +[eslint/eslint#8813]: https://github.com/eslint/eslint/issues/8813 +[eslint/eslint#9505]: https://github.com/eslint/eslint/issues/9505 +[eslint/eslint#9897]: https://github.com/eslint/eslint/issues/9897 +[eslint/eslint#10125]: https://github.com/eslint/eslint/issues/10125 +[eslint/eslint#10643]: https://github.com/eslint/eslint/issues/10643 +[eslint/eslint#10891]: https://github.com/eslint/eslint/issues/10891 +[eslint/eslint#11223]: https://github.com/eslint/eslint/issues/11223 +[eslint/eslint#11396]: https://github.com/eslint/eslint/issues/11396 +[Robustness Guarantee]: https://gist.github.com/not-an-aardvark/169bede8072c31a500e018ed7d6a8915 diff --git a/designs/2019-eslintrc-improvements/diagrams.plantuml b/designs/2019-eslintrc-improvements/diagrams.plantuml new file mode 100644 index 00000000..94d06678 --- /dev/null +++ b/designs/2019-eslintrc-improvements/diagrams.plantuml @@ -0,0 +1,66 @@ +@startuml current-deps +rectangle "depending on fs" { + object CLIEngine + object Config + object Plugins + object GlobUtils +} +object Linter +object ConfigCache +object Environments +object Rules +object "config object" as ConfigData +object "parser object" as Parser +object "plugin object" as Plugin +object "env object" as Environment +object "rule object" as Rule + +CLIEngine *-->"1" Linter +CLIEngine *-->"1" Config +CLIEngine *-->"1" GlobUtils +Config -->"1" Linter +Config *-->"1" ConfigCache : "register" +Config *-->"1" Plugins : "use" +Config ..> ConfigData : "load" +Config ..> Environments : "use" +Plugins -->"1" Environments : "register" +Plugins -->"1" Linter +Plugins ..> Rules : "register" +Plugins *-->"0..*" Plugin : "load" +Linter *-->"*" Parser : "load" +Linter *-->"1" Rules +Linter *-->"1" Environments +Linter ..> ConfigData : "use" +Environments -->"0..*" Environment +Rules -->"0..*" Rule +Plugin *-->"0..*" Rule +Plugin *-->"0..*" Environment +@enduml + +@startuml new-deps +rectangle "depending on fs" { + object CLIEngine + object FileEnumerator + object ConfigArrayFactory +} +object Linter +object ConfigArray +object ConfigArrayElement +object "parser object" as Parser +object "plugin object" as Plugin +object "env object" as Environment +object "rule object" as Rule + +CLIEngine *-->"1" Linter +CLIEngine *-->"1" FileEnumerator +Linter ..> ConfigArray +FileEnumerator *-->"1" ConfigArrayFactory +ConfigArrayFactory ..> ConfigArray : "load" +ConfigArrayFactory ..> Parser : "load" +ConfigArrayFactory ..> Plugin : "load" +ConfigArray *-->"0..*" ConfigArrayElement +ConfigArrayElement o-->"0..1" Parser +ConfigArrayElement o-->"0..*" Plugin +Plugin *-->"0..*" Rule +Plugin *-->"0..*" Environment +@enduml diff --git a/designs/2019-eslintrc-improvements/diagrams/current-deps.svg b/designs/2019-eslintrc-improvements/diagrams/current-deps.svg new file mode 100644 index 00000000..c4ad5779 --- /dev/null +++ b/designs/2019-eslintrc-improvements/diagrams/current-deps.svg @@ -0,0 +1,51 @@ +depending on fsCLIEngineConfigPluginsGlobUtilsLinterConfigCacheEnvironmentsRulesconfig objectparser objectplugin objectenv objectrule object1111register1use1loaduseregister11registerload0..*load*11use0..*0..*0..*0..* \ No newline at end of file diff --git a/designs/2019-eslintrc-improvements/diagrams/new-deps.svg b/designs/2019-eslintrc-improvements/diagrams/new-deps.svg new file mode 100644 index 00000000..e989a348 --- /dev/null +++ b/designs/2019-eslintrc-improvements/diagrams/new-deps.svg @@ -0,0 +1,40 @@ +depending on fsCLIEngineFileEnumeratorConfigArrayFactoryLinterConfigArrayConfigArrayElementparser objectplugin objectenv objectrule object111loadloadload0..*0..10..*0..*0..* \ No newline at end of file diff --git a/designs/2019-eslintrc-improvements/major-01-additional-lint-targets.md b/designs/2019-eslintrc-improvements/major-01-additional-lint-targets.md new file mode 100644 index 00000000..23f4f05a --- /dev/null +++ b/designs/2019-eslintrc-improvements/major-01-additional-lint-targets.md @@ -0,0 +1,105 @@ +# Config File Improvements: Additional Lint Targets + +## Summary + +This proposal adds the ability to specify additional target files in configuration files. This enhancement will solve the pain that people have to use `--ext` option with wanted file extensions even if they use plugins. + +## Motivation + +People have to use `--ext` option or glob patterns to check wanted files even if they use plugins to support additional file types. + +```yml +plugins: + - markdown + - html + - "@typescript-eslint" + - react + - vue +``` + +```bash +# ESLint checks only `*.js`. +eslint src docs + +# Needs `--ext` option +eslint src docs --ext .js,.md,.html,.ts,.jsx,.tsx,.vue +``` + +## Detailed Design + +> Proof of Concept (implementation): https://github.com/eslint/eslint/tree/proof-of-concept/config-array-in-eslintrc + +This proposal enhances [there](README.md#additional-lint-targets). + +When `FileEnumerator` checks if a file is matched by the given extensions, additionally, it checks the file is matched by any of `files` property of configs (except `files` patterns which end with `*` to avoid unlimited). + +> [lib/lookup/file-enumerator.js#L338](https://github.com/eslint/eslint/blob/153640180a8944af3a1c488462ed30d0c215f5ed/lib/_lookup/file-enumerator.js#L338) in PoC. + +
+💡 Example: +
+overrides:
+  - files: "*.ts"
+    extends: "plugin:@typescript-eslint/recommended"
+
+ +With the above config, `eslint .` command will check `*.ts` files additionally. +
+ +If a plugin has file extension processors, [it yields config array elements which have `overrides` property](README.md#yield-file-extension-processor-element), so this enhancement affects file extension processors. + +
+💡 Example: +
+plugins:
+  - vue # has `.vue` processor
+
+ +With the above config, `eslint .` command will check `*.vue` files additionally. +
+ +The ignoring configuration (`.eslintignore`) is prior to this enhancement. If `.eslintignore` contains the additional target files, ESLint just ignores those as same as currently. + +## Documentation + +This enhancement needs migration guide because of a breaking change. + +- If your config contains `overrides` property or plugins which have file extension processors, `eslint` command with directory paths lints the files which are matched automatically. This may increase errors of your command.
+ If you don't want to add file types to check, please use glob patterns instead of directory paths. + +This enhancement, so it needs to update some documents. + +- In the description of `--ext` CLI option, it should say that your config file may add file types automatically. +- In the description of `overrides` property, it should say that the `overrides[].files` property adds target files automatically. +- In the description of `plugins` property, it should say that plugins which have file extension processors add target files automatically. +- In the "working with plugins" page, it should say that file extension processors add target files automatically. + +## Drawbacks + +- This is a breaking change. +- Implicit behavior may make people confused. + +## Backwards Compatibility Analysis + +In the following situation, `eslint` command increases errors. + +- Their configuration has `overrides` property (with `files` property which doesn't end with `*`) or plugins which have file extension processors. +- Their `eslint` command is using directory paths without `--ext` option. + +I guess that the impact is limited because people use `--ext` option or glob patterns if they use configuration which affects files other than `*.js`. + +## Alternatives + +- To add `extensions` property that behaves as same as `--est` option. Explicit reduces confusion. However, plugins and shareable configs cannot add the `extensions` property without the breaking change that drops old ESLint support. + +## Open Questions + +- + +## Frequently Asked Questions + +- + +## Related Discussions + +- [eslint/eslint#11223](https://github.com/eslint/eslint/issues/11223) diff --git a/designs/2019-eslintrc-improvements/minor-01-array-config.md b/designs/2019-eslintrc-improvements/minor-01-array-config.md new file mode 100644 index 00000000..6dcf9380 --- /dev/null +++ b/designs/2019-eslintrc-improvements/minor-01-array-config.md @@ -0,0 +1,99 @@ +# Config File Improvements: Array Config + +## Summary + +This proposal makes `.eslintrc` files allowing an array as top-level config. This is a syntax sugar of `extends` property and `overrides` property to make configuration simpler. Also, this form is closer to ESLint internal structure about configuration. + +## Motivation + +The current config file has both parent config (`extends`) and child configs (`overrides`) as the same way. The configuration body overwrites the parent config and is overwritten by the child configs. + +The array notation makes this relationship simpler: "a later element in the array has precedence over an earlier element in the array." + +Also, this is useful to [use two shareable configs which have the same plugin ID with renaming](minor-03-plugin-renaming.md#using-two-shareable-configs-which-have-the-same-plugin-id-with-renaming). + +## Detailed Design + +> Proof of Concept (implementation): https://github.com/eslint/eslint/tree/proof-of-concept/config-array-in-eslintrc + +This proposal enhances [there](README.md#array-config). + +Currently, config files don't allow an array at top-level. + +This RFC adds the support of an array. Each array element can be: + +1. a string that `extends` property is supporting. +1. an object that is same as the current config files. +1. an object that is same as the elements of `overrides` property. +1. an array that is same as top-level's. (nested) + +`ConfigArrayFactory` loads strings as `extends` and normalizes others recursively. + +> [lib/lookup/config-array-factory.js#L653-L671](https://github.com/eslint/eslint/blob/c5c22cad7c27113840ac8005f18e6fcee657acf9/lib/_lookup/config-array-factory.js#L653-L671) + +
+💡 Example: +
+module.exports = {
+    extends: [
+        "eslint:recommended",
+        "plugin:node/recommended"
+    ],
+    rules: {
+        eqeqeq: "error"
+    },
+    overrides: [
+        {
+            files: "*.ts",
+            extends: "plugin:@typescript-eslint/recommended"
+        }
+    ]
+}
+
+is rewritable to: +
+module.exports = [
+    "eslint:recommended",
+    "plugin:node/recommended",
+    {
+        rules: {
+            eqeqeq: "error"
+        }
+    },
+    {
+        files: "*.ts",
+        extends: "plugin:@typescript-eslint/recommended"
+    }
+]
+
+
+ +## Documentation + +This should be described in the "Configuring ESLint" page. + +## Drawbacks + +This doesn't increase the capability of ESLint. + +## Backwards Compatibility Analysis + +If people depend on the behavior that ESLint throws an error if they give an array as configuration, this enhancement breaks that. However, I believe that we don't need to worry. + +## Alternatives + +- + +## Open Questions + +- + +## Frequently Asked Questions + +- + +## Related Discussions + +- [#9] + +[#9]: https://github.com/eslint/rfcs/pull/9 diff --git a/designs/2019-eslintrc-improvements/minor-02-extends-in-overrides.md b/designs/2019-eslintrc-improvements/minor-02-extends-in-overrides.md new file mode 100644 index 00000000..73e2d28a --- /dev/null +++ b/designs/2019-eslintrc-improvements/minor-02-extends-in-overrides.md @@ -0,0 +1,105 @@ +# Config File Improvements: `extends` in `overrides` + +## Summary + +This proposal makes the configuration in `overrides` property supporting `extends` property and `overrides` property (fixes [eslint/eslint#8813]). + +## Motivation + +Currently, cascading configuration supports `extends`, but globa-based configuration doesn't. This lacking has been preventing people to migrate their configuration to glob-based. + +To support `extends` property (and nested `overrides` property for completeness) in `overrides` solve the pain. + +## Detailed Design + +> Proof of Concept (implementation): https://github.com/eslint/eslint/tree/proof-of-concept/config-array-in-eslintrc + +This proposal enhances [there](README.md#extends-in-overrides). + +If a configuration in `overrides` property has `extends` property or `overrides` property, it flattens those recursively rather than throws errors. The `files` property and `excludedFiles` property of the configuration is applied to every flattened item. If a flattened item has own `files` property and `excludedFiles` property, it composes those by logical AND. + +> [lib/lookup/config-array-factory.js#L601-L624](https://github.com/eslint/eslint/blob/153640180a8944af3a1c488462ed30d0c215f5ed/lib/_lookup/config-array-factory.js#L601-L624) in PoC. + +
+💡 Example: +
+{
+    "extends": ["eslint:recommended"],
+    "rules": { ... },
+    "overrides": [
+        {
+            "files": ["*.ts"],
+            "extends": ["plugin:@typescript-eslint/recommended"],
+            "rules": { ... },
+        }
+    ]
+}
+
+is flattend to: +
+[
+    // extends
+    {
+        "name": ".eslintrc.json » eslint:recommended",
+        "filePath": "node_modules/eslint/conf/eslint-recommended.js",
+        "rules": { ... }
+    },
+    // main
+    {
+        "name": ".eslintrc.json",
+        "filePath": ".eslintrc.json",
+        "rules": { ... }
+    },
+    // overrides (because it flattens recursively, extends in overrides is here)
+    {
+        "name": ".eslintrc.json#overrides[0] » plugin:@typescript-eslint/recommended",
+        "filePath": "node_modules/@typescript-eslint/eslint-plugin/dist/index.js",
+        // `matchFile` is merged from the parent `overrides` entry and itself.
+        "matchFile": { "includes": ["*.ts"], "excludes": null },
+        "parser": { ... },
+        "parserOptions": { ... },
+        "plugins": { ... },
+        "rules": { ... }
+    },
+    {
+        "name": ".eslintrc.json#overrides[0]",
+        "filePath": ".eslintrc.json",
+        "matchFile": { "includes": ["*.ts"], "excludes": null },
+        "rules": { ... }
+    }
+]
+
+
+ +## Documentation + +This enhancement should be documented in "Configuring ESLint" page. + +## Drawbacks + +- I think no problem. + +## Backwards Compatibility Analysis + +If people depend on the behavior that ESLint throws an error if they give `extends` property or `overrides` property in a config of `overrides` property, this enhancement breaks that. However, I believe that we don't need to worry. + +## Alternatives + +- We can provide a utility that merges multiple configs for this purpose. However, people have gotten used to `extends` property, and it's easy to use. +- [Array Config](minor-01-array-config.md) may be an alternative, but it cannot apply `files` and `excludedFiles` to the extended config by logical AND. + +## Open Questions + +- + +## Frequently Asked Questions + +- + +## Related Discussions + +- [#9] +- [eslint/eslint#8813] + +[#9]: https://github.com/eslint/rfcs/pull/9 +[eslint/eslint#8813]: https://github.com/eslint/eslint/issues/8813 diff --git a/designs/2019-eslintrc-improvements/minor-03-plugin-renaming.md b/designs/2019-eslintrc-improvements/minor-03-plugin-renaming.md new file mode 100644 index 00000000..0f81dc42 --- /dev/null +++ b/designs/2019-eslintrc-improvements/minor-03-plugin-renaming.md @@ -0,0 +1,201 @@ +# Config File Improvements: Plugin Renaming + +## Summary + +This proposal adds the feature that renames plugins in config files. This enhancement will be the official successor of `--rulesdir` option. And, this solves the important pain of the ecosystem by making shareable configs being able to have plugins. + +## Motivation + +People cannot define local rules with config files (rather than `--rules` CLI option). The ecosystem has hacked this problem by several plugins to solve such as `eslint-plugin-rulesdir`, `eslint-plugin-local`, etc. However, the official way is great. + +And if a shareable config depends on plugins, people have to install the plugins manually. The combination of this enhancement and `require.resolve()` function makes shareable configs being able to have plugins, so solves the pain! + +## Detailed Design + +> Proof of Concept (implementation): https://github.com/eslint/eslint/tree/proof-of-concept/config-array-in-eslintrc + +This proposal enhances [there](README.md#plugin-renaming). + +If `plugins` property of a configuration was an object, `ConfigArrayFactory` loads the value of each property as a plugin then store the loaded plugin as the key of the property. Here, `["foo", "bar"]` is equivalent to `{ "foo": "foo", "bar": "bar" }`. The object form supports file paths. + +> [lib/lookup/config-array-factory.js#L807-L828](https://github.com/eslint/eslint/blob/fedb0293ef8fb3e2e17d88bdfeb5e5cfb725a282/lib/_lookup/config-array-factory.js#L807-L828) in PoC. + +
+💡 Example: +
+{
+    "plugins": {
+        "foo": "foo", // Or long version "eslint-plugin-foo" is OK as well.
+        "abc": "xyz",
+        "local": "./tools/eslint-rules/index.js"
+    },
+    "rules": {
+        "foo/a-rule": "error",
+        "abc/a-rule": "error",
+        "local/a-rule": "error"
+    }
+}
+
+is loaded as: +
+[
+    {
+        "name": ".eslintrc.json",
+        "filePath": ".eslintrc.json",
+        "plugins": {
+            // This plugin is not renamed because property key and `id` in value is same.
+            "foo": {
+                "definition": { ... },
+                "id": "foo",
+                "filePath": "node_modules/eslint-plugin-foo/index.js",
+                "importerPath": ".eslintrc.json"
+            },
+            // This plugin is renamed because property key and `id` in value is different.
+            "abc": {
+                "definition": { ... },
+                "id": "xyz",
+                "filePath": "node_modules/eslint-plugin-xyz/index.js",
+                "importerPath": ".eslintrc.json"
+            },
+            // This plugin is renamed because property key and `id` in value is different.
+            "local": {
+                "definition": { ... },
+                "id": "./tools/eslint-rules/index.js",
+                "filePath": "tools/eslint-rules/index.js",
+                "importerPath": ".eslintrc.json"
+            }
+        },
+        "rules": {
+            "foo/a-rule": "error",
+            "abc/a-rule": "error",
+            "local/a-rule": "error"
+        }
+    }
+]
+
+
+ +### Conflict handling + +If a plugin was renamed, it can be different content to other declarations even if it has the same plugin ID. +Therefore, if a config file had a renamed plugin and another config file included the same plugin ID regardless of renamed or not, ESLint throws "plugin conflict" error. + +ESLint loads plugins which are not renamed relative to CWD (by [#7]), it doesn't conflict because of unique if there are no renamed plugins. + +### Using two shareable configs which have the same plugin ID with renaming + +This section depends on an enhancement [Array Config](minor-01-array-config.md). + +We can provide a utility to use conflicted two shareable configs. +The utility does load a given shareable config with renaming conflicted plugins. + +I assume that the utility is used only in the pretty rare case. + +```js +// .eslintrc.js +const config = require("@eslint/config") + +module.exports = [ + // Rename conflicted plugins while loading. + // It renames the prefixes of rules, environments, and processors at the same time. + // It resolves the relative paths in `parser` and `plugins` to work file on this file. + config.withConvert("eslint-config-foo", { + relativeTo: __filename, + mapPluginName: id => `foo::${id}` + }), + + "eslint-config-bar", + + { + root: true, + rules: { ... } + } +] +``` + +
+💡 Example: +
+// eslint-config-foo
+module.exports = {
+    plugins: {
+        "a-plugin": require.resolve("eslint-plugin-a-plugin")
+    },
+    parser: "./lib/parser",
+    env: {
+        "a-plugin/env": true
+    },
+    rules: {
+        eqeqeq: "error",
+        "a-plugin/x": "error",
+        "a-plugin/y": "error"
+    }
+}
+
+config.withConvert(...) method with the above config returns as: +
+[
+    {
+        "plugins": {
+            // Renaming with absolute path.
+            "foo::a-plugin": "/path/to/node_modules/eslint-config-foo/node_modules/eslint-plugin-a-plugin/index.js"
+        },
+        // absolute path.
+        "parser": "/path/to/node_modules/eslint-plugin-foo/lib/parser.js",
+        "env": {
+            "foo::a-plugin/env": true
+        },
+        "rules": {
+            "eqeqeq": "error",
+            "foo::a-plugin/x": "error",
+            "foo::a-plugin/y": "error"
+        }
+    }
+]
+
+
+ +The `config.withConvert(request, options)` method loads `extends` property and flattens `extends` property and `overrides` property recursively. Then it converts all plugin names in the configs. + +With [`extends` in `overries`](minor-02-extends-in-overrides.md) enhancement, it uses nested `overrides` properties to express logical AND conditions. + +### Deprecating `--rulesdir` CLI option + +This feature allows us to define rules in config files in local. +Therefore, now we don't have to add `--rulesdir` CLI option for each `eslint` command call, we can deprecate `--rulesdir` CLI option. + +## Documentation + +This enhancement should be documented in "Configuring ESLint" page. + +## Drawbacks + +I think no problem. + +## Backwards Compatibility Analysis + +If people depend on the behavior that ESLint throws an error if they give an object to `plugins` property, this enhancement breaks that. However, I believe that we don't need to worry. + +## Alternatives + +- [#9] is the alternative. But double duplicate features cause confusion for the ecosystem. For newcomers, a mix of articles about two config systems makes hard to understand ESLint. For non-English users, the official document is far. Also, this proposal has information that a plugin name was renamed or not, so it helps to address [Robustness Guarantee]. +- [#14] is a different idea to handle local plugins. I think people have gotten used to key-value form via JavaScript object literals. + +## Open Questions + +- + +## Frequently Asked Questions + +- + +## Related Discussions + +- [#14] +- [#9] +- [#7] + +[#14]: https://github.com/eslint/rfcs/pull/14 +[#9]: https://github.com/eslint/rfcs/pull/9 +[#7]: https://github.com/eslint/rfcs/pull/7 +[Guarantee Robustness]: https://gist.github.com/not-an-aardvark/169bede8072c31a500e018ed7d6a8915