feat!: Look Up Config Files From Linted File

[nzakas]

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

https://github.com/eslint/eslint/issues/18385

[nzakas]

@mdjermanovic looking for your feedback here.

[nzakas]

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

[nzakas]

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]

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

[jakebailey]

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]

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

[jakebailey]

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]

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]

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

[nzakas]

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

[jakebailey]

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]

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).

[jakebailey]

Hm, wouldn't that fail when linting the file inside the subdir, then, since the outer wasn't consulted?

[nzakas]

@jakebailey you have the prototype, try it out and let me know. :)

[jakebailey]

Hm, wouldn't that fail when linting the file inside the subdir, then, since the outer wasn't consulted?

you have the prototype, try it out and let me know. :)

It doesn't seem like the ignore is doing anything at all, so the extra config load seems spurious:

diff --git a/eslint.config.mjs b/eslint.config.mjs
index 8be61c2..b61f24b 100644
--- a/eslint.config.mjs
+++ b/eslint.config.mjs
@@ -5,6 +5,7 @@ import pluginJs from "@eslint/js";
 console.log("Loading", import.meta.url);
 
 export default [
+  {ignores: ["subdir/*"]},
   {languageOptions: { globals: globals.node }},
   pluginJs.configs.recommended,
 ];
diff --git a/rootFile.js b/rootFile.js
index 1ae0fab..1b5e315 100644
--- a/rootFile.js
+++ b/rootFile.js
@@ -1 +1,3 @@
 const unused = 1234;
+
+const wrongEq = 1 == 2;
diff --git a/subdir/eslint.config.mjs b/subdir/eslint.config.mjs
index f89d07d..7f4598e 100644
--- a/subdir/eslint.config.mjs
+++ b/subdir/eslint.config.mjs
@@ -6,4 +6,5 @@ console.log("Loading", import.meta.url);
 
 export default [
   {languageOptions: { globals: globals.node }},
+  {rules: {"eqeqeq": "error"}},
 ];
diff --git a/subdir/subFile.js b/subdir/subFile.js
index 1ae0fab..1b5e315 100644
--- a/subdir/subFile.js
+++ b/subdir/subFile.js
@@ -1 +1,3 @@
 const unused = 1234;
+
+const wrongEq = 1 == 2;

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

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

/home/jabaile/work/eslint-test/subdir/subFile.js
  3:19  error  Expected '===' and instead saw '=='  eqeqeq

✖ 3 problems (3 errors, 0 warnings)
  1 error and 0 warnings potentially fixable with the `--fix` option.

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

/home/jabaile/work/eslint-test/subdir/subFile.js
  3:19  error  Expected '===' and instead saw '=='  eqeqeq

✖ 1 problem (1 error, 0 warnings)
  1 error and 0 warnings potentially fixable with the `--fix` option.

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

/home/jabaile/work/eslint-test/subdir/subFile.js
  3:19  error  Expected '===' and instead saw '=='  eqeqeq

✖ 1 problem (1 error, 0 warnings)
  1 error and 0 warnings potentially fixable with the `--fix` option.

(This behavior also applies to ignores of subdir/ and subdir/**.)

[nzakas]

Thanks, I'll take a look.

[nzakas]

@jakebailey okay, I've looked into this, and indeed, it works as you described. I think this is likely the correct behavior in terms of not allowing a parent config file to exclude a directory that itself has a config file.

The larger question you bring up is a good one: if the parent config file can't ignore a subdirectory with a config file, then why does it load the parent config file? I suspect this is probably due to checking the current directory for a config file during traversal, which makes sense when looking up from a file but doesn't make sense when looking up from a directory. I need to dig into this logic further.

[jakebailey]

Thanks for checking; I do think the behavior is consistent, though it's maybe a little odd to not be able to say, temporarily disable linting for a dir. But I guess you would just return an empty config in that subdir.

The double loading isn't such a big deal, just something interesting I noticed since I was logging config loads to see what it was doing. 😄

[nzakas]

Understood, and rationally, I don't think that behavior is observable from within a test.

For my own curiosity, though, if I can get away with just one method (loadConfigArrayForPath()) instead of two (loadConfigArrayForFile() and loadConfigArrayForDirectory()), I'll be happy. Right now I'm forking the logic based on whether it should begin the search from the path that is passed in or strip off the last bit. If I can always strip off the last bit, I can delete a bunch of code, and that's always a good thing.

[nzakas]

So interestingly, when I changed the lookup behavior to start from the parent directory instead of the directory as it was passed, I now get the behavior that a config file in the parent directory with ignores will skip a subdirectory even if that subdirectory has a config file.

I think this now becomes an open question, which behavior do we want?

To make things more concrete:

/usr/tmp/
├── eslint.config.js    <-- ignores: ['subdir']
└── subdir/
    ├── foo.js
    └── eslint.config.js

The two possible behaviors for eslint . are:

1. /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.
2. /usr/tmp/eslint.config.js is honored, so ESLint skips traversing into subdir.

I can see arguments for both, with a slight favorite of option 2 (which, ironically, is how I thought this prototype would work when I first built it).

What does everyone think?

[anomiex]

Would Option 1 also apply to something like this?

/usr/tmp/
├── eslint.config.js    <-- ignores: ['node_modules', 'vendor']
├── node_modules/
│   └── some-package/
│       ├── index.js
│       └── eslint.config.js    <-- oops, someone forgot to exclude this when publishing the package
└── vendor/
    └── organization/some-package/
        ├── foo.php
        └── eslint.config.js    <-- oops, someone forgot to exclude this when publishing the package to Packagist.

For that matter, even the scanning for Option 1 to find out if an eslint.config.js exists somewhere in the ignored subdirectory tree might turn out to be problematic in a monorepo. When packages depend on each other it's easy to wind up with symlink loops.

[mdjermanovic]

/usr/tmp/
├── eslint.config.js    <-- ignores: ['subdir']
└── subdir/
    ├── foo.js
    └── eslint.config.js

The two possible behaviors for eslint . are:

1. /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.

2. /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]

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]

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

[nzakas]

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]

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

[nzakas]

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]

I agree with option 2 as presented in https://github.com/eslint/rfcs/pull/120#issuecomment-2221153450: 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]

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]

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?