Nx: affected commands do not work in git pre-commit hook

[chris5287]

I wanted to setup a pre-commit hook (using husky) to run 'npm run affected:lint -- --uncommitted' or 'npm run affected:lint -- --base=master', but they both fail, as I believe GIT_DIR is set to '.git' whilst in pre-commit, which makes commands like 'git merge-base' fail :(

I found details about what environment variables git hooks set here: https://www.digitalocean.com/community/tutorials/how-to-use-git-hooks-to-automate-development-and-deployment-tasks#an-aside-about-environmental-variables-with-git-hooks

[chris5287]

Any ideas @FrozenPandaz ?

[FrozenPandaz]

Adding husky seems to work for me: https://github.com/FrozenPandaz/nx-node-poc/commit/6b37ceb57f61f9166fd0618fceae32ec5722fd11

Can you provide a repo?

[chris5287]

Thanks @FrozenPandaz for getting back to me.

I have create a repo, it appears the issue only occurs if the package.json is not in the root directory

See repo (i've just moved your code into a sub-folder): https://github.com/chris5287/nx-node-poc/tree/husky

git commit -m "Test commit"
husky > pre-commit (node v8.11.3)
yarn run v1.2.1
$ ./node_modules/.bin/nx affected:lint --uncommitted
error: Could not access 'HEAD'
Command failed: git diff --name-only HEAD .
error: Could not access 'HEAD'

error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
husky > pre-commit hook failed (add --no-verify to bypass)

[xmlking]

I have setup husky hooks via lint-staged here
https://github.com/xmlking/ngx-starter-kit/blob/develop/package.json

noticed sometime nx affected:lint --uncommitted lint checking all modules instead of files that are uncommitted

[chris5287]

@FrozenPandaz just wondering if you’ve had chance to look at my repo?

[xmlking]

  "husky": {
    "hooks": {
      "commit-msg": "commitlint -E HUSKY_GIT_PARAMS",
      "pre-commit": "lint-staged"
    }
  },
  "lint-staged": {
    "{apps,libs}/**/*.{ts,json,md,scss,html}": [
      "npm run affected:lint  -- --uncommitted --parallel -- --fix",
      "npm run format:write -- --uncommitted",
      "git add"
    ]
  },

for me git commit fails with hooks. but npm run affected:lint -- --uncommitted --parallel -- --fix command runs without error.

[chris5287]

@FrozenPandaz just wondering if you’ve had chance to look at my repo?

[maeri]

Hey @FrozenPandaz - any feedback or resolution on this? Thanks

[maeri]

@chris5287 - any chance you did resolve this?

[chris5287]

@maeri afraid not, I provided a repo earlier in this issue but I don’t think it’s been looked at yet

[vdiaz1130]

Hello, looking for this solution as well. Using Husky and trying to execute tests on pre-commit hook. Using the --uncommitted flag proceeds but nothing is tested.

// .huskyrc.js
const tasks = (t) => t.join(' && ');

module.exports = {
  hooks: {
    'pre-commit': tasks([
      'npm run affected:test -- --base=master --uncommitted', // tried --onlyChanged but get errors.
    ])
  }
};

From the log:

9 verbose lifecycle [email protected]~affected:test:master: CWD: /Users/username/Documents/workspace-dir/project-web
10 silly lifecycle [email protected]~affected:test:master: Args: [ '-c', 'nx affected:test -- --base=master --onlyChanged' ]
11 silly lifecycle [email protected]~affected:test:master: Returned: code: 1  signal: null
12 info lifecycle [email protected]~affected:test:master: Failed to exec affected:test:master script
13 verbose stack Error: [email protected] affected:test:master: `nx affected:test -- --base=master --onlyChanged`
13 verbose stack Exit status 1
13 verbose stack     at EventEmitter.<anonymous> (/usr/local/lib/node_modules/npm/node_modules/npm-lifecycle/index.js:301:16)
13 verbose stack     at EventEmitter.emit (events.js:198:13)
13 verbose stack     at ChildProcess.<anonymous> (/usr/local/lib/node_modules/npm/node_modules/npm-lifecycle/lib/spawn.js:55:14)
13 verbose stack     at ChildProcess.emit (events.js:198:13)
13 verbose stack     at maybeClose (internal/child_process.js:982:16)
13 verbose stack     at Process.ChildProcess._handle.onexit (internal/child_process.js:259:5)
14 verbose pkgid [email protected]
15 verbose cwd /Users/username/Documents/workspace-dir/project-web
16 verbose Darwin 18.6.0
17 verbose argv "/usr/local/bin/node" "/usr/local/bin/npm" "run" "affected:test:master"
18 verbose node v10.16.0
19 verbose npm  v6.9.0
20 error code ELIFECYCLE
21 error errno 1
22 error [email protected] affected:test:master: `nx affected:test -- --base=master --onlyChanged`
22 error Exit status 1
23 error Failed at the [email protected] affected:test:master script.
23 error This is probably not a problem with npm. There is likely additional logging output above.
24 verbose exit [ 1, true ]

[dsebastien]

Same issue here, the command runs fine when I execute it, but it fails when executed through Husky

[vsavkin]

I'm not sure if using precommit for running affected is a great idea.

Even in mid-sized project running affected:lint can take a couple of minutes if the change is large. You probably wouldn't want to block your commit for that long, right?

[maeri]

Currently, I'm using an external script to simulate the pre-commit phase where affected: commands are run. Ideally, I would like to get rid of this 'hack' and run 'affected:..' within git hooks - at least to have that possibility to decide for which project it can run. As for now it simply does not work.

[dsebastien]

@vsavkin I get your point but I thought that automating code linting and formatting through git hooks was appealing, as a way to avoid having to think about those.

Some developers get lazy when in a rush and they tend to skip some steps. Of course a good review process will keep such issues in control, but automation is nicer (i.e., it becomes a non-issue).

If using a pre-commit hook is not the way to go, then do you have other ideas/recommendations for this use case?

[vsavkin]

Folks,

Here are some thoughts:

1. I think that committing frequently while developing is a good idea.
2. If committing takes even 10 seconds, I either won't commit frequently or will use --no-verify to skip the hooks.
3. I think using pre-push can be a good idea. You push a lot more rarely comparing to committing and pushing already can take a few seconds, so waiting for a few more seconds is often OK. Not for linting though. Linting in a mid-ze repo can take 10 minutes. You obviously don't want to block pushing for 10 minutes. The pre-push hook can work well for formatting because it's relatively fast.

The pre-push hook should work because HEAD should be defined at this point.

[vsavkin]

Hey folks! Closing this issue due to inactivity. Feel free to open another or reopen this one if you want to continue the conversation.

[michelcve]

@vsavkin could you elaborate a bit on this. What would your pre-push look like, in case one would want to run a format:check in pre-push?

I'm not sure how to call it in pre-push, and let it check all previous (non-pushed) commits as well.

[iamandrewluca]

This https://github.com/xmlking/ngx-starter-kit/blob/2475ede178be5a79078454e1817fe2ea58b8cd2e/package.json#L84-L89
But I would exclude yarn affected:lint from precommit

[michelcve]

@iamandrewluca Thanks for your response, but your example refers to the pre-commit. I was hoping to add it to pre-push (which may span several local commits)