Unable to customize certain rules via shareable config

[vadimdemedes]

Using latest xo (master branch), I'm unable to customize babel/object-curly-spacing rule via shareable config.

package.json:

{
  "extends": "my-config"
}

my-config:

{
  "rules": {
    "babel/object-curly-spacing": 0
  }
}

file.js:

{ key: 'value' }

Given these files, xo throws error:

If I override babel/object-curly-spacing in package.json, it works as expected and I no longer get errors:

{
  "rules": {
    "babel/object-curly-spacing": 0
  }
}

Related: https://github.com/sindresorhus/xo/blob/master/options-manager.js#L98

There is a $60.00 open bounty on this issue. Add more on Issuehunt.

• Checkout the Issuehunt explorer to discover more funded issues.
• Need some help from other developers? Add your repositories on Issuehunt to raise funds.

[sindresorhus]

Related: https://github.com/sindresorhus/xo/blob/2d26d79ea81f893fc7b1ff9e586c1e74ad28c830/options-manager.js#L98

Hmm. We just set the default value there. It's later extended by your custom config.

[vadimdemedes]

It's later extended by your custom config.

Seems like it does not for some reason. Needs investigation.

[kevva]

Those rules gets set in the root of the options sent in to eslint and will therefore take precedence over any configs. Configs will only override other configs (I think). The rules referenced in https://github.com/sindresorhus/xo/blob/2d26d79ea81f893fc7b1ff9e586c1e74ad28c830/options-manager.js#L98 are only overridable through setting rules in the package.json, or wherever your config lives.

[vadimdemedes]

@kevva is right.

Why does xo forces these default rules here (https://github.com/sindresorhus/xo/blob/2d26d79ea81f893fc7b1ff9e586c1e74ad28c830/options-manager.js#L98) ?

[jamestalmage]

What would be the consequence of moving those defaults into another base config that we conditionally add to extends, instead of force setting them like that?

[kevva]

I don't think there are any consequences other than creating a config for it may seem unnecessary but in this case it might be needed.

[vadimdemedes]

Why are those rules not in the default xo (or xo-react) config in the first place?

[sindresorhus]

Why does xo forces these default rules here (https://github.com/sindresorhus/xo/blob/2d26d79ea81f893fc7b1ff9e586c1e74ad28c830/options-manager.js#L98) ?

The Babel parser and plugin have some incompatible rules. So we first need to reset the original rules, then set the Babel ones. That line just sets it to the same value as in eslint-config-xo.

What would be the consequence of moving those defaults into another base config that we conditionally add to extends, instead of force setting them like that?

That would be the way to go. Anyone wanna do a PR?

Why are those rules not in the default xo (or xo-react) config in the first place?

They are, but here we do something different as we're using the esnext rules even though the esnext option is not set. We do this so esnext features will still work without the esnext option, just not linted as such.

[sindresorhus]

This should be fixed in cece3dd, but with ESLint changes and this, it would be nice if you all could take a look at the changes.

@vdemedes Can you try it out in practice?

[vadimdemedes]

Tried it out in a real app and now shareable configs don't work at all:

$ npm install eslint-config-vdemedes

package.json:

"xo": {
  "extends": ["vdemedes"]
}

output when running latest xo:

Error: Cannot find module 'eslint-config-vdemedes' from '/Users/vdemedes/Projects/xo/node_modules'
Referenced from:
    at Function.module.exports [as sync] (/Users/vdemedes/Projects/xo/node_modules/resolve/lib/sync.js:33:11)
    at resolve (/Users/vdemedes/Projects/xo/node_modules/eslint/lib/config/config-file.js:417:38)
    at load (/Users/vdemedes/Projects/xo/node_modules/eslint/lib/config/config-file.js:436:24)
    at /Users/vdemedes/Projects/xo/node_modules/eslint/lib/config/config-file.js:355:36
    at Array.reduceRight (native)
    at Object.applyExtends (/Users/vdemedes/Projects/xo/node_modules/eslint/lib/config/config-file.js:338:28)
    at loadConfig (/Users/vdemedes/Projects/xo/node_modules/eslint/lib/config.js:63:37)
    at new Config (/Users/vdemedes/Projects/xo/node_modules/eslint/lib/config.js:170:44)
    at CLIEngine.executeOnFiles (/Users/vdemedes/Projects/xo/node_modules/eslint/lib/cli-engine.js:492:28)
    at runEslint (/Users/vdemedes/Projects/xo/index.js:67:16)

Might be related: eslint/eslint#5411.

[vadimdemedes]

Added a PR to demonstrate this issue - #83.

[sindresorhus]

I ended up reverting to the previous workaround. Don't have time to fix this properly right now. New version is out btw. I still would like to see this fixed at some point.

[fregante]

I ended up reverting to the previous workaround

Are you saying that there is another way to have different configs for subfolders?

[sindresorhus]

@bfred-it I'm not sure what you're asking? Doesn't seem relevant to this thread.

[fregante]

Sorry, I was referring to the overrides property. From what I understand it was disabled and it was dependent on this issue (as you said here #78 (comment))

I thought you were referring to overrides in "I ended up reverting to the previous workaround (for overrides?)"

[sindresorhus]

@bfred-it I don't remember, but it ended up landing regardless: https://github.com/sindresorhus/xo#config-overrides

[IssueHuntBot]

@IssueHunt has funded $60.00 to this issue.

---

• Submit pull request via IssueHunt to receive this reward.
• Want to contribute? Chip in to this issue via IssueHunt.
• Checkout the IssueHunt Issue Explorer to see more funded issues.
• Need help from developers? Add your repository on IssueHunt to raise funds.