-
Notifications
You must be signed in to change notification settings - Fork 507
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
Separate prettier command eg. "tsdx format" #917
Comments
tsdx format
tsdx format
TSDX does use
I'm not sure if this is actually correct, I feel like there should be a way of doing this and would also like to use that feature of Prettier. That being said, it's important to actually lint (not just format) the other types of files too, which might be outside the scope of TSDX and add bloat. I personally run a variety of linters in my IDE etc (and run Policy-as-Code and SAST in CI).
This proposal would be a breaking change in it's current state. To give an alternative that works today: Prettier is also very much usable in user-land too. Since it's already in your dependency tree with TSDX, you can add some {
"format": "prettier --write './**/*'",
"// format:check": "for pre-commit and CI usage",
"format:check": "prettier --check './**/*'"
} |
TSDX uses
Yes this is of course possible but then I have 2 ways to run prettier and things get confusing for less experiences people. Hiding prettier behind lint is not transparent and requires these workarounds. I understand it would be a breaking change. |
Yes, I've read that section, but aside from the performance piece, that section is all an opinion on usability. Some people definitely prefer using
IMO having 2 different tools doing similar things (and
Well TSDX's purpose is to hide a lot of things behind an abstraction. I would say "workaround" is a bit of a strong term given that I'm open to changing this if there's enough support for it, but I don't think that'll happen on a near-term timeline. |
Disclaimer: I understand tsdx IS opinionated tool and while this request is (imho) based on some facts, it's still mostly about my opinion. One of the points of the prettier is that user doesnt need to care about formatting at all, so showing bunch of red errors in editor while writing the code is just bad DX. I would say most of the devs just write js/ts code and hit save to auto-format the code. So when they see red errors coming from lint, they assume they are breaking some lint rules while its just formatting. It also adds more dependencies to tsdx which need to be updated and another layer of abstraction which might break when upgrading to major versions of eslint / prettier. When you have "clean & separate" eslint and prettier, it's easier to upgrade and to think about the tools. I personally believe in single-responsibility principle which means lint does linting and format does formatting. How can single responsibility make things confusing :) Usually its the other way round. |
Well But as I wrote there combining tools typically means for more bugs, nuances, and hacks and is well-evidenced in TSDX issues itself (some mentioned there too), so I'm much more inclined to separate them and am in fact planning to separate each command as a separate package and configs as separate presets (#634 ). |
@Kamahl19 I wonder, how do you see the red errors coming from prettier? It doesn't work for me. I just installed new tsdx project, changed Only after I run
Autofix on commit doesn't work either. I had to add new command to package.json scripts.
|
Current Behavior
Prettier is integrated into eslint and runs in one
tsdx lint
command. This comes with couple of disadvantages described at https://prettier.io/docs/en/integrating-with-linters.html . It is also not possible to run prettier on non JS/TS files eg. json, css/less/scss, md, yml etc..Desired Behavior / Suggested Solution
tsdx lint
runs only lint without prettier.tsdx format
runs prettier and it's possible to configure all file types we want to format. Default can bets,tsx..
while the user can decide to also formatyml, sass
etc.The text was updated successfully, but these errors were encountered: