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
[RFC] Editor Integration #918
Comments
Wow, great work writing all of this up.
This is something a number of users are requesting on prettier-atom. We haven't implemented yet, but what you're describing sounds about right to me.
I believe @kentcdodds solved this issue in a similar case where he needed to find the ESlint configuration for prettier-eslint. He solved it by optionally passing the filepath of the file as an option. Then the tool knows where to start recursively checking from.
I vote for this also.
This is definitely something we will need to watch out for.
Have we determined that performance is a problem? The |
Great way to do that is with find-up |
There's also read-pkg-up. |
JetBrains IDE (IntelliJ, WebStorm etc) issue: https://youtrack.jetbrains.com/issue/WEB-25077 |
One thing I added o esformatter was the concept of a This is basically how I decided to implement it: millermedeiros/esformatter#76 (comment) - cli options always take precedence (assumes user is overriding the defaults on purpose) The whole user config logic is handled by the The package version logic I do inside the
|
Most JS I write does not live in an npm package, so this would do nothing for me. I know that's kind of unusual, but I thought it worth mentioning - package.json is an npm thing, not a JS thing. |
Instead of implementing a JSON RPC protocol through stdin/stdout you could also just create a programmatic API like the one eslint has. var eslint = require('eslint')
eslint.lintText(text, options, callback); Both eslint and standard-engine-based linters implement similar APIs. I wrote a plugin for integrating standard-engine linters into Atom which use this API to implement a worker to avoid the node.js startup cost on each lint. It allows for the same separation as the stdin/stdout protocol, but is more flexible in my opinion, and probably a bit easier to do as well :-) It also won't rule out the stdin/stdout based solution, but rather be the first step to implement it. |
@gustavnikolaj that's only useful for editors written in JS (so atom and code?), not for emacs, vim, intellij, webstorm, sublime etc. And prettier already has a programatic API, so those integrations can be done today |
@SimenB Oh sorry, I should have checked whether it was already there. |
We also need it for atom. It has chrome flags enabled that prevent doing eval(), so you can't just require a js file found in package.json. What the current atom editor plugin does is to use a specific version, which isn't ideal since you may have multiple projects using different versions. So the workaround is to shell out and use stdio |
@vjeux |
Yeah, most atom packages that need to do this use |
@robwise I've bumped into performance issues stacking prettier with eslint --fix, so today I'm just using eslint. While my mac holds up well and I don't really "feel" sluggish while typing in Vim (as I'm sure no one does with current macs) -- CPU does spike, the laptop heats up, and battery drains much faster. Today, I'm using neoformat/ALE with Vim using I think using one |
@jondot When you say you've run into performance issues stacking prettier with eslint, have you tried https://github.com/prettier/prettier-eslint? We have this integrated inside of https://github.com/jlongster/prettier-atom. |
Yes, I've tried I gave the whole thing another try after reading the thread here. The deal is that with Vim, the integration is running a binary, providing input and taking the output via stdin/stdout. However you look at it - we still incur the startup time and the big CPU spike of spinning up With I spent a bit of time trying to trick prettier-eslint to use Would be happy to hear about any other altenative Vim configuration that gives out quick lint+fix results. |
I'm sure it's been discussed, but what about a Here is an example config --no-semi
--single-quote
--trailing-comma
es5
--write
"app/**/*.js" I'm sure this has been discussed before but figured I'd throw the idea out here again. Looks like this is being discussed over in #98 (see here and beyond) A
|
`prettier_d` is like [eslint_d], but it runs `prettier` instead of `eslint`. This eliminates the Node.js startup delay from all but the first run of `prettier_d`, making it a snappier experience. [eslint_d]: https://github.com/mantoni/eslint_d.js Related discussion: * prettier#575 (comment) * prettier#918 (comment) * prettier#753 (comment)
`prettier_d` is like [eslint_d], but it runs `prettier` instead of `eslint`. This eliminates the Node.js startup delay from all but the first run of `prettier_d`, making it a snappier experience. [eslint_d]: https://github.com/mantoni/eslint_d.js Related discussion: * #575 (comment) * #918 (comment) * #753 (comment)
I'm working on a very similar project: prettier_d, which just runs prettier as a server (but without eslint).
For linting, I'm trying out eslint-config-standard and eslint-config-prettier (details here). Not sure if it alone is faster than anything you've tried, but you might be able to use it in concert with eslint_d to speed things up. For continuous formatting inside the editor, I'm experimenting with using a local .vimrc file with some autocmds to format using prettier_d (details here). |
@josephfrazier have you seen https://github.com/sbdchd/neoformat ? Also, it's certainly slower than prettier_d would be, but I'm currently using eslint_d with https://github.com/not-an-aardvark/eslint-plugin-prettier which seems to work well now that prettier has no-semi |
I want to introduce prettier to my colleagues in the office, but it will be hard without config. Because i cant control my colleagues' editor' plugins' configurations. Config support will help me to maintain config per each project and let prettier-cli and relevant editors' plugins to get configuration from there. Another usecase is to introduce prettier to my OSS projects, where contributors use all kind of editors, and i need a way to say to those editors' plugins which options of prettier i do use. |
Third, usecase i have different es5 and es6+ projects, and i want to use the same tooling (cli and pluins), so i can't use |
I think it's pretty clear prettier should support some sort of local/global config file. I've created a simple little bash script that implements it that I call #!/usr/bin/env bash
if [ -f prettier.opts ]; then
prettier --stdin $(cat prettier.opts)
elif [ -f ~/prettier.opts ]; then
prettier --stdin $(cat ~/prettier.opts)
else
prettier --stdin
fi Now just make it executable ( " Use prettier with custom config
autocmd FileType javascript set formatprg=prettier-with-options
" Shared neoformat config
autocmd BufWritePre *.js Neoformat
let g:neoformat_try_formatprg = 1
let g:neoformat_only_msg_on_error = 1 This will always run prettier on any JS code so be aware of that. It would require a bit more work to only turn on projects that have a |
There are two important considerations that aren't being discussed that I want to ensure get considered as part of this proposal:
For those reasons (plus the ones already mentioned) I'd petition to have configuration in a dedicated file rather than inside the I believe the problem of finding and caching the appropriate config is a problem that's already been solved by projects like |
In all seriousness, a |
Any other tool in this category supports having a config file. There's probably half a dozen issues opened in this regard. Correct me if I'm wrong but usually you would have any of the following files or use the default settings:
Command line options are still nice for manually running a one of. Those would obviously override any configuration file. Also as mentioned above, please make this available to projects that are not npm modules, event though most might be. But a dedicated config file is easier to copy to new projects and makes it more obvious where to look. Need some examples? In my current project I see Please let's keep this discussion going. This is a key feature imho |
This uses [config-attendant] instead of [pkg-conf], in order to look for `.prettierrc` as well as `package.json` (see prettier/prettier#918 (comment)). I would have just used [rc], but it doesn't support reading package.json files (see dominictarr/rc#64). [config-attendant]: https://github.com/GarthDB/config-attendant [pkg-conf]: https://www.npmjs.com/package/pkg-conf [rc]: https://www.npmjs.com/package/rc --- Also, add documentation for `--pkg-conf` in general.
For anyone who wants to use |
@josephfrazier very nice! i think it'll alleviate some pains around Vim. Still, to update on my set up: there's still no polite way to use prettier on Vim (without thrashing CPU and lagging formats). Looks like I can't use eslint for linting such as unused variables, etc., and have prettier format code at the same time. So if I use |
If you look at the ESLint configuration for prettier_d itself, I think I've gotten this working as you described. Feel free to open an issue at |
@vjeux how about the following as a proposal?: Command line looks for If none of the above match, prettier falls back to defaults. The prettier # looks for -c/--config or ./prettier.opts or ~/prettier.opts or falls back to defaults
prettier -c prettier.custom.opts # -c/--config: pass in custom "prettier.opts" file path If this is acceptable, is a PR something that would be desired? NOTE: If it is preferable for some reason to use a |
@danawoodman would love to see exactly that! |
Been mentioned already but I would push for using something such as cosmiconfig which implements the config formats commonly seen across the ecosystem. Imo it gives the most predictable way for writing and consuming configuration for tools in the js world. From the README:
|
Is there anything we can do to help move configuration/editor support along? We seem to be getting in a bit of a muddle moving around projects with tools using different prettier versions (global vs project vs tool/editor vs ci) and it's causing us some degree of pain. |
I reckon we should be able to specify the prettier Ie.
|
I think it is fairly important to be able to flag subdirectories with a new prettier config/overrides because not all JS features are supported per directory, specifically IMO don't reinvent the wheel...copy how |
Fyi, node 8 that was just released supports trailing comma!! |
@vjeux Yeah it will be nice when it is LTS in October! nodejs/LTS#lts-schedule has Node 4 supported until April 2018 and Node 6 until April 2019. Because Node's LTS, I don't think Prettier should adopt a configuration style that makes it more difficult to use until April 2019. I think the configuration decisions should follow the Node LTS schedule. Here is my current configuration... .cache
.idea
.vscode
node_modules
build
static
flow-typed
flow
npm-packages-offline-cache
tools
"prettier": "find . -name \"*.js\" | grep -v -f .prettierignore | xargs prettier --tab-width 4 --single-quote --trailing-comma all --write",
"prettier:tools": "find ./tools -name \"*.js\" | xargs prettier --tab-width 4 --single-quote --trailing-comma es5 --write",
"precommit": "lint-staged"
},
"lint-staged": {
"./*.js": [
"prettier --tab-width 4 --single-quote --trailing-comma es5 --write",
"git add"
],
"tools/**/*.js": [
"prettier --tab-width 4 --single-quote --trailing-comma es5 --write",
"git add"
],
"src/**/*.js": [
"prettier --tab-width 4 --single-quote --trailing-comma all --write",
"git add"
],
"server/**/*.js": [
"prettier --tab-width 4 --single-quote --trailing-comma all --write",
"git add"
]
}, Because of the "no configuration" philosophy, there ends up being more hacks/workarounds getting If anyone has any suggestion on how to better handle this I'd love to hear how. |
@chrisblossom, since you're already using eslint, you could use eslint-plugin-prettier and eslint-config-prettier, add your prettier configuration to |
I'm a pretty big +1 to allowing Sharing bash files or "always run it with args --x --y z" within a company really isn't wonderful, and package.json simply doesn't work in all cases. |
Is this thread mostly about agreeing to filename (e.g. .prettierrc / package.json) and format in it, like opts or JSON? If so I think this should be decided by one maintainer, just do it. There is already a cottage industry of these non-standards, one example from VSCode extension here:
This is not good! We need standard for this, such a simple thing should not be outsourced when in the end it's maintainers that decide these simple things. |
EDIT: moved a proposal regarding perf to #2434 (comment) as more relevant conversations are happening there. |
Editors are the main place where we want people to use prettier, in order for it to be a great experience it should have the following properties:
Proposal
Allow people adding their configuration options inside of prettier in
package.json
:How to find a package version of prettier?
Given a file with a path
a/b/c/d/e.js
, we want to crawl up the tree up until we find a filepackage.json
.It should check in this order:
a/b/c/d/package.json
a/b/c/package.json
a/b/package.json
a/package.json
package.json
If it doesn't find one, then the file isn't inside of a prettier repo.
If it finds one in
a/b/package.json
, then look at the filea/b/node_modules/.bin/prettier
. If it finds one, then use this prettier binary.How to deal with caching?
This is a fair amount of io to be done for each invocation. If you want to add caching, you want to be careful, here are the things to watch out for:
a/b/package.json
is updateda/b/node_modules/.bin/prettier
is updateda/b/c/package.json
ora/b/c/d/package.json
is createdHow to find the configuration associated to a package version of prettier?
Given that a package version of prettier is always in
a/b/node_modules/.bin/prettier
and the package configuration is always ina/b/package.json
, we can inside of prettier check if../../package.json
exists, and if it does read it and use the prettier key.If prettier is being used as a CLI with
require('prettier')
, I'm not sure how to find it and if it's even a good idea to.How do you resolve conflicting rules?
I suggest that we merge rules one by one and the order they are loaded in/override each other is:
package.json
rulesFor example,
would result in
How to make it fast?
Because all the integrations now have to use the cli and can no longer just use the local require version, we're going to pay js/node startup time on every single invocation.
I recommend adding an option to prettier bin
--json-rpc
. With this, you can open the prettier process once and send formatting commands through stdin/stdout without having to restart it every time.A sample connection looks like this:
Conclusion
Let me know what you think :)
The text was updated successfully, but these errors were encountered: