New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Change Request: Make it easier to inherit flat configs from the repo root #18385
Comments
Thanks for writing this all up, it's very helpful. Some followup questions:
Anything else about how you're using ESLint would be helpful in thinking this through. |
Answering directly first:
We run eslint programmatically via
Individual file names, but this is an artifact of DT's old layout where the only source of config were Getting into more details: DT is sort of weird; we have a wrapper It's extra weird becuase So, no, we aren't actualy linting all in one go. However, I would like to be able to modify things such that That being said, downstream tooling that uses ESLint has to manually create new ESLint instances based on where configs are. For example, However, DT is itself very odd in that we can get away with never running |
I also forgot to mention again that one of DT's main features is "self service", where a given package can have owners that are allowed to approve PRs to their code, and authors/owners can merge their own code. Moving all config to the top level breaks that model, as any lint rule changes that need to be made will require DT maintainer (so, TS team) approval, as the repo root is considered "infrastructure" and cannot be touched by anyone. So, we need to have configs in the subdirs, somehow. |
I think that figuring out how to have configs in the subdirectories is our main goal here. 👍
How is this initiated? Are people already in the package directory and then run Some possible hypothetical approaches going through my head (none of which have been thoroughly thought through):
(Of course, both flags would need to be represented in the Do either of those seem like they'd solve your use case? re: VS Code - this continues to be a problem, primarily because we don't maintain that extension and therefore have very little control over how it does what it does. We can request changes, but ultimately have zero control. This is something we need to dig into separately on a more holistic level. |
At the moment, they run
Yes and no; right now, we always invoke ESLint ourselves via the API, we can already provide it a The first listed idea is no different than passing in a cwd. The second is what I was suggesting always be on in my original idea, but doesn't handle the more general problem of inheriting parent configs being a challenge due to paths being rooted in the wrong place. But, to be clear, I'm not trying to frame this issue in terms of DefinitelyTyped only, but as a more general "flat config is hard in monorepo" issue. The only saving grace for us is that we programmatically invoke ESLint and don't yet support running ESLint on its own, except in the editor, which now works at least in VS Code because it reimplemented the config file search algorithm to know when to invoke ESLint more than once. So, I still don't have a good idea, because in addition to the cwd thing, making flat configs work nicely in monorepos when inhereting a parent config is challenging, because:
Not sure how to distinguish those, and I apologize for the lack of clarity. It's just a bit hard to figure out what the "right thing" is, compared to the strictly hierarchical old config system.
I'm not sure I follow this; no matter who implements an editor integration, the fact that |
See, I'm not sure that's true. So far, aside from DT, when maintainers have contacted us about monorepo setup, they almost always end up finding a way to make it work within the confines of flat config. That's part of the benefit of flat config -- you can define your own loading mechanism right in the config file. That's why we were able to externalize the eslintrc loading mechanism in I can see determining where to start the search as an issue, but as you mentioned, you can always set However, I'm unclear on how many monorepos prefer to lint the entire repo, in which case one config file at the root makes a lot of sense, as opposed to monorepos where they prefer to lint just an individual package. That's the first question. The second question is, for those who prefer to lint inside of individual packages only, do they use the cascade to apply shared configs? And is there any reason that the shared configuration can't be moved to a file that is just imported into the package-level config file? That said, it would help to have a concrete example of how you're currently using cascade in DT. Can you point to a package and the config files it relies on? |
An example I can think of is those using The other case is people opening subdirs, where the behavior should match if you opened the entire repo.
I haven't worked in a repo that didn't use cascades. TS itself isn't a monorepo per se, but did use nested cascading configs, which I had to undo for consistent behavior in my WIP flat config PR: microsoft/TypeScript#57684 But nobody else is chiming in here about the problems they're having, so it's not super easy for me to made wider claims.
Sure, for DT:
There's a root config, then packages or subdirs within those packages override the settings, but within JSON (statically analyzable using our bot infra without executing code). I have not yet figured out what we're going to do on DT at all for flat config on the loading front; the fact that the code is exectuable is a security and infra processing gotcha for us (we can't really statically analyze a flat config...), so it's not super clear what to do. It may be the case that we give up on letting anyone write |
Thanks, that's helpful. I think there are two things the TSC needs to consider:
It seems like in order to work for your setup, ESLint would need to implement both items 1 and 2 in the core and also allow JSON config files, that is undoing a lot of the simplicity we got with flat config. So yeah, you may need to come up with a custom solution for that. @eslint/eslint-tsc would like some other opinions here. |
ESLint version
v8.57.0
What problem do you want to solve?
When using a flat config, the CLI only ever loads one config; this is resolved depending on where you invoke it. In a monorepo, you may want to have multiple
eslint.config.js
files; this is common for non-flat configs, where child.eslintrc.json
files merge with their parents.It's a design choice that flat configs no longer merge with parent directories; this is fine, and it seems reasonable that one could write a config like:
However, this doesn't acutally work!
eslint .
at the root of the monorepo, eslint only loads the root config. All other configs are ignored, so any of the overrides done by other configs are not reflected.eslint .
inside of the nested dir, now the other config is loaded, so it sometimes works, depending on the current working directory.scripts/**
in the root will also ignorescript/**
in the local dir, even if the intent was that only the top level dir was ignored. Or, maybe a rule applied to a completely different location and shouldn't be included at all.The only way to do this is to forego multiple configs entirely. The root
eslint.config.js
could include the overrides for everything, or invent a custom config loading mechanism, like (in psuedocode):But, now you're loading more configs than may be needed for a given file. This is especially a problem for DefinitelyTyped; the 9244 package monorepo currently has 2247
.eslintrc.json
files:Normally, this would make me quite worried because if you run eslint in parallel per package (like DT does), that could mean
9,244 * 2,247 = 2,0771,268
config loads. But, under flat config, each config is a module, so the runtime should theoretically be reusing that module, so the reality would likely be closer to just "load 2000 JSON files once" so long as the config is not invalidated.That doesn't solve the problem of "what does this config file look like", however; not everything can be represented in JSON anymore with flat config.
What do you think is the correct solution?
ESLint could look for the closest
eslint.config.js
file. This is similar to old configs, but explicitly without the merging semantics. This would make ESLint working directory invariant; no matter where you run things, you get the same result.I understand that there is some concern about the performance of this, but I don't actually think it will cost too much; you only need to walk a given dir once and remember which config applied to it. All in all, that's no more accesses to the disk than you already need to actually be able to list the files you're going to lint.
However, this does not fix the other flaw; inheriting another directory's config does not imply any modifications of file globs. If I spread in a parent dir's eslint config, none of the paths will line up, and I have no solution to that.
Another thing I thought of was to have/suggest a layout like:
This would also be cwd-invariant, but is pretty complicated, and also does not solve the problem of re-rooting paths from other configs / removing ones that don't apply to the subdir.
Participation
Additional comments
I have yet to do perf testing on the "load 2247 config files" method or the "base config" idea; I've been really busy and have not had time to work on DT related stuff like this.
The text was updated successfully, but these errors were encountered: