Skip to content
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

--prettier overrides {"prettier/prettier": "off"} in rules #512

Closed
jonahsnider opened this issue Dec 25, 2020 · 8 comments
Closed

--prettier overrides {"prettier/prettier": "off"} in rules #512

jonahsnider opened this issue Dec 25, 2020 · 8 comments

Comments

@jonahsnider
Copy link
Contributor

"xo": {
	"prettier": true,
	"rules": {
		"prettier/prettier": "off"
	}
}

This should turn off the prettier/prettier rule while still enabling Prettier mode in XO, but instead the rule remains enabled.

@jonahsnider jonahsnider changed the title --prettier overrides "prettier/prettier": "off" in rules --prettier overrides {"prettier/prettier": "off"} in rules Dec 25, 2020
@sindresorhus
Copy link
Member

What's the purpose of turning off prettier/prettier while leaving the prettier option to true? I don't personally use Prettier or the prettier option.

@jonahsnider
Copy link
Contributor Author

I use the Prettier CLI to check all the files Prettier supports in my project. I don't want both the check-format and lint jobs in CI to fail.

@fregante
Copy link
Member

fregante commented Aug 16, 2021

I don't get it, that doesn't seem to answer Sindre's question. What's the point of setting it to true if you disable it afterwards? What's even the intended outcome for XO?

In projects where I have both the Prettier CLI and XO, I just use prettier/prettier: "off" without needing to enable it in XO itself.

@jonahsnider
Copy link
Contributor Author

There is more to the Prettier integration than just the prettier/prettier rule. When possible, I want to use Prettier exclusively to check code style and to avoid both the Prettier CLI and XO erroring from a formatting error.

@fregante
Copy link
Member

There is more to the Prettier integration than just the prettier/prettier rule.

What would that be?

What's even the intended outcome for XO?

@jonahsnider
Copy link
Contributor Author

Using eslint-config-prettier to disable other formatting issues that should be handled by Prettier.

@fregante
Copy link
Member

fregante commented Aug 6, 2022

xo/lib/options-manager.js

Lines 412 to 421 in 3a4c9c9

if (options.prettier) {
// The prettier plugin uses Prettier to format the code with `--fix`
config.baseConfig.plugins.push('prettier');
// The prettier plugin overrides ESLint stylistic rules that are handled by Prettier
config.baseConfig.extends.push('plugin:prettier/recommended');
// The `prettier/prettier` rule reports errors if the code is not formatted in accordance to Prettier
config.baseConfig.rules['prettier/prettier'] = ['error', mergeWithPrettierConfig(options, prettierConfig)];
}

If you're going to set --prettier and prettier/prettier: "off" in your config, you might as well just set prettier/recommended once instead, it's less config and your intent is much clearer:

{
	"extends": "plugin:prettier/recommended"
}

Asking XO to first run --prettier and then apparently disabling it later looks super confusing.

Feel free to reopen if this is considered a bug though.

@fregante fregante closed this as not planned Won't fix, can't repro, duplicate, stale Aug 6, 2022
@lachieh
Copy link

lachieh commented Mar 21, 2024

I wouldn't consider this a bug, but I'd love a way to disable conflicting prettier rules (which is the role of eslint-config-prettier) but not surface the warnings or run prettier --fix through eslint/xo (which is the role of eslint-plugin-prettier).

Personally, I find it much nicer to just let prettier format on save silently than showing an error/warning in the IDE. Formatting should be just handled but surfacing a warning means I have to pay attention to something.

I've tried to solve this by just enabling the eslint-config-prettier rule:

{
    "extends": "prettier"
}

However, because the typescript rules are applied after, this causes fights between @typescript-eslint/indent because it is applied over the top of the user config:

xo/lib/options-manager.js

Lines 356 to 359 in 1245088

if (options.space && !options.prettier) {
if (options.ts) {
config.baseConfig.rules['@typescript-eslint/indent'] = ['error', spaces, {SwitchCase: 1}];
} else {

I'm happy to file this as a separate issue, but figured the original discussion was relevant. I'm also happy to contribute a PR once the direction is nailed down.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants