Skip to content

chore: add lefthook pre-commit hooks - #198

Open
lanesawyer wants to merge 25 commits into
lane/upgrade-pnpm-11from
lane/lint-fix-and-pre-commit-hooks
Open

chore: add lefthook pre-commit hooks#198
lanesawyer wants to merge 25 commits into
lane/upgrade-pnpm-11from
lane/lint-fix-and-pre-commit-hooks

Conversation

@lanesawyer

@lanesawyer lanesawyer commented Oct 2, 2025

Copy link
Copy Markdown
Collaborator

What

Add lefthook pre-commit hooks. The hook autofixes what it can on commit and never blocks — CI remains the gate for anything unfixable.

📚 Stacked on #289 (pnpm 11 upgrade). Hook installation relies on allowBuilds in pnpm-workspace.yaml, which is pnpm 11 syntax, so this must merge after #289. Review the diff against lane/upgrade-pnpm-11.

How

Adds lefthook.yml with three pre-commit commands:

Command Files Tool
lint JS/TS + .astro oxlint --fix
fmt JS/TS/JSON/YAML/MD/MDX/HTML oxfmt
fmt-astro .astro prettier --write

Plus a lefthook devDependency and lefthook: true under allowBuilds in pnpm-workspace.yaml.

Notable details:

  • Non-blocking. Each command ends in || true, so an unfixable lint error is printed but still lets the commit through.
  • stage_fixed: true re-stages files the tools modified, so the commit records the formatted output rather than what was originally staged.
  • priority forces oxlint --fix to run before oxfmt. Lefthook orders commands alphabetically by default, which would have run the formatter first and left lint autofixes unformatted.
  • glob per command must cover every file type its tool handles. The globs were derived by probing the tools rather than assuming: oxfmt reformats js/jsx/mjs/cjs/mts/cts/ts/tsx/json/jsonc/yaml/yml/md/mdx/html, and oxlint reads .astro in addition to JS/TS. An earlier revision omitted md/mdx/html/astro, which let a markdown- or astro-only commit skip the hook and then fail CI.
  • Hook installation follows lefthook's documented pnpm setup — allow its postinstall to run, and that syncs the hooks. Note the docs also say to add pnpm.onlyBuiltDependencies to package.json; pnpm 11 ignores that field and warns about it, so only the pnpm-workspace.yaml entry is used.

Verification

Tested from fresh clones on pnpm 11.21.0, driving real git commit calls and asserting on committed blob contents:

  • Fresh clone → pnpm installpre-commit hook present; a badly formatted .ts then commits as formatted output.
  • A .ts with an unfixable no-console error plus bad formatting: commit succeeds, error reported, committed blob is formatted.
  • An unformatted .md makes pnpm run check exit 1; the hook formats and re-stages it, so CI stays green.
  • An .astro file with a lint error is reported by oxlint and formatted by prettier.
  • Staging only a file no tool handles: all three commands skip, zero files touched.

PR Checklist

  • Is your PR title following our conventional commit naming recommendations?
  • Have you filled in the PR Description Template?
  • Is your branch up to date with the latest in main?
  • Do the CI checks pass successfully? (Lint fails by design until the demo console.log is removed)
  • Have you smoke tested the example applications?
  • Did you check that the changes meet accessibility standards?
  • Have you tested the application on these browsers?
    • Chrome (Fully supported)
    • Firefox (Major bug fixes supported)
    • Safari (Major bug fixes supported)

@lanesawyer
lanesawyer requested a review from a team as a code owner October 2, 2025 04:40

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made a trivial spacing change on this file to see if it would run it on the staged file and it did!! Woooo!

@froyo-np

froyo-np commented Oct 2, 2025

Copy link
Copy Markdown
Collaborator

So I disable husky pre-hook stuff locally - it does not play nice with git on windows. I'm agreeable to adding this, but I'd really really like to remove some (most?) of these lint rules.

@lanesawyer

Copy link
Copy Markdown
Collaborator Author

Sorry, could've made the intent here clearer. This is to auto-fix any fixable rules or formatting on commit, not to block commits if they fail a check.

Also I need to verify it's working how I expect! I forgot to test if it allows you to successfully commit with a linting error in the code.

@lanesawyer
lanesawyer marked this pull request as draft October 2, 2025 17:12
@lanesawyer

Copy link
Copy Markdown
Collaborator Author

Shoot, it does prevent commits if there are errors. Will look into this again later!

@lanesawyer
lanesawyer marked this pull request as ready for review February 19, 2026 05:36
@lanesawyer

Copy link
Copy Markdown
Collaborator Author

Finally got back to this and made it so it doesn't block any commits if there are lint errors happening while still doing any formatting or safe linting fixes! This is ready to go 😁

lanesawyer and others added 4 commits February 20, 2026 14:21
- Add lefthook.yml running oxlint --fix, oxfmt, and prettier on staged files
- Autofix only: the hook never blocks a commit, CI remains the gate
- Re-stage fixes via stage_fixed so commits record the formatted output
- Remove .husky/ and the biome-era lint-staged config
- Restore pnpm-workspace.yaml to match main (pnpm 11 scaffolding backed out)

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@lanesawyer lanesawyer changed the title chore: Husky and lint staged chore: replace husky and lint-staged with lefthook Aug 14, 2026
@lanesawyer lanesawyer changed the title chore: replace husky and lint-staged with lefthook chore: add lefthook pre-commit hooks Aug 14, 2026
lanesawyer and others added 5 commits August 13, 2026 21:09
These import sorts came from the branch's original Biome-based work and are
unrelated to the lefthook change. Restores both files to match main.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
oxfmt formats md/mdx/html and oxlint reads astro, but the globs listed
only the js/ts subset. A markdown- or astro-only commit skipped the hook
entirely and then failed CI, which is the exact case the hook exists to
prevent.

Also includes a deliberate console.log in site/src/examples/types.ts
demonstrating that the hook reformats the line and lets the commit
through while still reporting the lint error. Remove before merge.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Follow lefthook's documented pnpm setup instead of a custom prepare script:
allow its postinstall to run, which is what syncs the git hooks.

pnpm 11 reads this from pnpm-workspace.yaml's allowBuilds. The docs also
mention pnpm.onlyBuiltDependencies in package.json, but pnpm 11 ignores
that field and warns about it, so it is omitted.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@lanesawyer
lanesawyer changed the base branch from main to lane/upgrade-pnpm-11 August 14, 2026 04:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants