vercel / git-hooks

No nonsense Git hook management
MIT License
203 stars 10 forks source link

Mono repo support? #2

Open jaydenseric opened 5 years ago

jaydenseric commented 5 years ago

With a structure like this:

.
├── .git
├── api/
│   └── package.json
└── app/
    ├── hasLintError.js
    └── package.json

In app/package.json, using @zeit/git-hooks with lint-staged has no effect.

Checking the .git/hooks directory, there are only .sample files in there.

Does that mean @zeit/git-hooks does not work for monorepo packages?

Qix- commented 5 years ago

Not currently, no.

Supporting monorepos will require some opinions:

  1. What happens when two package.json files use @zeit/git-hooks?
  2. What happens when two package.json files use different versions of @zeit/git-hooks?
  3. What happens when two different projects (not necessarily even package.json-based projects) need to install Git hooks?
  4. How do we determine which package.json to evaluate for hooks? By file? What if there are cross-sub-repo dependencies (e.g where something in api/ depends directly on app/)?
  5. If we evaluate all hooks on commit, then projects with many sub-projects will have all of their hooks run for each Git operation, which means potentially high Git times for simplistic changes.

Perhaps you can help answer some of these as I do not use monorepos much myself.

jaydenseric commented 5 years ago

All good questions. This used to be a problem in husky too: https://github.com/typicode/husky/issues/134.

These days husky supports mono repos:

Qix- commented 5 years ago

Sure I think it's possible.


I think there will have to be a file listing at .git/hooks/zeit_hooked_directories that is a line-delimited list of paths relative to the repository root that require hooking.

If git-hooks detects hooks there, it will update the list of directories.

That solves points 1 and 2 (in most cases).


However, the other points still need to be answered.

I don't think there's any clean solution for point 3. The best we can do is to detect if we're a submodule. In that case, we probably shouldn't install hooks (as submodules aren't meant to be working directories anyway).

Do we run ALL hooks, or only hooks for paths that are in or above directories of staged files? That will solve point 4.

Point 5 is probably up to the developer to make sure that this doesn't happen, or that hooks don't take forever. That kind of addresses point 4 as well.

antoine-pous commented 5 years ago

IMO described problems aren't.

  1. What happens when two package.json files use @zeit/git-hooks?

Nothing, it's not your concern.

  1. What happens when two package.json files use different versions of @zeit/git-hooks?

Nothing, it's not your concern too.

  1. What happens when two different projects (not necessarily even package.json-based projects) need to install Git hooks?

This part is a bit hard to implement, but IMO the right way is manage hooks dynamically. Nothing disallow your lib to generate some files. So instead of creating one big hook, generate small dedicated sub-hooks which will be called by git hooks.

It can results to this kind of FS:

/.git/hooks
=> pre-commit
=> pre-commit-scripts/
=> => project1
=> => project2

Where project1 and project2 are the package name into the package.json file.

  1. How do we determine which package.json to evaluate for hooks? By file? What if there are cross-sub-repo dependencies (e.g where something in api/ depends directly on app/)?

Yup, exclusively by the file path nothing else, this is the only way to do something easily compliant with most of monorepo projects and easy to understand. If someone need something more specific he can fork this project and made a PR if it's approach is enough generic for some uses case. Your main goal as maintainer is providing a reliable feature, not considering each exotic cases.

  1. If we evaluate all hooks on commit, then projects with many sub-projects will have all of their hooks run for each Git operation, which means potentially high Git times for simplistic changes.

Point 3 solve this problem naturally

Qix- commented 5 years ago

@antoine-pous Unless I'm missing something with how Git works, what you're describing isn't possible in a portable way. It is our concern if we're going to support other projects to use git hooks too - node.js or otherwise.

If you can PR something that proves what you're describing is possible then cool, but AFAIU that's not possible.

antoine-pous commented 5 years ago

I'll made some tests soon to validate my approach, but i don't see why this isn't possible. Hooks files managed by git should be usable as router 🤔

nathanredblur commented 4 years ago

I would like to throw some ideas that could be useful in this discussion.

I have a a repo with 4 micro-services each with his own package.json. I wanted to run some lints rules before do a commit in /app micro-services, but I have the problem that if some of my workmates works in other micro-service, the the hook was triggered.

so i did this small script and change my package.json configuration to run it. I need it to use husky to make it work

"husky": {
    "hooks": {
      "pre-push": "sh husky.sh"
    }
  },
#!/bin/bash

SRC_PATTERN="app/"

if git diff @{push} --cached --name-only | grep --quiet "$SRC_PATTERN"
then
  npm run lint && npm run type-check
else
  exit 0
fi

this will check if the changes to be pushed is part of the folder that this hook belong. that would be the ideal solution.

  1. If you have a monoRepo, the hook would check where the git-hooks was installed or where you have packages.json with a valid git-hooks configuration and save this locations for later use.
  2. git-hooks will then check which paths changes happen and trigger the task in each package.json where the files where modify. e.g. this is your repo structure
    
    -app
    |- app.js
    |- package.json // pre-push: "npm lint"
    - api
    |- api.js
    |- package.json  // pre-push: "npm test"
    - utils
    |- index.js
    |- package.json  // no hook

- if you modify `api.js` only `npm test` is execute,
- if you modify any file inside `/app` then `npm lint` is execute
- if you change any file in `/utils`  nothing happen
- if you modify `app.js`, `api.js` and `utils/index.js`, then `npm lint` and `npm lint` is executed
itpropro commented 3 years ago

Been nearly a year, any update on monorepo support or is husky v4 still the way to go?

Qix- commented 3 years ago

@itpropro Any updates would otherwise show up on this issue, so no. If you need that support, Husky might be able to do it for you.