Conversation
mdjermanovic
added
the
Initial Commenting
|
|
||
| ### `--base-path` with config lookup? | ||
|
|
||
| Should `--base-path` be allowed when config lookup is enabled (i.e. neither `--config` nor `--no-config-lookup` is passed)? In that case, ESLint searches for a config file in a file's directory and its ancestors. This means that config files outside that line of search are not considered, so that each file is required to have its associated config in an expected location. In this setup, I'm not sure how `--base-path` would be useful. |
There was a problem hiding this comment.
I think it shouldn't be allowed as it would be effectively ignored so seems better to throw an error to avoid confusion.
| The above solution is fully backward-compatible but it's hard to extend to existing configurations because changing the base path also means that all relative paths and patterns in existing configs (in `files`, `ignores` and `basePath`) must be modified to be relative to the new base path. | ||
|
|
||
| For example, in the example in the previous section, a project may already have a config that lints files in the current directory (`app`) like this: | ||
|
|
||
| ```js | ||
| export default defineConfig([ | ||
| { | ||
| name: "app-config", | ||
| files: ["**/*.{js,cjs,mjs}"], | ||
| ...myAppConfig | ||
| }, | ||
| ]); | ||
| ``` |
There was a problem hiding this comment.
On the other hand, if a config object with files: ["**/*.{js,cjs,mjs}"] is intended to apply to all .js, .cjs, and .mjs files regardless of directories, it would need to be updated to e.g., files: ["../**/*.{js,cjs,mjs}"]?
There was a problem hiding this comment.
Also, most of the default config objects (https://github.com/eslint/eslint/blob/main/lib/config/default-config.js) would only apply in the directory where the config file is?
There was a problem hiding this comment.
On the other hand, if a config object with
files: ["**/*.{js,cjs,mjs}"]is intended to apply to all .js, .cjs, and .mjs files regardless of directories, it would need to be updated to e.g.,files: ["../**/*.{js,cjs,mjs}"]?
I just tested that. A pattern like "**/*.{js,cjs,mjs}" would already match files in the parent directory, probably because universal patterns apply to all directories regardless of base path.
Also, most of the default config objects (https://github.com/eslint/eslint/blob/main/lib/config/default-config.js) would only apply in the directory where the config file is?
The ".git/" ignore pattern would apply to the directory that contains the config file, which seems suboptimal for the scenario in the example. Otherwise, the default config objects would behave the same.
|
|
||
| In order to extend this config to also lint files in `../tmp`, besides passing `--base-path=..`, the existing config object(s) in the config must be changed if they should still apply to the current directory only, either by modifying the `files` patterns as in [the first example](#example-1), or by adding an explicit `basePath` like in [the second example](#example-2). Note that in both cases, the name of the current working directory (`app`) must be explicitly added, because there is no longer an implicit way to reference it from the config. | ||
|
|
||
| To avoid this inconvenience, we could add a new property to config arrays, tentatively named `referenceLocation`. This property will hold the value of what is currently the base path (CWD or config file location), and it will be used to resolve relative `basePath` values in config objects. This will ensure that existing config arrays can be extended to lint additional files with a new common directory root without changing existing config objects. For example, with this alternative, the example above can be extended to lint `../tmp` without changing the other config object(s), like this: |
There was a problem hiding this comment.
Is the example missing referenceLocation?
There was a problem hiding this comment.
referenceLocation is a property of config arrays (as opposed to config objects). It would be passed to the ConfigArray constructor directly by ESLint along with the other options.
So it's not something that users could set arbitrarily in their configs. I'll clarify that part.
|
|
||
| ## Alternatives | ||
|
|
||
| ### Change `**/` semantics to match outside the base path |
There was a problem hiding this comment.
I still feel like this approach is the cleanest option. I know there are some questions about ignores and some other edge cases, but I feel like most users would expect **/*.js to match all JavaScript files passed on the command line.
Adding one or more new properties to config objects and then also a command line option feels like building a house of cards.
We could try this under an unstable feature flag to let people give it a try without committing to any changes.
Another alternative, which may be a bit more dramatic, is to enable the matchBase option of minimatch. This would enable patterns such as foo.js to match all paths that end with foo.js (likewise, *.js would match all JS files). This might actually work better because it does not change the behavior of ** (I think...).
There was a problem hiding this comment.
I still feel like this approach is the cleanest option. I know there are some questions about ignores and some other edge cases, but I feel like most users would expect
**/*.jsto match all JavaScript files passed on the command line.
I can try to prepare a prototype for that so we can test how it works. This will need changes in @eslint/config-array because of the ability to configure external files.
Adding one or more new properties to config objects and then also a command line option feels like building a house of cards.
Just to clarify: no new config object properties are being proposed in this RFC.
|
|
||
| ## Detailed Design | ||
|
|
||
| ### Current Behavior |
There was a problem hiding this comment.
I think it would also be helpful to provide some context as to why ESLint has this behavior today. Going back to first principles may help in this discussion.
3 participants
Summary
This RFC proposes the introduction of a new CLI option
--base-path, and a correspondingbasePathoption on theESLint. The new option will allow configuring and linting files in arbitrary locations when used together with--configor--no-config-lookup.Related Issues