nx icon indicating copy to clipboard operation
nx copied to clipboard

Feature Request: nx format:check should provide a workaround for Prettier not supporting format diffing

Open jacopolanzoni opened this issue 4 years ago • 60 comments

UPDATE FROM NX TEAM (2024-05-17)

Please read: https://github.com/nrwl/nx/issues/4159#issuecomment-2118165264


Description

As nx format:check shows which files fail the format check but not what is actually wrong in them, I would love if the --verbose option could actually add that information. At the moment, unless I am doing something wrong, nx format:check --verbose has the same output as nx format:check.

Motivation

The command would give more information about what is actually wrong in the file format.

Suggested Implementation

I would love if the --verbose option could actually add that information.

Alternate Implementations

As an alternative, I would like even only to see either the line where the check fails, or maybe the broken rule.

jacopolanzoni avatar Nov 26 '20 17:11 jacopolanzoni

This issue has been automatically marked as stale because it hasn't had any recent activity. It will be closed in 14 days if no further activity occurs. If we missed this issue please reply to keep it active. Thanks for being a part of the Nx community! 🙏

github-actions[bot] avatar Mar 29 '21 14:03 github-actions[bot]

It's like a slap in the face without ever even telling you why. Imagine a compiler telling you "Error, GFY, period".

Please add this.

konrad-garus avatar Sep 06 '21 11:09 konrad-garus

My entire team is complaining about this, going to have to remove my formatting check from pre-commit hook because it is just confusing everyone. The --verbose flag should be at minimum spitting out the same output of the format:write command which at least shows the command crawling through the project directories doing the formatting.

FYI for others with this issue it seems like you can run prettier using npm and get the output from running the prettier command directly instead of through Nx.

TCModus avatar Nov 18 '21 15:11 TCModus

Why it's closed? My project is failing on nx format:check without any message besides the error Command failed with exit code 1. and --verbose adds nothing to it

Alspaladin avatar Dec 28 '21 18:12 Alspaladin

I don't understand "stale" logic as well. My pipeline also failed at this step without any reason. Verbose is not informative at all.

eprokofev avatar Dec 29 '21 19:12 eprokofev

Yeah, reopen this issue - verbose is broken

xaphod avatar Jan 12 '22 18:01 xaphod

Same here

KlausNie avatar Jan 20 '22 10:01 KlausNie

Just ran into the same issue, would like to see this issue being reopened...

konnic avatar Feb 22 '22 14:02 konnic

If it helps anyone, running nx format:write solved issue for me. Nevertheless, it would be nice if the --verbose flag provided some extra info.

konnic avatar Feb 22 '22 16:02 konnic

Try adding angular.json, nx.json, and tsconfig.base.json to your prettier ignore file and that should work. I noticed on every change prettier was touching angular.json file even when it wasn't changed, once I ignored those files I was getting proper behavior running nx format:check without the verbose flag (we actually no longer run check and our pre-commit hooks run nx format:write which has been working fine after ignoring the above files).

TCModus

On Tue, Feb 22, 2022, 11:33 AM konnic @.***> wrote:

If it helps anyone, running nx format:write solved issue for me.

— Reply to this email directly, view it on GitHub https://github.com/nrwl/nx/issues/4159#issuecomment-1047984058, or unsubscribe https://github.com/notifications/unsubscribe-auth/APEHQMLCL653STVWERMXE43U4O3DZANCNFSM4UD77B6A . You are receiving this because you commented.Message ID: @.***>

TCModus avatar Feb 22 '22 17:02 TCModus

nx format:check --verbose is still broken

edit: It also appears that if you provide the base and head params the command exits with 1 but doesn't print the files that failed formatting.

e.g.

nx format:check --base=main --head=HEAD
# exit code 0 

nx format:check --base=main --head=HEAD --verbose
# exit code 0 

nx format:check
 >  NX   Affected criteria defaulted to --base=main --head=HEAD
/path/to/failed/file
# exit code 1

nx format:check --verbose
 >  NX   Affected criteria defaulted to --base=main --head=HEAD
/path/to/failed/file
# exit code 1

Unless there's some way to turn off the "warning" logging that nx has select a base and head, we can't even pipe the results of nx format:check and parse the results.

StephenStrickland avatar May 04 '22 22:05 StephenStrickland

This is passing locally and failing in CI for me. Both running on same node and npm versions.

shawnmclean avatar Jun 21 '22 20:06 shawnmclean

How has this not been addressed , it's running fine locally, failing in CI with no information.

savvyshell avatar Jun 21 '22 23:06 savvyshell

