Skip to content
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

husky/lint-staged compatibility #1127

Open
AlecRust opened this issue Mar 16, 2024 · 4 comments
Open

husky/lint-staged compatibility #1127

AlecRust opened this issue Mar 16, 2024 · 4 comments
Labels

Comments

@AlecRust
Copy link

Q A
Version 2.5.0
Bug? unsure
New feature? yes
Question? yes
Documentation? yes

I use husky and lint-staged in my WordPress plugin to e.g. run Prettier to format my PHP code on commit.

I've added GrumPHP via Composer to handle PHP-related linting tasks. Since GrumPHP also looks to "sniff your commits" I assumed GrumPHP would run on the staged files during commit, but this does not seem to happen - only the husky hooks run.

Is there any way you know of to work around this issue, or have you considered native compatibility with husky/lint-staged?

I did try running GrumPHP through lint-staged like I do Prettier:

.lintstagedrc.json

{
    "*": "prettier --write --ignore-unknown",
    "*.php": "./vendor/bin/grumphp run"
}

But it appears you can't pass an argument to the GrumPHP run command, in this case the path(s) of the staged files.

Another option I considered was running Prettier through GrumPHP using npm_script:

grumphp.yml

parameters:
    tasks:
        npm_script:
            script: node_modules/.bin/prettier --check .

But the check never seems to complete with . Any ideas?

My configuration

# grumphp.yml
parameters:
    tasks:
        phpparser:
            visitors:
                forbidden_function_calls:
                    blacklist:
                        - "var_dump"
                        - "print_r"
        phpversion:
            project: "7.0"
        phpcs:
            standard: PSR2
        phpmd:
            ruleset: ["cleancode", "naming"]
        phpmnd:
        phpstan:
            memory_limit: 512M

Steps to reproduce:

  1. Setup repository with husky and lint-staged e.g. to run Prettier on commit
  2. Setup GrumPHP and tasks for it to run
  3. On commit, no GrumPHP Git hooks occur, but lint-staged hooks do
@veewee
Copy link
Contributor

veewee commented Mar 18, 2024

Hello,

It depends which tool you want to make leading I suppose.

I don't know the tool lint-staged, but there might be 2 options:

  • the run command takes a list of files on STDIN which could be used
  • You could use the git:pre-commit command which uses the changed files only

You could indeed make grumphp leading as well and add a custom bash script. I don't know why the command does not return an exit code of 0, but that's probably more related to the tool than to grumphp.

@AlecRust
Copy link
Author

Thanks for your response!

It depends which tool you want to make leading I suppose.

Whichever one will allow me to run both Prettier and GrumPHP 🙂 feels like either approach is close really - I can "almost" run Prettier as a GrumPHP npm task, and I can "almost" run GrumPHP via lint-staged.

the run command takes a list of files on STDIN which could be used

This would be great. Is there any reason GrumPHP currently can't be given paths to run on?

You could use the git:pre-commit command which uses the changed files only

Great idea, thanks. Sadly this doesn't work though as lint-staged still passes the path which causes an error:

[STARTED] Preparing lint-staged...
[COMPLETED] Preparing lint-staged...
[STARTED] Running tasks for staged files...
[STARTED] .lintstagedrc.json — 2 files
[STARTED] * — 2 files
[STARTED] *.php — 1 file
[STARTED] prettier --write --ignore-unknown
[STARTED] ./vendor/bin/grumphp git:pre-commit
[FAILED] ./vendor/bin/grumphp git:pre-commit [FAILED]
[FAILED] ./vendor/bin/grumphp git:pre-commit [FAILED]
[COMPLETED] Running tasks for staged files...
[STARTED] Applying modifications from tasks...
[SKIPPED] Skipped because of errors from tasks.
[STARTED] Reverting to original state because of errors...
[FAILED] prettier --write --ignore-unknown [KILLED]
[FAILED] prettier --write --ignore-unknown [KILLED]
[COMPLETED] Reverting to original state because of errors...
[STARTED] Cleaning up temporary files...
[COMPLETED] Cleaning up temporary files...

✖ ./vendor/bin/grumphp git:pre-commit:

                                                                               
  No arguments expected for "git:pre-commit" command, got "/Users/alec/projec  
  ts/personal/brevwoo/includes/admin.php".  

I don't know why the command does not return an exit code of 0

I'm pretty sure Prettier CLI does return an exit code of 0 when it succeeds. In fact even this npm script doesn't work:

parameters:
    tasks:
        npm_script:
            script: "echo 'Hello World'; exit 0"

Which leads me to think npm script tasks aren't working in GrumPHP at all.

@veewee
Copy link
Contributor

veewee commented Mar 19, 2024

Which leads me to think npm script tasks aren't working in GrumPHP at all.

You are basically running:

 npm run "echo 'Hello World'; exit 0"

Which I dont think is supported.
You should run a script that is defined in package.json.
For exampe:

npm run prettier

when the package.json contains:

{
    "scripts": {
        "prettier": "prettier --check"
    }
}

Next, you need to configure grumphp to execute a nmp run task:

grumphp:
    tasks:
        prettier:
            script: "prettier"
            triggered_by: [js, jsx, coffee, ts, less, sass, scss]
            working_directory: "./"
            is_run_task: true
            silent: false
            metadata:
                task: npm_script

If you just want to run a bash command, you need the shell task.

@AlecRust
Copy link
Author

Ah I see, that makes sense thanks. However in my testing even that is not working 😅

  "scripts": {
    "prettier": "prettier --check ."
  }
parameters:
    tasks:
        prettier:
            script: "prettier"
            triggered_by: [js, jsx, coffee, ts, less, sass, scss]
            working_directory: "./"
            is_run_task: true
            silent: false
            metadata:
                task: npm_script
Running tasks with priority 0!
==============================

Running task 1/7: prettier...
Running task 2/7: phpparser... ✔
Running task 3/7: phpversion... ✔
Running task 4/7: phpcs... ✔
Running task 5/7: phpmd... ✔
Running task 6/7: phpmnd... ✔
Running task 7/7: phpstan... ✔

(same with and without Prettier check errors)

Besides, even if I was able to run Prettier through GrumPHP, it might simplify e.g. CI but it doesn't solve the GrumPHP pre-commit hook not running.

My current lint-staged setup runs all staged files through Prettier on commit. It would be nice to be able to include GrumPHP in that list for *.php files.

The alternative of removing husky/lint-staged and relying on GrumPHP for commit hooks isn't as appealing. Doesn't look like I would be able to e.g. run only all staged files though Prettier. I think I'd need to run a full prettier --write . on every commit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants