regression: `xo: false` in nested `package.json` is not supported any more

Question

regression: `xo: false` in nested `package.json` is not supported any more

JounQin opened this issue 6 months ago · 9 comments

xo: false means ignoring the files under the package.json file previously, but it stops working in recent versions.

Answer 1 · 2023-11-20T21:11:37.000Z

Could you describe the scenario better? I don't think this fix is what we want to do.

I take it that you are in a mono-repo, you have a xo installed in a nested package, and your xo config is at the root level in your package.json.

.
├── packages
   └── some-package
         |------- package.json (xo installed here, but config is 2 dirs up)
|- package.json (xo config here)

You should instead do something like
from your nested package folder:
npx xo --cwd ../..

This will fix your problem I think, it's a common trip up for people with mono-repos.

Answer 2 · 2023-11-21T01:57:14.000Z

npx xo --cwd ../..

It seems too verbose to me personally, and I didn't know it would work. Why not fix like this?

Answer 3 · 2023-11-21T02:47:11.000Z

OK, I found out that it's actually specifically disabled at https://github.com/rehypejs/rehype-github/blob/7d7e851b751413c1ec95e3fbdf1c48e4b71155e5/packages/break/package.json#L37, however in this case, xo should just ignore linting actually or change to use the root config like the current fix.

I would prefer not linting at all because xo is specifically disabled.

Answer 4 · 2023-11-21T13:04:32.000Z

Because previously xo used xo: false for nested packages, to continue looking for a parent package.json.
Omitting xo: false, previously, resulted in the defaults being applied.
Which is exactly what xo is now doing when xo: false is used.

from @wooorm at rehypejs/rehype-github#2 (comment)

I think I'll change the fix by check whether xo is false instead. Do you think this is a regression? @spence-s

Answer 5 · 2023-11-21T15:37:01.000Z

At some point, years ago, xo would search all the way up to the home directory for a "global" xo config. But this in itself caused lots of confusion and bugs for many users and was hard to integrate with eslint properly. (ie, couldn't lookup plugins and configs correctly for configs outside of the repo)...

This behavior was specifically removed in favor of requiring configs to live in the repos, so this is not a bug nor a regression, but is working as intended and stopping search at cwd as intended.

Does it still limit people with mono-repos? Yes it does limit them a bit and they may have to "hack" cwd like I showed above.

Can you still use xo with mono repos using the recommended approach? absolutely, xo works just fine in mono-repos if set up correctly.

Could we improve this or accept functionality that improves this UX? That would be up to @sindresorhus, but probably would be willing to change things slightly if there was a good argument for it.

However, it looks to me that xo is just being used in a mono repo set up here in a way that doesn't play nicely with its design. I would recommend refactoring the original repo per recommended mono repo setup.

TL;DR: not a bug nor regression, nor do the PRs that keep searching up something we want to add at this time. If/when we move to the the new config style, we may reconsider if that makes solving resolution bugs simpler.

Answer 6 · 2023-11-21T15:56:31.000Z

OK, then it could be feature request then? xo: false to disable xo's functionality for nested package.json.

Answer 7 · 2023-11-21T16:05:47.000Z

I would personally not support this behavior change.

You can already get the behavior you want from the way it is right now.

A pr for this would need to be comprehensive and have a lot of tests to account for resolving plugins and configs potentially outside of the repo. So maybe it would be considered if you decided to take on this relatively large amount of work.

Note: if you have an xo field - even if it is "false" in the package.json, search will prematurely stop there. You need to remove the xo field entirely from package.json to have it continue searching up to the cwd you set.

Answer 8 · 2023-11-21T16:21:02.000Z

xo: false was recommended before. I don’t think this feature was removed that many years ago (e0f81a7).
I’m pretty sure we’ve used it in many places, since it was added 7 years ago, up until maybe a year ago? 50 or so?

Anyway, I’m fine not supporting this.

Answer 9 · 2023-11-21T16:37:16.000Z

The PR you linked was from 3.5 years ago and it does look like the change happened there.

It also includes the documentation to simply omit the "xo" field in order to continue searching up to the cwd, changed from the previously recommended xo: false

I would support this change to respect xo: false as long as it definitely stops searching at the cwd (like it does now), which would be a very minor behavior change.

However, its probably easier to just remove the xo: false fields from your package.jsons

as with all changes the final say is up to @sindresorhus not me.