init-commands: |
  npx nx-cloud record -- npx prettier --write .
  npx nx-cloud record -- npx nx format:write
  npx nx-cloud record -- npx prettier --check .
  npx nx-cloud record -- npx nx format:check

I added this to my actions.yml file and it seems to do the trick. Rather than rely on nx format:check, I hand it over to prettier since it's tied to my eslint configuration and I'd imagine nx format:check is using part of that anyways. I have it auto format with prettier followed by nx format:write just to be safe before checking with both prettier and nx format check.

It's probably not necessary to use both but just to still make sure NX is some-what happy, I wanted to use their format checker.

Seems to work now, hope we can get a proper solution for this in the future. At least with npx prettier --check, we get a bit more of a verbose output than format:check's tragic lack of insight in its output.

savvyshell avatar Jun 22 '22 10:06 savvyshell

Please correct me if I'm wrong but it seems like all nx does is execute prettier under the hood anyways: https://github.com/nrwl/nx/blob/969aa32659f6918cb0eeeba6b02d6485659dbdd2/packages/nx/src/command-line/format.ts#L201

So if we want more verbose output, this will probably be the place to implement it.

MaxKless avatar Jun 23 '22 13:06 MaxKless

angular.json, nx.json, and tsconfig.base.json

That worked for me thanks friend

finally got nxcloud builds in flight

npmjs.org held me down for whole month in a user account debilitation locked out from their case logic creating users for emails that they dont even validate... ditched npmjs for github registry and I think this is the trend. sorry npmjs its been real.

nxcloud rocks... integrated to everything

do not release bad stuff into the public domain its going to get engineers very upset who are live in the field operating prod instances and thus far Nx and GitHub has been great. Wish I could say same for yarn, npm, webpack and other flanking entities. I know I wont sign my name to anything buggy. npmjs was last straw.

nhhockeyplayer avatar Jul 09 '22 11:07 nhhockeyplayer

why is this closed as completed if it's still a problem?

pujux avatar Aug 01 '22 10:08 pujux

Our team is having the same issue.

Clean-Cole avatar Aug 03 '22 14:08 Clean-Cole

+1 for showing what is wrong with the file(s)

kekiel avatar Aug 22 '22 07:08 kekiel

I am still having this issue with my CI any solution?

dgesteves avatar Aug 22 '22 10:08 dgesteves

Same issue here, we are missing the detailed error output of prettier. Would be great if you could reopen the issue.

StefanGreffenius avatar Aug 25 '22 12:08 StefanGreffenius

We parse our build logs to collect data on issues. The output of nx format:check is less than useless and is impossible to parse since there is nothing identifying the error, except the exit code. I would at least expect some kind of line (even without the --verbose flag) like:

NX/Prettier format check error(s) for file(s):
<the file list>

But as others have mentioned, actually showing some verbose output is desperately needed.

PayBas avatar Sep 29 '22 00:09 PayBas

bump This is affecting my work flow as well. Please add descriptive text that identifies what is being flagged.

gerald-kalafut avatar Oct 14 '22 16:10 gerald-kalafut

+1 for providing info on failed check please 🙏

amogower avatar Oct 18 '22 16:10 amogower

bump, please this is needed. 🙏

kodeine avatar Oct 24 '22 17:10 kodeine

Still verbose not working correctly

meisterveda avatar Oct 25 '22 01:10 meisterveda

$ nx format:write

NX Affected criteria defaulted to --base=develop --head=HEAD

libs\common\ui-sales-components\src\lib... 477ms libs\common\ui-sales-components\src\lib\save... 183ms angular.json 236ms nx.json 12ms tsconfig.base.json 49ms

Et voilá!

JPedroSMFerreira avatar Nov 11 '22 19:11 JPedroSMFerreira

@JPedroSMFerreira, it worked like a gem!

But --verbose still needs a fix.

Thank you!

brumargues avatar Nov 12 '22 15:11 brumargues

@JPedroSMFerreira, it worked like a gem!

But --verbose still needs a fix.

Thank you!

This issue is about the format:check command. It's not a solution if we have to change the command, which is completely changing the logic, by the way.

We could forward args to the prettier command, or you could find a way to print your message before the format:check execution.

It would've been nicer to have a solution developed by the NX team to make it more flexible. I had a complaint from my team that they couldn't figure out why they could not commit their changes because of the output of the format command. We got used to it, though, and adjusted our processes.

exsesx avatar Nov 12 '22 16:11 exsesx

What's happening with this lol, two years and nothing!

algoflows avatar Nov 18 '22 12:11 algoflows