feat!: Look Up Config Files From Linted File #120

nzakas · 2024-06-17T14:29:10Z

Summary

This RFC proposes a new way to look up where eslint.config.js is located. Currently, we perform the lookup from the current working directory. However, that has a number of downsides, especially as it relates to monorepo usage. This proposes that we change the lookup strategy to be from the file being linted instead of the cwd.

Related Issues

eslint/eslint#18385

jakebailey · 2024-06-17T15:54:02Z

designs/2024-config-lookup-from-file/README.md

+#### v9.x
+
+* Add the `unstable_config_lookup_from_file` flag while the feature is in development.
+* Once stabilized, retire the `unstable_config_lookup_from_file` flag and create the `v10_config_lookup_from_file` flag, which will let people opt-in to the breaking behavior.


I think it's unlikely that anyone is running ESLint from a completely different tree than the file they're linting; I would think anyone who got v9 to work likely moved everything into the root config, so searching from a given file will still find that same config.

Is it possible that this change could be made as the default without waiting for a major?

(Maybe someone had worked around this config issue by creating multiple config files and sharing them in every dir, rewriting a shared config to ensure the paths work, but that seems less than likely without helpers to enable that sort of thing reliably.)

Is it possible that this change could be made as the default without waiting for a major?

Yes, with a flag, as described in the RFC. 😄

Yes, that's why I'm commenting on this line! A flag can't be set in a config, so there's no way to force it to be on except to always remember to pass it at the CLI, insert it in scripts, and/or configure editors to pass the flag in. And if I can pass the flag in, I can probably also just confgure tooling to cd before running eslint or similar.

Of course, it can't be a config option, because this option is intended to control config lookup, so it's a chicken and the egg problem.

Hence, hoping that this could just "be changed" without going through the full cycle if nobody in this situation was able to correctly configure things anyway.

This is the process we need to go through for any breaking change, which this definitely is (it's breaking a core assumption of how ESLint works). The flags allow people to try it out before it's fully released and know when it's stable enough to rely on.

IIRC, you mentioned that you're using the ESLint class for linting DefinitelyTyped? In which case, you can pass the flags to the constructor to try it out.

Yes, we can pass whatever flags we want in dtslint, but we can't also enforce that for people using their editors on DT, or anyone else in a similar situation. It's also probable that anyone in this situation has not migrated to flat config anyway. It just seems to me like this won't actually break anyone in practice (rather, help people go to v9).

@jakebailey Not to sound like a broken record (pun intended), but we have this process for breaking changes for a reason. It's not enough that you think something won't affect people that much. Semver is a contract that we take seriously, and while all of your concerns are valid, they do not change how we implement breaking changes.

As I said in the original issue, I'm not opposed to doing another major version release this year, so you'll just have to be patient.

At this point, the most helpful you can do is read through the RFC, leave your feedback about how it would affect your use case, and once implemented, try it out and leave feedback on the implementation.

fasttime

Thanks for the RFC. I left some comments/questions.

fasttime · 2024-06-17T17:14:00Z

designs/2024-config-lookup-from-file/README.md

+1. Update `lib/eslint/eslint.js`:
+    1. Create a new `ConfigLoader` based on the presence or absence of the flag. This should be used in the `ESLint#lintText()` and `ESLint#lintFiles()` methods.
+    1. Update `ESLint#calculateConfigForFile()` to use the config loader.
+    1. Update `ESLint#findConfigFile()` to use the config loader.


ESLint#findConfigFile() is called without arguments. What file will it return when unstable_config_lookup_from_file is set?

Another thing we should consider is that ESLint#findConfigFile() currently throws an error if it doesn't find a config file in the current directory or above. With the new config lookup it seems possible that users will start putting ESLint configs files in the subdirectories that contain JS files, for example in the packages of a monorepo, instead of having a eslint.config.js at the top-level. This seems a legitimate use case so probably there should be no error if a config file is not found.

We will still throw an error if no config file is found in this method. We still need a config file for ESLint to run.

We could basically keep the current behavior: start looking for a config file from the current directory upwards and throw an error if none is found. That would just no longer help locating config files in subdirectories.

We still need a config file for ESLint to run.

Do you mean a config file for the current directory (like now)?

Sorry, I'm having trouble understanding what you're suggesting or asking.

What I'm saying is that the current behavior remains the same. When we attempt to lint a file, we must have a config file explaining how to do it. Running ESLint without a config file will always throw an error. It doesn't matter what resolution logic ESLint is using to find that config file -- if it's not found, that's an error.

I was talking about the behavior of ESLint#findConfigFile(): currently, it starts looking for a config file from the current directory upwards and throws an error if none is found. This method is not used to lint files, it's only called by API consumers to locate the config file. If I understand correctly, you're saying that we should keep that behavior even with the new config resolution despite the fact that there could be other config files?

Gotcha. Yes, my intent was to let it work the same way it does now. However, it would make sense to let people also pass in a directory or file to look up from. I'll update the RFC to reflect that.

Oh, I actually already updated that portion to reflect this. :)

Oh, I totally missed that. Thanks for the update!

fasttime · 2024-06-17T17:22:04Z

designs/2024-config-lookup-from-file/README.md

+    1. Create a new `ConfigLoader` based on the presence or absence of the flag. This should be used in the `ESLint#lintText()` and `ESLint#lintFiles()` methods.
+    1. Update `ESLint#calculateConfigForFile()` to use the config loader.
+    1. Update `ESLint#findConfigFile()` to use the config loader.
+    1. Update `getOrFindUsedDeprecatedRules()` to use the config loader.


ESLint#getOrFindUsedDeprecatedRules() returns an array synchronously, but both findConfigFileForFile() and loadConfigArrayForFile() return a promise, so it's not clear to me how the config loader will be used. Can you clarify?

I'm not 100% sure at this point, but this is a minor implementation detail that I don't think we should be caught up on. Worst case scenario I just change the function to be async.

Worst case scenario I just change the function to be async.

That should work if the call to getOrFindUsedDeprecatedRules() is moved directly into processLintReport() out of the get(), because processLintReport() is called by async functions only.

Right, as I said, this is a pretty minor implementation detail that I'm sure has a solution once I get to that point.

fasttime · 2024-06-17T17:49:40Z

designs/2024-config-lookup-from-file/README.md

+    1. Update `ESLint#findConfigFile()` to use the config loader.
+    1. Update `getOrFindUsedDeprecatedRules()` to use the config loader.
+1. Update `findFiles()` in `lib/eslint/eslint-helpers.js` to use the config loader. This also requires a change to the file system walking logic because `fswalk` filter functions are synchronous, but we'll need them to be asynchronous to use with the config loader. I plan on using [`humanfs`](https://github.com/humanwhocodes/humanfs/blob/main/docs/directory-operations.md#reading-directory-entries-recursively) (which I wrote).
+


Currently, the CLI flag -c and the corresponding ESLint constructor option overrideConfigFile can be used to override the default config lookup with an arbitrary file. It's not obvious how these options will behave when unstable_config_lookup_from_file is set. I can imagine three possibilities:

Keep the current behavior and always use the config specified with -c in place of the per-file configs that the new config loader would otherwise look up.

Merge the config specified with -c with the per-file configs loaded by the new config loader.

Ignore/disallow the -c flag. In this case we should also ignore/disallow the constructor option overrideConfigFile when it's not true.

Good call out. Option 1 is what will happen. I'll add a FAQ.

Option 1 should also make it possible to opt out of the new config resolution when it becomes the default. If a user for some reason prefers to keep the current behavior, they could do so by specifying a config file explicitly, e.g.:

npx eslint -c eslint.config.js

People can also "opt out" by having a eslint.config.js in the root of their project and nowhere else.

designs/2024-config-lookup-from-file/README.md

fasttime · 2024-06-17T18:26:45Z

designs/2024-config-lookup-from-file/README.md

+
+This method behaves similarly as the current [`ESLint#calculateConfigForFile()` method](https://github.com/eslint/eslint/blob/455f7fd1662069e9e0f4dc912ecda72962679fbe/lib/eslint/eslint.js#L1165-L1174) except that it returns the `FlatConfigArray` instead of the config for the given file. All of the logic in `ESLint#calculateConfigForFile()` will be moved to the `ConfigLoader` class and the `ESLint` class will call the `ConfigLoader` method to provide this functionality. This requires moving the [`calculateConfigArray()` function](https://github.com/eslint/eslint/blob/455f7fd1662069e9e0f4dc912ecda72962679fbe/lib/eslint/eslint.js#L369) into `ConfigLoader` (as a private method).
+
+In most of the `ESLint` class logic, `FlatConfigArray#getConfig()` will need to be replaced by `ConfigLoader#loadConfigArrayForFile()` to ensure that the file system is always searched to find the correct configuration.


This could get tricky because ConfigLoader#loadConfigArrayForFile() returns a promise whereas FlatConfigArray#getConfig() is called in many synchronous functions. Probably we will need a mechanism to retrieve the config arrays synchronously once the config files have been cached.

Yeah, I've already gone through and prototyped this out, and this will work as I've described it.

I actually started with a synchronous way to retrieve configs and then removed it because it wasn't needed. I can always add it back if I discover it's needed at some point.

designs/2024-config-lookup-from-file/README.md

Co-authored-by: Francesco Trotta <[email protected]>

designs/2024-config-lookup-from-file/README.md

Co-authored-by: Francesco Trotta <[email protected]>

nzakas · 2024-06-20T15:10:33Z

@mdjermanovic looking for your feedback here.

mdjermanovic · 2024-06-24T09:11:16Z

designs/2024-config-lookup-from-file/README.md

+    1. Update `ESLint#calculateConfigForFile()` to use the config loader.
+    1. Update `ESLint#findConfigFile()` to use the config loader. To preserve backwards compatibility, when called without an argument, this method will start the config file search from the current working directory; it will also accept an argument, which is the path to search from.
+    1. Update `getOrFindUsedDeprecatedRules()` to use the config loader.
+1. Update `findFiles()` in `lib/eslint/eslint-helpers.js` to use the config loader. This also requires a change to the file system walking logic because `fswalk` filter functions are synchronous, but we'll need them to be asynchronous to use with the config loader. I plan on using [`humanfs`](https://github.com/humanwhocodes/humanfs/blob/main/docs/directory-operations.md#reading-directory-entries-recursively) (which I wrote).


If there's no config file in the root, but there is a config file in a subdirectory, e.g., src/eslint.config.js, what happens when user runs:

eslint . from the project root. Does it error saying that there's no config file?

eslint src from the project root. Does it load src/eslint.config.js and lint files in src?

This works the same way as in eslintrc: A config file is looked up from the file being linted not from the patterns passed on the command line. So eslint . and eslint src work the same for the same files. If there's ./src/foo.js and ./src/eslint.config.js, then when ESLint goes to lint ./src/foo.js it loads ./src/eslint.config.js. Then, in the same run, if there's a ./tests/eslint.config.js, that will be used for ./tests/foo.test.js.

In that case, how does the search determine which directories should be traversed when running eslint . and there is no config file in the root? Will it skip those matched by default ignores?

Everything is dependent on the directory that's visited. For each directory, we search up to find a config file and use that to determine whether or not the directory should be skipped. The same is true for files. We always search for the relevant config file.

That would mean we'd search node_modules too, and lint files in it if some packages have an eslint.config.js file published?

I think we can better answer these questions with a prototype, so I'm going to work on that.

I've tried the above scenario with the issue18385b branch. Both npx eslint . --flag unstable_config_lookup_from_file and npx eslint src --flag unstable_config_lookup_from_file result in ESLint couldn't find an eslint.config.(js|mjs|cjs) file. error.

Furthermore, if I add an empty (export default []) config file in the root, both commands now work but neither seems to use src/eslint.config.js.

I've tried the latest version of the issue18385b branch with a test project that has a src/eslint.config.js file but no root eslint.config.js file.

Running eslint . from the root fails this way:

$ npx eslint . --flag unstable_config_lookup_from_file --debug eslint:cli CLI args: [ '.', '--flag', 'unstable_config_lookup_from_file', '--debug' ] +0ms eslint:cli Using flat config? true +3ms eslint:cli Running on files +4ms eslint:cli Checking for inactive flags +1ms eslint:eslint Using file patterns: . +0ms eslint:eslint Deleting cache file at C:\projects\tmp\tmp\.eslintcache +1ms eslint:config-loader Calculating config for directory C:/projects/tmp/tmp/node_modules +0ms eslint:config-loader Searching for eslint.config.js +1ms Oops! Something went wrong! :( ESLint: 9.5.0 ESLint couldn't find an eslint.config.(js|mjs|cjs) file. From ESLint v9.0.0, the default configuration file is now eslint.config.js. If you are using a .eslintrc.* file, please follow the migration guide to update your configuration file to the new format: https://eslint.org/docs/latest/use/configure/migration-guide If you still have problems after following the migration guide, please stop by https://eslint.org/chat/help to chat with the team.

Does this mean that there must be an eslint.config.js in the root (or ancestry) of the search pattern?

Running eslint src from the root fails in the same way:

$ npx eslint src --flag unstable_config_lookup_from_file --debug eslint:cli CLI args: [ 'src', '--flag', 'unstable_config_lookup_from_file', '--debug' ] +0ms eslint:cli Using flat config? true +3ms eslint:cli Running on files +3ms eslint:cli Checking for inactive flags +1ms eslint:eslint Using file patterns: src +0ms eslint:eslint Deleting cache file at C:\projects\tmp\tmp\.eslintcache +1ms eslint:config-loader Calculating config for directory C:/projects/tmp/tmp/node_modules +0ms eslint:config-loader Searching for eslint.config.js +0ms Oops! Something went wrong! :( ESLint: 9.5.0 ESLint couldn't find an eslint.config.(js|mjs|cjs) file. From ESLint v9.0.0, the default configuration file is now eslint.config.js. If you are using a .eslintrc.* file, please follow the migration guide to update your configuration file to the new format: https://eslint.org/docs/latest/use/configure/migration-guide If you still have problems after following the migration guide, please stop by https://eslint.org/chat/help to chat with the team.

This is, I believe, an unexpected behavior because the argument is src so the absence of config files in other directories shouldn't matter I think. Running cd src && npx eslint . --flag unstable_config_lookup_from_file --debug works well.

Yes, that's unexpected for sure. You should not need a root eslint.config.js file for this to work.

I'll take a look.

Okay, so I've updated the behavior such that traversing directories does not require a config file but will honor one if it's found. There will be no error thrown when a config file is not found when traversing directories.

There will be an error thrown when attempting to lint a file that doesn't have a config file in its ancestry.

I believe this behavior mimics what we have now.

nzakas · 2024-06-26T15:39:41Z

So @fasttime was right, we do need a synchronous method to retrieve configuration information. I've updated the RFC with that.

nzakas · 2024-06-27T18:50:12Z

I've got a prototype mostly working. By default, of course, it uses the current lookup scheme. You'll need to enable the new lookup scheme using --flag unstable_config_lookup_from_file:

npm install eslint/eslint#issue18385b

nzakas · 2024-06-27T18:50:39Z

@jakebailey it would be helpful if you could try out the prototype and share your feedback here.

jakebailey · 2024-06-27T19:09:55Z

Will do! I've been really busy recently (sorry to not deep dive on the text here), but made a TODO item when I saw your last comment! 😄

nzakas · 2024-07-01T15:53:51Z

A quick update: I've now verified that all of our existing tests for the ESLint class pass when this feature is enabled. 🎉 So, disruption should be minimal for most users.

jakebailey · 2024-07-01T16:03:44Z

I gave it a test, but wasn't able to get it to work how I expected. Made this test repo: https://github.com/jakebailey/eslint-config-lookup-test

When I do eslint ., the file in the subdir still gets the top level config, but subdir contains its own config which turns off all rules. Logging shows that only one config was loaded. But when I point the CLI to the one file, it loads the correct config.

$ npx eslint --flag unstable_config_lookup_from_file .
Loading file:///home/jabaile/work/eslint-test/eslint.config.mjs?mtime=1719849618920

/home/jabaile/work/eslint-test/subdir/eslint.config.mjs
  2:8  error  'pluginJs' is defined but never used  no-unused-vars

✖ 1 problem (1 error, 0 warnings)

$ npx eslint --flag unstable_config_lookup_from_file ./subdir/subFile.js
Loading file:///home/jabaile/work/eslint-test/subdir/eslint.config.mjs?mtime=1719849629220

jakebailey · 2024-07-01T16:05:22Z

Updated the test after writing the above to make the behavior more clear:

$ npx eslint --flag unstable_config_lookup_from_file .
Loading file:///home/jabaile/work/eslint-test/eslint.config.mjs?mtime=1719849618920

/home/jabaile/work/eslint-test/rootFile.js
  1:7  error  'unused' is assigned a value but never used  no-unused-vars

/home/jabaile/work/eslint-test/subdir/eslint.config.mjs
  2:8  error  'pluginJs' is defined but never used  no-unused-vars

/home/jabaile/work/eslint-test/subdir/subFile.js
  1:7  error  'unused' is assigned a value but never used  no-unused-vars

✖ 3 problems (3 errors, 0 warnings)

$ npx eslint --flag unstable_config_lookup_from_file ./subdir           
Loading file:///home/jabaile/work/eslint-test/eslint.config.mjs?mtime=1719849618920

/home/jabaile/work/eslint-test/subdir/eslint.config.mjs
  2:8  error  'pluginJs' is defined but never used  no-unused-vars

/home/jabaile/work/eslint-test/subdir/subFile.js
  1:7  error  'unused' is assigned a value but never used  no-unused-vars

✖ 2 problems (2 errors, 0 warnings)

$ npx eslint --flag unstable_config_lookup_from_file ./subdir/subFile.js 
Loading file:///home/jabaile/work/eslint-test/subdir/eslint.config.mjs?mtime=1719849629220

I would expect that all of these are loading both config files and applying them to their respective files.

nzakas · 2024-07-01T20:48:23Z

Thanks, I'll take a look and see what's going on.

nzakas · 2024-07-01T21:25:57Z

@jakebailey I just pushed a commit to fix the error. Please give it another try at your convenience.

jakebailey · 2024-07-01T21:32:26Z

Much better, though for some reason the config above subdir is being loaded when run on subdir itself.

Good:

$ npx eslint --flag unstable_config_lookup_from_file .
Loading file:///home/jabaile/work/eslint-test/eslint.config.mjs?mtime=1719849618920
Loading file:///home/jabaile/work/eslint-test/subdir/eslint.config.mjs?mtime=1719849629220

/home/jabaile/work/eslint-test/rootFile.js
  1:7  error  'unused' is assigned a value but never used  no-unused-vars

✖ 1 problem (1 error, 0 warnings)

Good, though loaded an extra config:

$ npx eslint --flag unstable_config_lookup_from_file ./subdir
Loading file:///home/jabaile/work/eslint-test/eslint.config.mjs?mtime=1719849618920
Loading file:///home/jabaile/work/eslint-test/subdir/eslint.config.mjs?mtime=1719849629220

Good:

$ npx eslint --flag unstable_config_lookup_from_file ./subdir/subFile.js
Loading file:///home/jabaile/work/eslint-test/subdir/eslint.config.mjs?mtime=1719849629220

nzakas · 2024-07-02T15:43:26Z

Loading the extra config is expected because we need to check that subdir isn't being ignored (which would happen from a config in its ancestry).

mdjermanovic · 2024-07-11T16:35:34Z

/usr/tmp/
├── eslint.config.js    <-- ignores: ['subdir']
└── subdir/
    ├── foo.js
    └── eslint.config.js
The two possible behaviors for eslint . are:

/usr/tmp/eslint.config.js is disregarded, so ESLint traverses into subdir and lints the two files there according to /usr/tmp/subdir/eslint.config.js.

/usr/tmp/eslint.config.js is honored, so ESLint skips traversing into subdir.

With option 1, I believe eslint . would always need to traverse all subdirectories, so it seems that option 2 makes more sense.

Also, option 2 is how eslint . works in eslintrc. But, if you run eslint subdir, and subdir/.eslintrc config file has "root": true, it will disregard the root .eslintrc config file, even though it's in the current working directory and has subdir in ignorePatterns, and will lint contents of subdir.

nzakas · 2024-07-11T18:46:46Z

Also, option 2 is how eslint . works in eslintrc. But, if you run eslint subdir, and subdir/.eslintrc config file has "root": true, it will disregard the root .eslintrc config file, even though it's in the current working directory and has subdir in ignorePatterns, and will lint contents of subdir.

That's a good point. I think not covering that edge case from eslintrc is okay because root: true was mostly there to prevent additional rules from being applied by ancestor config files, which is already the case with flat config.

nzakas · 2024-07-11T18:47:11Z

@anomiex instead of hypothesizing, you could use the prototype and let us know what happens. 😄

nzakas · 2024-07-12T20:21:50Z

I pushed a new branch with the changed lookup behavior, where we now look up from the parent directory first. That allows option 2 as discussed above.

npm install eslint/eslint#issue18385c

nzakas · 2024-07-16T14:48:31Z

I've updated the RFC to describe option 2, as it seems like that is the direction to go in.

mdjermanovic · 2024-07-20T13:22:00Z

designs/2024-config-lookup-from-file/README.md

+### What happens if there's a `eslint.config.js` file with an ignore pattern of `subdir` in the parent directory `subdir`?
+
+Here's the structure in question:
+
+```
+/usr/tmp/
+├── eslint.config.js    <-- ignores: ["subdir"]
+└── subdir/
+    ├── foo.js
+    └── eslint.config.js
+```
+
+When `eslint subdir/foo.js` is called, no files will be linted because the root `eslint.config.js` file ignores `subdir`.


With issue18385c branch, in this scenario, eslint subdir/foo.js does lint the file. eslint subdir doesn't though.

Also, if linting subdir is run from another subdirectory, it does lint subdir, which looks a bit inconsistent.

$ npx eslint --flag unstable_config_lookup_from_file subdir/foo.js C:\projects\tmp\tmp\subdir\foo.js 1:1 error 'a' is not defined no-undef ✖ 1 problem (1 error, 0 warnings) milos@DESKTOP-4GFK8EQ MINGW64 /c/projects/tmp/tmp $ npx eslint --flag unstable_config_lookup_from_file subdir Oops! Something went wrong! :( ESLint: 9.5.0 You are linting "subdir", but all of the files matching the glob pattern "subdir" are ignored. If you don't want to lint these files, remove the pattern "subdir" from the list of arguments passed to ESLint. If you do want to lint these files, explicitly list one or more of the files from this glob that you'd like to lint to see more details about why they are ignored. * If the file is ignored because of a matching ignore pattern, check global ignores in your config file. https://eslint.org/docs/latest/use/configure/ignore * If the file is ignored because no matching configuration was supplied, check file patterns in your config file. https://eslint.org/docs/latest/use/configure/configuration-files#specifying-files-with-arbitrary-extensions * If the file is ignored because it is located outside of the base path, change the location of your config file to be in a parent directory. milos@DESKTOP-4GFK8EQ MINGW64 /c/projects/tmp/tmp $ cd subdir1 milos@DESKTOP-4GFK8EQ MINGW64 /c/projects/tmp/tmp/subdir1 $ npx eslint ../subdir C:\projects\tmp\tmp\subdir\foo.js 1:1 error 'a' is not defined no-undef ✖ 1 problem (1 error, 0 warnings)

nzakas · 2024-07-22T15:02:36Z

At this point, I'd like to propose that we approve this RFC.

The prototypes were meant to illustrate behavior and not be completely bug-free. If we can agree that option 2, which is currently represented in this RFC, is the way to go, then I think the RFC process is complete and I can move on to bug fixing.

What do you think?

mdjermanovic · 2024-07-25T18:10:38Z

I agree with option 2 as presented in #120 (comment): with that setup, eslint . should not traverse into subir. But, with the same setup, I'm still not sure if eslint subdir, and especially eslint subdir/foo.js should result in files being ignored (as in the FAQ of this RFC document).

If we can leave these particular details open for discussion on the implementation PR (as you noted, it's much easier to discuss this on a working implementation), I agree with approving the RFC and proceeding to the implementation.

mdjermanovic · 2024-07-25T21:21:57Z

In case it might be helpful, here's a stackblitz example we made while discussing this in today's meeting, demonstrating eslintrc behavior:

https://stackblitz.com/edit/stackblitz-starters-wjz9em?file=package.json

nzakas · 2024-07-26T15:02:27Z

Okay, just to summarize the current behavior of option 2 in the prototype:

npx eslint .                 # Does not traverse into subdir, doesn't lint files
npx eslint subdir            # Does not traverse into subdir, doesn't lint files
npx eslint subdir/foo.js     # Does lint file
npx eslint subdir/*.js       # Does not traverse into subdir, doesn't lint files
npx eslint ../subdir         # Does lint file
npx eslint ../subdir/foo.js  # Does lint file
npx eslint ../subdir/*.js    # Does lint file

Looking at this and with the benefit of some sleep, I think the expectation is that all but the first (npx eslint .) should lint the file. The explanation:

When passed a directory, ESLint should traverse all of the children of that directory unless it contains an eslint.config.js file with ignores that specifies children to ignore. So when passed . the only config file that matters is ./eslint.config.js, and when subdir is passed, the only config file that matters is subdir/eslint.config.js.
When passed a file, ESLint should look up the nearest config file from that file to determine how to interpret it.
When passed a glob pattern, ESLint should follow the above rules when the glob pattern evaluates to a file or directory.

Does that make sense?

mdjermanovic · 2024-07-27T18:27:24Z

When passed a directory, ESLint should traverse all of the children of that directory unless it contains an eslint.config.js file with ignores that specifies children to ignore. So when passed . the only config file that matters is ./eslint.config.js,

To clarify which config files matter, what should happen in the following case when passed . in /usr/tmp/:

/usr/tmp/
├── eslint.config.js       <-- ignores: ['**/subsubdir1']
└── subdir/
    ├── eslint.config.js   <-- ignores: ['**/subsubdir2'] 
    ├── subsubdir1/
    │   └── file.js
    └── subsubdir2/
        └── file.js

./eslint.config.js doesn't ignore subdir, so ESLint should go into subdir. But, inside the subdir/, only subdir/eslint.config.js matters? So at the end, subdir/subsubdir1/file.js is linted (although its directory subdir/subsubdir1/ would be ignored by the root ./eslint.config.js) while subdir/subsubdir2/file.js is ignored?

(this would match eslintrc behavior with root: true in nested config files)

nzakas · 2024-07-29T14:24:52Z

So at the end, subdir/subsubdir1/file.js is linted (although its directory subdir/subsubdir1/ would be ignored by the root ./eslint.config.js) while subdir/subsubdir2/file.js is ignored?

That's correct. We go into . first, read ./eslint.config.js to see if it ignores any of the direct children, and because it doesn't, we then traverse into subdir. Once in subdir, we read subdir/eslint.config.js, which ignores subdir/subdir2 but not subdir/subdir1.

fasttime · 2024-07-30T07:05:15Z

When passed a glob pattern, ESLint should follow the above rules when the glob pattern evaluates to a file or directory.

Does this mean that eslint * would behave as if all files and directories in the current directory were specified? In that case, for the eslint repo for example, that would mean that subdirectories like docs, node_modules, .git, tmp, etc. should be linted, because they do not contain an eslint.config.js file with ignores.

nzakas · 2024-07-30T14:05:18Z

Does this mean that eslint * would behave as if all files and directories in the current directory were specified?

Yes. It would match every subdirectory of ..

that would mean that subdirectories like docs, node_modules, .git, tmp, etc. should be linted,

No. node_modules and .git are default ignores and are applied regardless if a config file is found. docs and tmp would end up being traversed but would only be linted if a config file was found.

I'll add this to the FAQ.

mdjermanovic · 2024-07-30T16:13:26Z

designs/2024-config-lookup-from-file/README.md

+### What happens when I run `eslint *`?
+
+Consider this example:
+
+```
+/usr/tmp/
+└── subdir/
+    ├── eslint.config.js
+    ├── subsubdir1/
+    │   └── file.js
+    └── subsubdir2/
+        └── file.js
+```
+
+When you run `eslint *`, it's the same as if you ran `eslint subdir`, as it matches all immediate children of `.`. If the immediate children include `node_modules` and `.git`, those will still be ignored because they are default ignores in ESLint. All other subdirectories will be traversed but will only be linted if there's an `eslint.config.js` file present, as is the case with `./subdir` in this example.


Globs match only files, so eslint * run in /user/tmp/ should not lint files in subdir because * doesn't match them.

At least on Windows, * matches directories when the shell expansion happens on the command line:

$ node ./bin/eslint.js * --debug eslint:cli CLI args: [ 'bin', 'build', 'CHANGELOG.md', 'conf', 'CONTRIBUTING.md', 'coverage', 'docs', 'eslint.config.js', 'knip.jsonc', 'lib', 'LICENSE', 'Makefile.js', 'messages', 'node_modules', 'package.json', 'packages', 'README.md', 'SUPPORT.md', 'templates', 'test.js', 'tests', 'tmp', 'tools', 'versions.json', 'wdio.conf.js', 'webpack.config.js', '--debug' ]

Ah okay. I didn't notice that the pattern was being expanded by the shell in this case.

At least on Windows, * matches directories when the shell expansion happens on the command line

Yes, but when the glob is passed through as-is to ESLint, it matches only files. In both flat config and eslintrc modes, eslint "*" lints only files in the current working directory. Another example: eslint "**/subdir" doesn't lint contents of directories named subdir; it would only lint files named subdir.

The question wasn't about eslint "*", though. 😄

I really don't think it's worth our time to start diving into edge cases at this point. That's what implementation is for.

nzakas · 2024-07-30T19:49:43Z

Just a reminder, if we agree with this comment, then the next step is to approve this RFC. If I implement the behavior as described in the comment, then you'll be free to test "what if" situations to find inconsistencies. 😄

fasttime · 2024-07-31T06:30:20Z

Just a reminder, if we agree with this comment, then the next step is to approve this RFC. If I implement the behavior as described in the comment, then you'll be free to test "what if" situations to find inconsistencies. 😄

Works for me 👍 I'd leave it to @mdjermanovic to approve the RFC once his concerns are addressed.

nzakas · 2024-07-31T14:06:04Z

@fasttime if that's the case, please mark the PR as approved.

mdjermanovic

LGTM, I agree with #120 (comment).

fasttime

LGTM.

nzakas · 2024-07-31T15:14:43Z

Moving to final commenting.

feat!: Look Up Config Files From Linted File

6949683

eslint-github-bot bot added breaking feature labels Jun 17, 2024

nzakas added the Initial Commenting This RFC is in the initial feedback stage label Jun 17, 2024

nzakas mentioned this pull request Jun 17, 2024

Change Request: Make it easier to inherit flat configs from the repo root eslint/eslint#18385

Closed

1 task

jakebailey reviewed Jun 17, 2024

View reviewed changes

fasttime reviewed Jun 18, 2024

View reviewed changes

nzakas and others added 3 commits June 18, 2024 10:29

Update designs/2024-config-lookup-from-file/README.md

e0a95e8

Co-authored-by: Francesco Trotta <[email protected]>

Update designs/2024-config-lookup-from-file/README.md

51a75f0

Co-authored-by: Francesco Trotta <[email protected]>

Incorporate feedback

a6afa8a

fasttime reviewed Jun 19, 2024

View reviewed changes

designs/2024-config-lookup-from-file/README.md Outdated Show resolved Hide resolved

fasttime reviewed Jun 19, 2024

View reviewed changes

designs/2024-config-lookup-from-file/README.md Outdated Show resolved Hide resolved

nzakas and others added 2 commits June 19, 2024 09:57

Update designs/2024-config-lookup-from-file/README.md

d94deba

Co-authored-by: Francesco Trotta <[email protected]>

Fix copy-paste error

89d41a2

nzakas mentioned this pull request Jun 20, 2024

feat: Implement feature flags eslint/eslint#18516

Merged

1 task

mdjermanovic reviewed Jun 24, 2024

View reviewed changes

nzakas added 2 commits June 26, 2024 11:38

Add getCachedConfigArrayForPath() method

e9637e1

Add PR link

b0d51dd

Update design with option 2

3865ab6

nzakas mentioned this pull request Jul 17, 2024

Allow cwd on command line eslint/eslint#18688

Closed

1 task

mdjermanovic reviewed Jul 20, 2024

View reviewed changes

Update FAQ

6e0f3fa

Add FAQ

2dcef73

mdjermanovic reviewed Jul 30, 2024

View reviewed changes

mdjermanovic approved these changes Jul 31, 2024

View reviewed changes

fasttime approved these changes Jul 31, 2024

View reviewed changes

nzakas added Final Commenting This RFC is in the final week of commenting and removed Initial Commenting This RFC is in the initial feedback stage labels Jul 31, 2024

nzakas mentioned this pull request Aug 1, 2024

feat: Implement alternate config lookup eslint/eslint#18742

Merged

1 task

nzakas merged commit e0a3299 into main Aug 7, 2024
5 checks passed

nzakas mentioned this pull request Sep 2, 2024

Change Request: Flat Config does not support FileEnumerator eslint/eslint#18087

Open

1 task


		This method behaves similarly as the current [`ESLint#calculateConfigForFile()` method](https://github.com/eslint/eslint/blob/455f7fd1662069e9e0f4dc912ecda72962679fbe/lib/eslint/eslint.js#L1165-L1174) except that it returns the `FlatConfigArray` instead of the config for the given file. All of the logic in `ESLint#calculateConfigForFile()` will be moved to the `ConfigLoader` class and the `ESLint` class will call the `ConfigLoader` method to provide this functionality. This requires moving the [`calculateConfigArray()` function](https://github.com/eslint/eslint/blob/455f7fd1662069e9e0f4dc912ecda72962679fbe/lib/eslint/eslint.js#L369) into `ConfigLoader` (as a private method).

		In most of the `ESLint` class logic, `FlatConfigArray#getConfig()` will need to be replaced by `ConfigLoader#loadConfigArrayForFile()` to ensure that the file system is always searched to find the correct configuration.

feat!: Look Up Config Files From Linted File #120

feat!: Look Up Config Files From Linted File #120

Conversation

nzakas commented Jun 17, 2024

Summary

Related Issues

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jakebailey Jun 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fasttime left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fasttime Jun 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nzakas Jun 18, 2024 • edited Loading

Choose a reason for hiding this comment

nzakas commented Jun 20, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nzakas commented Jun 26, 2024

nzakas commented Jun 27, 2024

nzakas commented Jun 27, 2024

jakebailey commented Jun 27, 2024

nzakas commented Jul 1, 2024

jakebailey commented Jul 1, 2024

jakebailey commented Jul 1, 2024 • edited Loading

nzakas commented Jul 1, 2024

nzakas commented Jul 1, 2024

jakebailey commented Jul 1, 2024

nzakas commented Jul 2, 2024

mdjermanovic commented Jul 11, 2024

nzakas commented Jul 11, 2024

nzakas commented Jul 11, 2024

nzakas commented Jul 12, 2024

nzakas commented Jul 16, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nzakas commented Jul 22, 2024

mdjermanovic commented Jul 25, 2024

mdjermanovic commented Jul 25, 2024

nzakas commented Jul 26, 2024

mdjermanovic commented Jul 27, 2024

nzakas commented Jul 29, 2024

fasttime commented Jul 30, 2024

nzakas commented Jul 30, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mdjermanovic Jul 31, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nzakas commented Jul 30, 2024

jakebailey Jun 18, 2024 •

edited

Loading

fasttime Jun 19, 2024 •

edited

Loading

nzakas Jun 18, 2024 •

edited

Loading

jakebailey commented Jul 1, 2024 •

edited

Loading

mdjermanovic Jul 31, 2024 •

edited

Loading