[Pre-commit] Improve Cross-Platform Support, Maintainability & Performance

[coderabbitai]

Summary

This issue tracks improvements for cross-platform compatibility, maintainability, and performance of the Husky pre-commit script in the develop branch. These enhancements will improve developer experience and script efficiency across different operating systems.

Related

• Part 1: [Pre-commit] Fix Critical Issues and Improve Code Quality #6317 (Critical Issues & Code Quality fixes)

---

4. 🖥️ CROSS-PLATFORM COMPATIBILITY

Issue 4.1: Missing Windows Support

Severity: MEDIUM
Lines: 60-66

Description:
Windows developers using Git Bash will hit the "else" branch, which may not work correctly.

Current Code:

if [ "$(uname)" = "Darwin" ]; then
  FILE_MOD_TIME=$(stat -f %m "$SCRIPT_PATH")
else
  FILE_MOD_TIME=$(stat -c %Y "$SCRIPT_PATH")
fi

Proposed Fix:

-if [ "$(uname)" = "Darwin" ]; then
-  FILE_MOD_TIME=$(stat -f %m "$SCRIPT_PATH")
-else
-  FILE_MOD_TIME=$(stat -c %Y "$SCRIPT_PATH")
-fi
+# Detect OS more robustly
+OS_TYPE=$(uname -s)
+case "$OS_TYPE" in
+  Darwin*)
+    # macOS
+    FILE_MOD_TIME=$(stat -f %m "$SCRIPT_PATH" 2>/dev/null)
+    ;;
+  Linux*)
+    # Linux
+    FILE_MOD_TIME=$(stat -c %Y "$SCRIPT_PATH" 2>/dev/null)
+    ;;
+  MINGW*|MSYS*|CYGWIN*)
+    # Windows (Git Bash)
+    FILE_MOD_TIME=$(stat -c %Y "$SCRIPT_PATH" 2>/dev/null)
+    ;;
+  *)
+    echo "⚠️  Unsupported OS: $OS_TYPE, cache disabled"
+    FILE_MOD_TIME=""
+    ;;
+esac

---

Issue 4.2: date +%s Not Available on Old Systems

Severity: LOW
Line: 73

Description:
Some older Unix systems or minimal Docker images might not support date +%s.

Current Code:

CURRENT_TIME=$(date +%s)

Proposed Fix:

-CURRENT_TIME=$(date +%s)
+# Use Python as fallback (you already have Python in venv)
+CURRENT_TIME=$(date +%s 2>/dev/null || python3 -c "import time; print(int(time.time()))")

---

5. 🧹 MAINTAINABILITY

Issue 5.1: Magic Numbers Without Context

Severity: LOW
Lines: 46, 54

Description:
Why 600 lines? Why 24 hours? Future maintainers won't know.

Current Code:

"$VENV_BIN" .github/workflows/scripts/countline.py --lines 600 --files ...
CACHE_MAX_AGE_HOURS=24

Proposed Fix:

+# Define constants at the top with comments
+MAX_FILE_LINES=600  # Maximum lines per file before triggering complexity warning
+SCRIPT_CACHE_HOURS=24  # Cache external scripts for 24h to reduce network dependency
+
 # Later in the script:
-"$VENV_BIN" .github/workflows/scripts/countline.py --lines 600 --files ...
-CACHE_MAX_AGE_HOURS=24
+"$VENV_BIN" .github/workflows/scripts/countline.py --lines "$MAX_FILE_LINES" --files ...
+CACHE_MAX_AGE_HOURS="$SCRIPT_CACHE_HOURS"

---

Issue 5.2: Repeated Pattern for Staged File Filtering

Severity: LOW
Lines: 7-9, 143-147

Description:
Duplicated logic is harder to maintain. Changes need to be made in multiple places.

Current Code:

# Pattern repeated twice with slight variations
STAGED_SRC=$(git diff --cached --name-only --diff-filter=ACMRT ...)
CSS_STAGED=$(git diff --cached --name-only --diff-filter=ACMRT ...)

Proposed Fix:

+# Extract into a function
+get_staged_files() {
+  local pattern="$1"
+  local exclude_patterns="${2:-}"
+  
+  local files=$(git diff --cached --name-only --diff-filter=ACMRT)
+  
+  if [ -n "$pattern" ]; then
+    files=$(echo "$files" | grep -E "$pattern" || true)
+  fi
+  
+  if [ -n "$exclude_patterns" ]; then
+    for exclude in $exclude_patterns; do
+      files=$(echo "$files" | grep -v "$exclude" || true)
+    done
+  fi
+  
+  echo "$files"
+}
+
 # Usage:
-STAGED_SRC=$(git diff --cached --name-only --diff-filter=ACMRT | grep -E '^(src|scripts)/.*\.(ts|tsx|js|jsx)$' | tr '\n' ' ')
-CSS_STAGED=$(git diff --cached --name-only --diff-filter=ACMRT | grep -v '^src/utils/' | grep -v '^src/types/' || true)
+STAGED_SRC=$(get_staged_files '^(src|scripts)/.*\.(ts|tsx|js|jsx)$')
+CSS_STAGED=$(get_staged_files '' '^src/utils/ ^src/types/')

---

Issue 5.3: Insufficient Inline Documentation

Severity: LOW
Lines: Various

