New: Add resolveRelativeToConfigFile setting (fixes #3458)

[octogonz]

What is the purpose of this pull request? (put an "X" next to item)

[ ] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofixing to a rule
[ ] Add a CLI option
[x] Add something to the core
[ ] Other, please explain:

What changes did you make? (Give an overview)

This PR fixes #3458. It introduces a new setting resolveRelativeToConfigFile for .eslintrc.js. The setting is an optional boolean value that defaults to false. By default, ESLint behaves the old way, where plugins always get loaded from one centralized folder.

Whereas if resolveRelativeToConfigFile is true, then ESLint uses the modern convention of resolving plugins relative to the config file that imports them. This enables a shared ESLint config package to supply its own dependencies, rather than imposing that responsibility on every consumer of the package. See #3458 (comment) for a complete explanation of this scenario.

Is there anything you'd like reviewers to focus on?

I struggled a bit with defining the setting, because ESLint doesn't inherit config settings until _finalizeConfigArray(), which doesn't happen until after all config files and plugins have been loaded. I worked around this by using internalSlotsMap to store the state. It means that if we encounter a config file with resolveRelativeToConfigFile=true, then the new behavior is enabled for all subsequent config files. Although not ideal, this approach should work just fine for the expected usage of this feature. I'm open to other approaches, though.

[jsf-clabot]

Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

[kaicataldo]

Thanks for contributing! We ask that all changes to core go through the RFC process outlined here. That being said, it looks like there might already be some RFCs in progress that address this problem. Would you mind looking at eslint/rfcs#14? We'd love your input!

[octogonz]

@kaicataldo thanks for taking a look at this!

From what I can tell, RFC 14 was opened in February and hasn't had any activity since August. (?) It is now November. One gets the impression that RFC 14 is proposing a major design change to modernize the way ESLint resolves modules. That's good!

My PR has much more humble aspirations: In #3458 (comment), I shared a monkey patch that modifies just one line of config-array-factory.js. This fixes the issue for many scenarios that will realistically arise in practice. A number of people tested the patch and reported that it worked for them, including a few somewhat complex scenarios involving non-standard package managers.

But it's not wise or safe to be monkey-patching the ESLint binary. I wouldn't want to support that in a production repo at work. Instead, this PR an adds a built-in config for ESLint to "opt in" to the one-line-fix.

My solution isn't perfect. It doesn't consider all the nuances discussed in RFC 14. Whenever an official solution is finally released, the resolveRelativeToConfigFile option can be deprecated and removed.

But the hope here is to provide a stopgap measure, so people don't have to wait LITERALLY YEARS for a grand vision to finally get sorted out. (This isn't hyperbole -- issue #3548 has been open for 4 years!) Seems that it's still a ways off from getting implemented.

If you believe that the RFC process is the right way to wrap up my PR, cool, I'm happy to follow the steps. But can we get some assurance of a quick turnaround? For example, what's the typical lifetime of an ESLint RFC? At the point where this is no longer a "quick fix", the point of the PR would be lost.

Thanks!

[yordis]

Even when there is a lot of value in the RFC, it got nowhere without ownership nor attention.

It had been 4 years.

I much rather support this PR that going for the RFC route that proved that it doesn't work, again, 4 years folks, it is not fair for some of us that some Core maintainers have such strong personal opinion holding everyone back even when a solution meet in the middle.

I would love it if I don't have to have 8 devDeps simply because of this issue, I much rather deal with the side consequences of this PR as I had been doing for few months now after the hack (works without any issues what so ever).

[mysticatea]

Please follow our RFC process to add a new feature.

This proposal is problematic (see also https://gist.github.com/not-an-aardvark/169bede8072c31a500e018ed7d6a8915). I don't think this direction is our future, and as consider that npm has planned to revive the auto install of peerDependencies, I don't think that we need to do something around this.

[yordis]

npm has planned to revive the auto install of peerDependencies

Had been 2 years without using npm, does this mean that you will force me to use npm or depends on yarn adding the feature or not?

[mysticatea]

The npm's behavior change will affect other compatible tools because otherwise loose compatibility.

[arcanis]

Yarn doesn't plan on implementing this change, as has been made abundantly clear on the npm rfc. I'd recommend to aim for the strictest rules, not the goofiest.

[siric]

This proposal is problematic (see also https://gist.github.com/not-an-aardvark/169bede8072c31a500e018ed7d6a8915).

This PR merely introduces an opt-in setting which improves current behavior; behavior which has been discussed for the past 4 years in multiple RFCs and countless of issues, without any sight of a solution. At least allow the people accepting the drawbacks of this fix to decide for themselves whether it is worth opting in.

npm has planned to revive the auto install of peerDependencies

Since Yarn will not be auto-installing peerdeps as mentioned by arcanis, this is not something ESLint can rely on, as a proper solution to support the Yarn userbase would still be required. This PR seems like great middleground for the in-between time.

[yordis]

Here we go again, another thread, once again, where we say the same back and forward ...

I much rather support this PR that going for the RFC route that proved that it doesn't work, again, 4 years folks, it is not fair for some of us that some Core maintainers have such strong personal opinion holding everyone back even when a solution meet in the middle.

I hear you folks (Core team), and I love you and respect your work, but at this point only you are the once that is holding everyone back, regardless that many people (included me) had been using this feature for months with more than, in an Org with 300+ active engineers without any issue (this is not counting MS folks) 😉

Don't take it personally, it is the reality.

Many folks had tried to help you and we never get anywhere.

I will stop now, I think my words are not valuable at all, at this point, I am burned out.

https://en.wikipedia.org/wiki/Perfect_is_the_enemy_of_good

[mysticatea]

There are multiple imperfect solutions around this:

1. Using dependencies of shareable configs to install plugins then the package manager hoists the plugins. This is simple and just work in most cases. On the other hand, the confliction of plugin versions may cause trouble. But if you use peerDependencies along with the dependencies, the package manager will warn the installation problem if found.
2. Using plugin configs and rules. Exporting the rules of other plugins solves both of the rule's identifiability problem and the installation problem. On the other hand, we cannot use this solution with shareable configs.
3. Using the postinstall hook of shareable configs to install plugins. On the other hand, it seems to have remaining problems.
4. Using peerDependencies and npm 7 (not available yet).

I'm not sure that modifying core to add fifth imperfect solutions makes sense. This proposal has the identifiability problem of rules and it doesn't notify any warnings/errors even if the problem happened. Silent broken is bad. Anyway, please don't try to skip the RFC process.

[kaicataldo]

If you believe that the RFC process is the right way to wrap up my PR, cool, I'm happy to follow the steps. But can we get some assurance of a quick turnaround? For example, what's the typical lifetime of an ESLint RFC? At the point where this is no longer a "quick fix", the point of the PR would be lost.

@octogonz You can find more details about ESLint's RFC process here. Regarding turnaround time, please keep in mind that, while the team does its best to respond to issues in an expedient way, the team is mostly comprised of volunteers who work on this project in their spare time.

[octogonz]

There are multiple imperfect solutions around this:

1. Using dependencies of shareable configs to install plugins then the package manager hoists the plugins.

This "hoisting" behavior is only implemented by NPM and Yarn classic. It is a special consequence of NPM's unusual installation algorithm. That design works okay for smaller projects, but when you need to support side-by-side versions in complex installations, it causes serious problems. There's no straightforward way to solve those problems without redesigning NPM's algorithm. Without getting into details, it's enough to say that this problem was so significant that the two competing package managers made huge investments in replacing it: PNPM (by using symlinks) and Yarn Plug'n'Play (by changing NodeJS itself). Both are widely used today.

2. Using plugin configs and rules. Exporting the rules of other plugins solves both of the rule's identifiability problem and the installation problem. On the other hand, we cannot use this solution with shareable configs.

Hmm... I wasn't aware of this option. Lemme go read about it. But it sounds like you're saying it won't address our need for shareable configs. (?)

3. Using the postinstall hook of shareable configs to install plugins. On the other hand, it seems to have remaining problems.

😄 C'mon.

4. Using peerDependencies and npm 7 (not available yet).

NPM's proposal to auto-fix broken peer dependencies is controversial. People don't believe it can work correctly for complex installations. The discussion was locked without any consensus among the other package managers. Not sure I'd bet on a feature that may only be compatible with NPM 7.

I'm not sure that modifying core to add fifth imperfect solutions makes sense. This proposal has the identifiability problem of rules and it doesn't notify any warnings/errors even if the problem happened. Silent broken is bad. Anyway, please don't try to skip the RFC process.

Hmm... but how bad, in practice? This PR doesn't claim to be an ideal 100% correct solution. I only claimed that it works fine for the most common cases. And it's simple: We don't necessarily need to spend months debating the design of a one-line logic change.

If ESLint's RFC process will give us a proper solution by end of year, I'll eagerly abandon my PR. Otherwise, why are you opposed to a small off-by-default hack that unblocks your users? We can call it "experimental" and "unsupported" -- nobody would complain about that. People just want some way to make a shareable config while they wait for ESLint 7 or 8 or whenever the properly designed feature will finally arrive.

[mysticatea]

If ESLint's RFC process will give us a proper solution by end of year, I'll eagerly abandon my PR. Otherwise, why are you opposed to a small off-by-default hack that unblocks your users?

I think that you have a misunderstanding. Anybody, including you, can send RFC. And the change of core functionality requires the RFC process to discuss design before imprementing.

[yordis]

I think that you have a misunderstanding. Anybody, including you, can send RFC.

Are you responding with "Follow the RFC" for everything we say? Are you listening to him?

And the change of core functionality requires the RFC process to discuss design before implementing it.

Four years waiting for the "RFC process," four years, at this point, we are all frustrated by it, and I feel offended that you are not even listening to what people are saying; it is rude.

Everyone gets it, we need RFC, but your RFC failed, failed for the last four years on the same topic.

Like I said before, "Perfect is the enemy of Good."

The perfect solution where trade-offs don't exist is naive; the whole point of software development is to make trade-offs on what is the best solution and what we have in hand; otherwise, we will end up waiting four years without any resolution.

People that opt-in using this flag must understand the feature

Nobody is asking changing the core by default for everyone.

Still, for those who want to read the documentation and understand the technical limitations and trade-offs, this is a viable solution.

That is the whole point of adding a flag.

[octogonz]

In the meantime, here's a small RFC: eslint/rfcs#46

I'm not excited about arbitrarily delaying my small fix by a minimum of 28 days, but if that's how ESLint wants to do this, we should go with it.

[kaicataldo]

For what it's worth, the RFC process was established about a year ago.

[mysticatea]

@yordis You have misunderstood something if you aren't a time traveler. "Four years waiting for the RFC process" is definitely false because the RFC process has been introduced Nov 2018. We don't live in 2022 yet. The purpose that we have introduced the RFC process is to advance long-standing issues that are due to difficult of design, such as #3458.

[yordis]

Exactly, I am talking about the feature that the person is trying to fix at the moment, If that makes you happy, 2 years of RFC, 4 years on waiting for this to be resolved.

I digress, we are not getting anything out of the conversation at this point, nor talking to you will resolve anything at the moment.

The RFC is open, the PR is opened, that is good enough.

[timmmichaud]

I have a team of 50 engineerings working on over a dozen of apps. I've been holding off on creating a shared eslint config package because of the current situation that has been going on since Aug 20, 2015 in 3458. It's discouraging to see.

I fully accept the experimental status of this opt-in feature and I can't hardly wait for this PR to be merged so I can finalize our organization's shared eslint config package.

Thank you @octogonz for your perseverance

[pantoninho]

I'm also having some trouble setting up shared eslint config across packages due to the plugin resolution system.

It would be great to have this option while eslint/rfcs#14 and eslint/rfcs#9 do not get implemented/merged (which I guess it might take a while..)

Thank you for your hard work on this @octogonz.

[octogonz]

We've decided to abandon this PR. The maintainers weren't able to accept the fix without reworking it into a more complete design, whereas the intent of this approach was to provide a quick opt-in workaround.

Instead, we've released our patch as a standalone NPM package: @rushstack/eslint-patch

I've updated the implementation to work with both ESLint 6.x and 7.x, and we'll maintain it until whenever ESLint can provide an official solution. Thanks!