nrwl / nx

Smart Monorepos · Fast CI
https://nx.dev
MIT License
23.53k stars 2.35k forks source link

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

Open jacopolanzoni opened 3 years ago

jacopolanzoni commented 3 years ago

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.

tamoore commented 1 year ago

Seriously, why does it output nothing but the affected files. Yes you can write to fix, but it's so unbelievably frustrating to have no idea what the output is.

algoflows commented 1 year ago

I've blasted up there youtube threads with links too this issue, maybe the team will finally address it.

davidplappert commented 1 year ago

please fix ...

Israeltheminer commented 1 year ago

Why hasn't this issue been formally addressed yet? this should be fixed

jasongerbes commented 1 year ago

@vsavkin, this issue was raised nearly 3 years ago. It has the most 👍 and ❤️ reactions of any issue for this project, and has about 46 subscribers.

nx format:check is essentially unusable in it's current state, yet it's still included in the Nx docs.

Can you please give an update on whether this issue will ever be addressed, or mark nx format:check and nx format:write as deprecated and provide an alternative (e.g. npx prettier --check . and npx prettier --write .).

riencoertjens commented 1 year ago

I added a message on their discord if everyone here replies to that maybe they'll notice something? 🤷🏻‍♂️

pujux commented 1 year ago

So I mentioned Miroslav Jonas on Twitter to try and see if the team has taken notice on the issue and here is his reply: https://x.com/meeroslav/status/1706345582242001064?s=46&t=ZnrURlqYmjZQZciRM14ZDA

we might be getting a solution soon 🥳

jared-christensen commented 11 months ago

Hey everyone,

We've found it challenging to determine when nx format:check fails because it merely lists out files without any additional context about the formatting issues. To make this clearer for our team, I created a bash script. This script not only runs nx format:check, but it also explicitly highlights the files with formatting errors and recommends running npx nx format:write to address these issues.

Here is an example of it in action:

1562806300-Window_and_package_json_—_dice-web-platform_—_Dice_Web_Platform

Here's the script:

pretty-format-check.sh

#!/bin/bash

# ANSI color codes
RED='\033[0;31m'
GREEN='\033[0;32m'
CYAN='\033[0;36m'
NC='\033[0m'
SEPARATOR="----------------------------------------"

# Run nx pretty-format-check and capture the output
output=$(nx format:check 2>&1)

# Capture the exit status
result=$?

# Check if the command failed
if [ $result -ne 0 ]; then
    echo -e "${RED}$SEPARATOR"
    echo -e "Error: Formatting check failed"
    echo -e "$SEPARATOR${NC}"
    echo -e "Files with formatting issues:\n$output\n"
    echo -e "${CYAN}Please run 'npx nx format:write' to fix formatting issues."
    exit $result
fi

echo -e "${GREEN}Formatting check passed successfully.${NC}"

To use this script, save it as pretty-format-check.sh and make sure it's executable. You can do this with:

chmod +x pretty-format-check.sh

Also, I've added it as an npm script in package.json for convenience:

"scripts": {
    "pretty-format-check": "./pretty-format-check.sh"
}

Now, you can simply run npm run pretty-format-check to execute the script.

Hope this helps make formatting checks a breeze!

JamesHenry commented 5 months ago

Hi Folks,

Nx format is currently exclusively built on top of prettier. For nx format:check, we leverage the --list-different feature of prettier to show the user which files failed the formatting check just like prettier itself does https://prettier.io/docs/en/cli.html#--list-different).

The simple common case solution is to run nx format:write (powered by prettier --write) and commit the result.

To summarize this thread, however:

Right now a user might want to try and learn more about their format:check failure by passing --verbose but it will not change the output in any way.

Based on the feedback in this issue some users are expecting to see the diff output of what correct vs incorrect formatting would look like. However, because nx format is currently exclusively built on top of prettier, it is therefore subject to its limitations. https://github.com/prettier/prettier/issues/6885 remains unresolved and so there is no native way for users to see a diff.

It is important to note that only showing the names of the files in the default nx format:check output is an important feature - it allows easy piping into custom scripts to perform further handling. Therefore we do not want to change the default output here.

I have opened PR https://github.com/nrwl/nx/pull/23503 which updates the case where --verbose is passed in order to provide more information/context about the overall problem and how to fix it, including linking to the prettier limitation which is at the heart of this.

Before

image

After

image


Notes on actually diffing

Naturally, what we recommend is to continue to give feedback to the Prettier team that you want this feature.

However, if a user here would like to take things a step further and prototype what it might look like for Nx to attempt to make up for the lack of diffing in prettier, then please do, we would be willing to consider a PR for this if a reliable solution can be found.

However, it is worth noting that this behavior should not be activated by simply passing --verbose because

  1. It will be expensive (and the number of failing files could be very large in certain situations)
  2. It will have to be powered by side-effectful disk and git operations, which is not what --verbose is intended to perform

Instead, it would have to be powered by some opt-in additional flag, such as --diff.

JamesHenry commented 5 months ago

I have updated the issue description to more accurately reflect the nature of this request and provided a link to my most recent comment as the latest status on this.

Thanks all!