Fix executeHooks to quote args for shell command

[vhenzl]

Description

Related #8382

Arguments for commands in CLI hooks are escaped properly using shell-quote.

Arguments are escaped only for execShellCommand; those passed to a custom script function are preserved in their original form.

Type of change

• Bug fix (non-breaking change which fixes an issue)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)

Possibly breaking for onError hook

How Has This Been Tested?

Locally in our project.

Checklist:

• I have followed the CONTRIBUTING doc and the style guidelines of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes
• Any dependent changes have been merged and published in downstream modules

[changeset-bot]

🦋 Changeset detected

Latest commit: f7a1630

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

Name	Type
@graphql-codegen/cli	Patch
@graphql-cli/codegen	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[vhenzl]

quotedArgs is not used here on purpose as the text in the log message would contain different values than script receives. It could be confusing to have escaped arguments in the log but not in the function.

[charlypoly]

@vhenzl, I did some local QA and found out that the script should be also quote().
With the following hooks configuration:

hooks:
  afterAllFileWrite:
    - prettier --write /project/app/routes/blog/$postId/comments.generated.ts

The prettier --write /project/app/routes/blog/$postId/comments.generated.ts command is not properly escaped with your current implementation.
It seems that quoting the script variables (in executeHooks()) could do the work.

[vhenzl]

Rebased to resolve conflict in yarn.lock.

[vhenzl]

@charlypoly, I dissagre.

Whatever is specified in a hook should be a valid command on its own. You should be able to copy&paste it to a terminal, and it should run. You are in control of what's in the config, so you should escape it if needed:

hooks:
  afterAllFileWrite:
    - prettier --write '/project/app/routes/blog/$postId/comments.generated.ts'

But if you really want to use variables in your hooks, you should be able to do that:

hooks:
  afterAllFileWrite:
    - prettier --write $MY_PROJECT/frontend

If we escape everything automatically, this wouldn't be possible.

[Dodobibi]

This doesn't work on Windows...

hooks:
  afterAllFileWrite:
    - prettier --write

is breaking because the quoted path is wrong when absolute.
C:/PATH/TO/FILE is transformed as C\:/PATH/TO/FILE instead of C:/PATH/TO/FILE

[charlypoly]

This doesn't work on Windows...

hooks:
  afterAllFileWrite:
    - prettier --write

is breaking because the quoted path is wrong when absolute. C:/PATH/TO/FILE is transformed as C:/PATH/TO/FILE instead of C:/PATH/TO/FILE

Hi @Dodobibi,

Could you open an issue? 🙏🏼

Thank you!