`prettier: true`, but no spaces inside braces: `{Foo}`
devinrhode2 opened this issue · 11 comments
I have prettier: true in my xo config, but, I'm seeing this formatting:
import {Foo} from './foo'
This is fundamentally the "object-curly-spacing" style rule from eslint (or @typescript-eslint/object-curly-spacing).
I see this is set inside eslint-config-xo and eslint-config-xo-typescript, to ["error", "never"] - "never" meaning we never want spacing around Foo.
I confirmed this by setting prettier: false inside my xo config, and inside vscode, I see a warning regarding this style rule.
As a sanity check, I did run prettier directly:
yarn prettier --write /full/path/to/file.ts
And it restores spaces around Foo, resulting in:
import { Foo } from './foo'
I tried running yarn prettier --write /full/path/to/file.ts with multiple different prettier versions, 2.2.1, 2.3.2, 2.4.1, all of them restore spaces around Foo.
I tried modifying the rules inside of my xo config, setting them to ['error', 'always'] - which in eslint world, would restore the spaces around Foo - but - this was causing chaos when saving code inside vscode. Creating double spaces, trading one formatting error for a different type of formatting error.
Q: Does the vscode extension support codeActionsOnSave?
=====
My general thinking is that we should address this at an architectural level, so that any formatting inconsistencies are not possible. I think that prettier should run after eslint, and eslint should ignore all formatting issues.
However, I would like to avoid writing eslint fixed code and then telling prettier to read from disk. Passing an ast directly to prettier would be awesome, but would be quite the undertaking.
The biggest overhead with eslint-plugin-prettier I'm guessing is probably the generateDifferences call.
I will check what the prettier.format( call is returning today
@fisker I noticed you have some PRs for eslint-plugin-prettier, which fix the arrow-body-style issues
Any thoughts here? My interest is in improving performance, but main issue here is the inconsistent formatting
If prettier is just a second final step, --fix simply needs to pass final source to prettier before writing to disk.
Likewise, xo --check needs to call prettier --check
@devinrhode2 xo is only a wrapper around eslint - prettier is only ran via eslint-plugin-prettier from xo. You can configure prettier by adding a prettier field to your package.json or a .prettierrc file, you probably get this. Yes, I don't think xo follows the same defaults as prettier itself, but xo's default prettier config does NOT conflict with the xo eslint rules. - it is kind of confusing.
Best idea is to let xo manage both formatting and linting for you completely. Or turn prettier off in xo and configure prettier to manage formatting (as long as you have not set up conflicting rules)
For VSCode - you should turn the prettier plugin off for typescript and javascript if you are using the xo plugin because they will conflict. The xo plugin supports formatting on save and can be set up correctly as follows:
"typescript.format.enable": false, // turn off vscode defaults
"javascript.validate.enable": false,
"editor.formatOnSave": true, // turn on formatting on save
"[javascript]": {
// only ever set editor.defaultFormatter under a specific language
"editor.defaultFormatter": "samverschueren.linter-xo"
},
"[typescript]": {
"editor.defaultFormatter": "samverschueren.linter-xo"
},I personally set up all my VSCode linting and formatting on a per language basis as shown above.
Regarding performance in VSCode - I have some works in progress on the plugin and some updates and improvements will be coming for that soon, I just haven't quite had the time to finalize my WIP.
Inside of eslint-plugin-prettier, I added log statements around the prettier.format( call:
console.log('input', source);
console.log('opts', prettierOptions);
console.log('prettier version:', prettier.version);
prettierSource = prettier.format(source, prettierOptions);
console.log('output', prettierSource);
I'm actually noticing that eslint-plugin-prettier is getting called twice for one yarn xo --fix /path/to/file.ts
If it's given import { isDev} from './isDev' then it'll format it to import {isDev} from './isDev'
Happening with:
eslint-plugin-prettier v3.4.1, xo v0.43.0 (with some minor patching), prettier v2.3.2
eslint-plugin-prettier v4.0.0, xo v0.45.0 (with some minor patching), prettier v2.3.2
eslint-plugin-prettier v4.0.0, xo v0.45.0 (with some minor patching), prettier v2.4.1
(Inside of eslint-plugin-prettier, I'm doing console.log('prettier version:', prettier.version) - so there's no question as to what prettier version is running)
(yarn v1.22.15, node 16.10.0)
(I'm hoping calling eslint-plugin-prettier twice may be intentional, due to some bugs around arrow functions, or eslint-disable~line/next-line comments)
Also, performance inside of vscode has been great, no issues there. It's command line that is slow for me. Maybe, we don't need a pre-commit hook, but we'd still need to run xo --check inside our CI process to check for errors.
I'm am thinking that inside of xo we'd somehow directly use ./node_modules/.bin/prettier instead of require('prettier').format. I believe this would solve the problem.
Alternatively, eslint-plugin-prettier could directly use .bin/prettier. (Going to explore their issues/discuss over there some too: prettier/eslint-plugin-prettier#436)
Wow, dreams come true, prettier actually has an option bracketSpacing. XO seems to be changing the default value for that. So it's just a matter of getting the right config value passed to prettier!
FTR prettier was being provided these options:
{
// These seem to be reflection of my xo config (semi: false, space: 2)
useTabs: false,
tabWidth: 2,
// These appear to be my options:
semi: false,
singleQuote: true,
printWidth: 80,
jsxSingleQuote: true,
arrowParens: 'avoid',
trailingComma: 'es5',
quoteProps: 'consistent',
// THIS is causing my issue:
bracketSpacing: false,
bracketSameLine: false,
}
In the readme, I read this to mean that if I have a prettier config file, that is the prettier config to be used:
The Prettier options will be read from the Prettier config and if not set will be determined as follow:
Either it should say is something like:
The Prettier options will be read from the Prettier config then combined with these XO defaults:
...
If you don't like the XO defaults, you should set them to your own preferred value inside your prettier config
Alternatively - we could simply NOT set these extra prettier defaults.
I think most people using prettier will have their own prettier config. The useTabs, tabWidth both make sense for XO to be setting based on the semi, space XO config options. Other options, appear to be introduced with the original prettier PR here: #271