lint-staged uses prettier before git commit

[abhcs]

Description

Fix #1097.

Changes

• deleted husky post-commit because it takes too long.
• modified husky pre-commit to use only prettier on all files (.prettierignore should ensure that the correct files are fixed).
• added a script for installing husky, as described here.
• removed eslintconfig from package.json since we use .eslintrc.json for eslint settings.

Tests

Note that you must use npm run prepare-husky to enable husky (this should be in PolicyEngine readme).
I tested the change by trying to commit a file with the wrong format. The format was correct after commit.

[anth-volk]

Can you expound a bit more on the benefits of moving husky to pre-commit? I'm hoping to weigh the advantages here, because I see two potential major disadvantages:

1. Contributors who make, say, only a few contributions will almost certainly forget to run npm run prepare-husky, though this could be included in some sort of pre-commit make rule; if it's not included in a make rule, then it's also inconsistent with what we currently do for most other actions
2. I have to wonder if moving husky expands these changes beyond the target of this issue; is it possible to move the prettier check without moving husky around, then open a new issue for husky?

[abhcs]

husky is only there to manage git commits. lint-staged is there to ensure that the scope of lint during pre-commit is restricted to staged files. lint-staged depends on husky.

No one is using husky or lint-staged right now. You only use husky if you have installed it through husky install.

We can add npm run prepare-husky to make install.

[abhcs]

husky post-commit is doing too much. Why lint the entire code after commit? It slows down commits by a lot. Right now since husky is not installed by anyone, we do nothing pre- or post-commit.

It is best to start with a minimal pre-commit file and add more things if deemed necessary.

[anth-volk]

Okay, I get what you're saying - also, strange to have husky config files without installation. I know this might come off as nitpicky, but I just want to clarify a couple more things on this so that we can get it right:

1. I'm curious why you removed the --ignore-unknown flag on prettier in lint-staged
2. Is there a benefit to dropping eslint from lint-staged?
3. By removing the eslintConfig option from the package.json, we also lose the two presets that were declared there, as they don't exist within .eslintrc.json at the moment - could you add those in there?

[abhcs]

Okay, I get what you're saying - also, strange to have husky config files without installation. I know this might come off as nitpicky, but I just want to clarify a couple more things on this so that we can get it right:

1. I'm curious why you removed the --ignore-unknown flag on prettier in lint-staged

Because prettier does not use the flag in the check in the lint or fix commands. .prettierignore is deciding what prettier ignores.

2. Is there a benefit to dropping eslint from lint-staged?

We can use eslint --fix as well. prettier is just a lightweight formatter while eslint can do much more depending on how it is configured, and hence can be much slower. I found that the annoying lint failures in the PR workflow usually stem from prettier checks -- so adding just the prettier fix here made sense to me.

3. By removing the eslintConfig option from the package.json, we also lose the two presets that were declared there, as they don't exist within .eslintrc.json at the moment - could you add those in there?

This section in the file is overridden by .eslintrc.json -- it may have been used in the past when the file did not exist. Enabling the react-app config is the point of PR #1081.

[anth-volk]

I gotcha. In that case, would you mind fixing the merge conflict and adding eslint --fix back into the script? Otherwise, I think it'd be good to go.

[anth-volk]

Thanks so much! Looks great.