nextcloud / nextcloud/office

Pre-commit hook doesn't flag deleted build assets (js/css)

Open Beginner friendly
#132 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

0. Needs triage low
Dominant language
JavaScript
Stars
6
Forks
1
Avg merge
1d 3h
Merged PRs (30d)
10

Description

TL;DR: .githooks/pre-commit only checks staged files with --diff-filter=ACM (Added/Copied/Modified), never D (Deleted). Before #129, local builds never deleted anything under js//css/ (fixed js/ filenames, css/ only grew), so the gap was invisible. After #129 (which empties css/ before each build to stop stale hashed chunks from accumulating), a local npm run build && git commit -am ... will silently commit CSS deletions — the hook stays quiet, and CI's npm-build "check build changes" job even goes green on that commit since the output matches.

Detail
  • .githooks/pre-commit line ~14: git diff --cached --name-only --diff-filter=ACM -- js/ css/ (or equivalent) never surfaces deleted paths, so the hook's "you're about to commit generated build assets" guard silently misses deletions.
  • This repo's intended flow (per AGENTS.md) is: contributors run npm run build locally to test, then git restore --staged js css before committing — the hook is meant as a backstop for anyone who forgets. That backstop has a hole specifically for deletions.
  • Suggested fix: include D in the --diff-filter (e.g. --diff-filter=ACMD), so a staged deletion under js//css/ trips the same warning as an addition/modification.
  • Found during review of #129 (build: remove outdated CSS files when building).

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with .githooks/pre-commit around line 14 and read AGENTS.md for the intended build and staging flow. Run npm run build, stage a deletion under js/ or css/, and verify the hook warns about it; done means deleted build assets trigger the same guard as additions and modifications.

Written by the indexing model from the issue text.

Assessment

Tech stack
git, javascript
Domain
build-system, tooling
Issue type
Bug
Difficulty
1/5
Estimated time
Under an hour
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
90/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.