Husky hooks

[gmrchk]

Description

Adding husky hooks:

• commit: npm run lint which checks for linting problems and fails commit if no good
• push: npm run test which runs all the e2e tests to ensure all is good with the implementation

both can be skipped with --no-verify

Also added/renamed lint:fix to auto-fix the issues found, and kept the lint for only checking so the commit can be failed in such case.

Checks

• The PR is submitted to the master branch
• The code was linted before pushing (npm run lint)
• All tests are passing (npm run test)
• New or updated tests are included
• The documentation was updated as required

Additional information

[cypress]

---

Test summary

42 • 0 • 0 • 0

---

Run details

Project	swup
Status	Passed
Commit	55f49a3 ℹ️
Started	Jan 14, 2023 1:40 PM
Ended	Jan 14, 2023 1:41 PM
Duration	00:50 💡
OS	Linux Ubuntu -
Browser	Electron 106

View run in Cypress Dashboard ➡️

---

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

[github-actions]

Size Change: 0 B

Total Size: 14.9 kB

ℹ️ View Unchanged

Filename	Size
dist/Swup.modern.js	5.04 kB
dist/Swup.module.js	4.88 kB
dist/Swup.umd.js	4.99 kB

_{compressed-size-action}

[daun]

I like precommit hooks, but I think having the tests run before every push is a bit much and could make everything really cumbersome. Especially since we can't merge without passing tests anyway.

How do you feel about renaming lint:fix to just format?

[daun]

Now that we separated out the lint to not autofix things, we should also update the lint action to use npm run lint directly instead of invoking prettier manually.

[gmrchk]

I like precommit hooks, but I think having the tests run before every push is a bit much and could make everything really cumbersome. Especially since we can't merge without passing tests anyway.

I was thinking the same, tho I have the habit of --no-verify everything when I know what I'm doing. It would probably be more targeted to other people than the maintainers directly. Either way, I'm fine with only having the tests run in CI by default as you suggested, it's just for people to find out something is wrong at some point.

How do you feel about renaming lint:fix to just format?

Sounds good. 👍 Any places it should be updated in other than the command itself? 🤔

Now that we separated out the lint to not autofix things, we should also update the lint action to use npm run lint directly instead of invoking prettier manually.

That's done already, unless I'm missing something.

[daun]

Perfect! I think renaming the single script should be enough as it's not referenced anywhere I assume.

[hirasso]

Never heard of husky before. I'm curious – what advantage has this tool for our use case?

For git-hooks, I usually have a folder .githooks in my projects, where I have my hooks defined. And in my package.json I have this:

{
  "scripts": {
    "prepare": "git config core.hooksPath .githooks",
  }
}

That is all that is needed to have auto-installing git-hooks. Am I missing something here?

[daun]

@hirasso I'm all in favor of doing this without additional dependencies or config folders if it works just the same. Doing some research, I've found simple-git-hooks which would let us get rid of the folder completely but at the cost of another dependency.

[hirasso]

@daun what would you think about no dependency for setting up the git hook, as described above, but a dependency (lint-staged) for linting and re-adding git-format-staged for auto-formatting the staged changes? This seems to be the real challenge here, as @gmrchk pointed out in our chat.

[hirasso]

I have created an alternative for you guys to check out: #588 That one is not using husky or any other library, it just sets core.hooksPath to .githooks (the folder where we can store our custom hooks) on npm install. WDYT?

[daun]

If we can do it without another dependency and just plain git hooks, let's do that 🌝