Add Prettier script and pre-commit check for HTML and stylesheets

[nospame]

Product Description

Dev only. Adds a check to pre-commit to identify if modified HTML or stylesheets could use reformatting, e.g.:

$ git commit -m "billing statements"
corehq/apps/domain/templates/domain/billing_statements.html
HTML and/or stylesheets have code style issues.
Run ./scripts/prettify-changed.sh to fix them in a separate commit.

There are formatting errors in your commit. Commit with --no-verify to proceed without fixing.

If there are also Python formatting issues raised by flake8, they will appear first, followed by the above notes about html or stylesheets. The --no-verify flag will still work as it currently does, by skipping the check entirely.

$ ./scripts/prettify-changed.sh
Formatting files changed on this branch:
corehq/apps/domain/templates/domain/billing_statements.html 33ms

Remember to commit these changes.

Technical Summary

Adds prettier and prettier-plugin-jinja-template to dev dependencies along with relatively simple shell scripts to use the dependency. As written will require devs to run yarn install or they will see an error from node on trying to commit html/css/scss/less files (if using the pre-commit hook).

Prettier was chosen here over other formatters for its speed, multi-format compatibility, ease of use, and minimal dependencies. The downside is it is not exceptionally configurable. If devs find it truly does not format to our liking, we might consider a different (more explicitly configurable) formatter.

Safety Assurance

Safety story

This adds two dev dependencies, both widely-used and maintained. Other changes are only in dev scripts, so would have no effect elsewhere.

Automated test coverage

Nope.

QA Plan

Not formally, but requesting that devs try it out.

Rollback instructions

• This PR can be reverted after deploy with no further considerations

Labels & Review

• Risk label is set correctly
• The set of people pinged as reviewers is appropriate for the level of risk of the change

[mkangia]

Nice!

Can we include in PR description what formatting rules we are adding?
I see we have set tab spaces. or there are no rules and that is just to set the tabsize.

[mkangia]

fyi @millerdev for the dependencies being added.

[millerdev]

staged and uncommitted

Does this include files that are not staged (or only partially staged)? If yes, I would be annoyed if I unsuspectingly ran a script and it modified files that had unstaged changes since it could be difficult to untangle hand-crafted changes from automated prettifier changes.

[orangejenny]

Yeah, and as a reviewer I much prefer when formatting changes are separate from logical changes. One way to mitigate this is to change the instructions to encourage committing the formatting separately. Another is to reformat the entire repo once this is merged (which I think we should do anyway, it'll take forever to reformat opportunistically).

[nospame]

In local testing this correctly excludes files that are not staged, but it does include files that are partially staged and formats them as a whole. If that's a major issue, we could use something like lint-staged or git-format-staged to get formatting for only those portions of files that are staged.

[nospame]

@orangejenny would preferable behavior in your mind be something more like this?

• Identify which files want reformatting in pre-commit, but don't block the commit for it
• Recommend running the command to reformat those whole files
• Use the prettify-staged (or a renamed/modified version) script to optionally commit those files as well, similar to the build_bootstrap5_diffs command

[orangejenny]

git-format-staged seems nice for its purpose (I'm also a devotee of -p when staging changes). I think the downside is that we'll then have files that have old formatting interspersed with new, but that's fixable by then formatting the entire file in a new commit that reviewers can largely ignore.

[orangejenny]

(commented before seeing your comment addressed to me, and seeing your previous comment gave me more to think about) I think blocking the commit is fine, since --no-verify exists. It's good to have an attention-grabbing moment, and blocking the commit does that. I was basically asking to append "in a separate commit" to "Run ./scripts/prettify-staged.sh to fix them automatically."

[nospame]

@orangejenny That seems reasonable. I think there would also need to be a corresponding change to the behavior of prettify-staged for usability. If we're asking a developer to make style fixes in a separate commit, perhaps that script should target all files that have been changed and are committed relative to master, and maybe call it prettify-changed or something?

[nospame]

Nice!

Can we include in PR description what formatting rules we are adding? I see we have set tab spaces. or there are no rules and that is just to set the tabsize.

@mkangia Yeah, Prettier in particular is really minimal in the options it allows. Aside from tab width which we want as 2-space instead of the default 4-space they use, the default options seemed the closest to what we liked. Other relevant options that this currently uses the default for are print width (which is different from a "max width"), bracket line, whitespace sensitivity, and single attribute per line.

[jingcheng16]

@nospame Reviewed but have no questions! Nice work!