Description:
Complex logic (especially the caching mechanism) lacks comments explaining the "why" behind decisions.

Proposed Fix:

+# =============================================================================
+# Talawa Admin Pre-Commit Hook
+# =============================================================================
+# This hook runs multiple validation checks before allowing commits:
+# 1. Code formatting (Prettier, Black)
+# 2. Linting (ESLint, flake8, pydocstyle)
+# 3. Type checking (TypeScript)
+# 4. Translation file validation
+# 5. Security checks (MinIO compliance, disable statements)
+# 6. Documentation freshness (ToC, API docs)
+#
+# External Script Caching:
+# - Centralized Python scripts are downloaded from PalisadoesFoundation/.github
+# - Cached locally for 24h to reduce network dependency
+# - Falls back to stale cache if offline
+# =============================================================================
 #!/usr/bin/env sh

---

6. ⚡ PERFORMANCE

Issue 6.1: Sequential Execution - No Parallelization

Severity: MEDIUM
Lines: 2-24

Description:
Independent checks run one after another. On large codebases, this wastes time.

Current Code:

pnpm run generate-docs
pnpm run format:fix
pnpm run lint-staged
pnpm run typecheck
# ... all run sequentially

Proposed Fix:

-pnpm run generate-docs
+# Run independent checks in parallel using background jobs
+pnpm run generate-docs &
+PID_DOCS=$!
 
-pnpm run typecheck
+pnpm run typecheck &
+PID_TYPECHECK=$!
+
+# Wait for parallel jobs to complete
+wait $PID_DOCS || { echo "❌ Documentation generation failed"; exit 1; }
+wait $PID_TYPECHECK || { echo "❌ Type checking failed"; exit 1; }
 
+# Formatting and linting need to run sequentially (they modify files)
 pnpm run format:fix || exit 1
 pnpm run lint-staged || exit 1

---

Issue 6.2: Redundant git diff Calls

Severity: LOW
Lines: 7, 143

Description:
Calling git diff twice when you could call it once and filter differently.

Current Code:

STAGED_SRC=$(git diff --cached --name-only --diff-filter=ACMRT | ...)
# ... later ...
CSS_STAGED=$(git diff --cached --name-only --diff-filter=ACMRT | ...)

Proposed Fix:

-STAGED_SRC=$(git diff --cached --name-only --diff-filter=ACMRT | grep -E '^(src|scripts)/.*\.(ts|tsx|js|jsx)$' | tr '\n' ' ')
+# Get all staged files once
+ALL_STAGED=$(git diff --cached --name-only --diff-filter=ACMRT)
+
+# Filter for different purposes
+STAGED_SRC=$(echo "$ALL_STAGED" | grep -E '^(src|scripts)/.*\.(ts|tsx|js|jsx)$' | tr '\n' ' ')
 # ... later ...
-CSS_STAGED=$(git diff --cached --name-only --diff-filter=ACMRT | grep -v '^src/utils/' | grep -v '^src/types/' || true)
+CSS_STAGED=$(echo "$ALL_STAGED" | grep -v '^src/utils/' | grep -v '^src/types/' || true)

---

Issue 6.3: Knip Runs Twice Separately

Severity: LOW
Lines: 22-23

Description:
Knip startup overhead happens twice. Could potentially combine if possible.

Current Code:

npx knip --include files,exports,nsExports,nsTypes
npx knip --config knip.deps.json --include dependencies

Proposed Fix:

-npx knip --include files,exports,nsExports,nsTypes
-npx knip --config knip.deps.json --include dependencies
+# Run knip commands in parallel to reduce startup overhead
+npx knip --include files,exports,nsExports,nsTypes &
+PID_KNIP1=$!
+npx knip --config knip.deps.json --include dependencies &
+PID_KNIP2=$!
+
+wait $PID_KNIP1 || exit 1
+wait $PID_KNIP2 || exit 1

---

📋 Implementation Checklist

Cross-Platform Compatibility

• Issue 4.1: Add robust OS detection with Windows support
• Issue 4.2: Add Python fallback for date +%s

Maintainability

• Issue 5.1: Move magic numbers to named constants with comments
• Issue 5.2: Extract repeated staged file filtering into function
• Issue 5.3: Add comprehensive header documentation

Performance

• Issue 6.1: Parallelize independent checks (docs generation, type checking)
• Issue 6.2: Call git diff once and reuse results
• Issue 6.3: Run knip commands in parallel

---

🧪 Testing Checklist

macOS Testing

• Pre-commit hook runs with improved performance
• OS detection works correctly
• All parallelized checks complete successfully
• Cache mechanism works correctly

Linux Testing

• Pre-commit hook runs with improved performance
• OS detection works correctly
• All parallelized checks complete successfully
• Cache mechanism works correctly

Windows Testing

• Pre-commit hook runs with improved performance
• OS detection correctly identifies Windows (MINGW/MSYS/CYGWIN)
• stat command works on Windows Git Bash
• date +%s fallback works if needed
• All parallelized checks complete successfully
• Cache mechanism works correctly on Windows

Performance Verification

• Measure pre-commit execution time before changes
• Measure pre-commit execution time after parallelization
• Verify at least 20-30% improvement in execution time
• Confirm no race conditions in parallel execution

Edge Cases

• Test on minimal Docker images (date fallback)
• Test with slow network (parallel execution resilience)
• Test documentation clarity for new contributors