Skip to content

Front-end Linting#1165

Open
zandre-eng wants to merge 5 commits into
ze/frontend-linting-reformatfrom
ze/frontend-linting
Open

Front-end Linting#1165
zandre-eng wants to merge 5 commits into
ze/frontend-linting-reformatfrom
ze/frontend-linting

Conversation

@zandre-eng
Copy link
Copy Markdown
Contributor

@zandre-eng zandre-eng commented May 7, 2026

Product Description

This PR adds automated linting for JavaScript and Django HTML templates to the pre-commit pipeline. There are no user-facing changes.

Technical Summary

CCCT-2388

Adds two linters as pre-commit hooks alongside the existing Python linting:

  • ESLint (eslint:recommended) — lints commcare_connect/static/js/**/*.js. JS-in-HTML templates is intentionally excluded: Django template tags ({% %}, {{ }}) interpolated into <script> blocks cause parse errors. Existing JS files are grandfathered via an overrides block (rules downgraded to warn); new files get error-level rules, blocking commits.
  • djlint (Django profile) — lints and reformats Django HTML templates in commcare_connect/templates/. Runs as two hooks: djlint-reformat-django (auto-fixes indentation) then djlint-django (blocks on lint violations). A number of rules are suppressed — see [tool.djlint] in pyproject.toml for the full list. Notable suppressions: Two deferred cleanup items (H026: empty class attributes, H029: uppercase method="POST") that will be addressed in a bulk reformat PR.

As mentioned a separate PR here has been opened to do a bulk reformat of all our HTML code to bring it in line with the new linters.

For .eslintrc.js here is a summary breakdown of what each of the settings do:

  • env: Declares global variables that are pre-defined in those environments.
    • browser: true: Makes browser globals like window, document, and fetch available. Would otherwise flag these as no-undef errors.
    • es2020: true: Makes ES2020 globals like Promise and BigInt available. Would otherwise flag these as no-undef errors.
  • parserOptions: Controls how ESLint parses the JS.
    • ecmaVersion: 2020: Allows modern syntax like optional chaining and nullish coalescing.
    • sourceType: 'module': Enables ES module syntax (import/export).
  • extends: ['eslint:recommended']: Enables a curated set of rules that catch common bugs (e.g. no duplicate keys, no unreachable code).
  • rules: Project-wide overrides on top of recommended.
    • no-console: 'error': Disallows console.log etc.
    • no-unused-vars: Disallows declared but unused variables
  • overrides: Applies different rules to specific files. The grandfathered existing JS files all get rules set to warn instead of error, so that existing files won't break CI but will still surface issues as warnings.
  • ignorePatterns: Files ESLint skips entirely.

Safety Assurance

Safety story

Config-only change. No application code, models, views, or migrations are modified. The hooks only run at commit time and have no effect on the running application. The ESLint grandfathering strategy ensures existing code produces warnings only, so no commits are blocked on legacy violations.

This change was also tested locally by creating an HTML and JS file with linting errors and ensuring that commits were blocked for those files.

Automated test coverage

No automated tests — this change adds developer tooling with no runtime behaviour to test.

QA Plan

QA will not be performed for this change. Below is the testing plan for reference:

  • Run pre-commit run --all-files and confirm all hooks pass
  • Create a new JS file in commcare_connect/static/js/ with a console.log and an unused variable, stage it, and confirm ESLint blocks the commit with errors
  • Confirm that staging an existing grandfathered JS file (e.g. dashboard.js) produces warnings but does not block the commit
  • Create a Django template with a missing alt attribute on an <img>, stage it, and confirm djlint blocks the commit
  • Confirm that djlint-reformat auto-fixes indentation in staged templates before the lint check runs

Labels & Review

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

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 7, 2026

Review Change Stack

Walkthrough

This PR establishes JavaScript and Django template linting infrastructure by introducing ESLint and djLint tools. A new .eslintrc.js file defines ESLint rules enforcing no-console and no-unused-vars as errors globally, with relaxed rules for legacy static files. djLint configuration is added to pyproject.toml with django profile and specific rule ignores. Tool dependencies are added to both package.json (eslint@^8.57.1) and pyproject.toml (djlint>=1.36.4). Pre-commit hooks are registered in .pre-commit-config.yaml to run both linters automatically, with ESLint targeting only JavaScript files in commcare_connect/static/js/ and excluding the bundles directory.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title "Front-end Linting" clearly and concisely summarizes the main change: adding linting tools for front-end assets (ESLint and djlint) to the pre-commit pipeline.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed The PR description comprehensively explains the changes, technical approach, safety rationale, and testing plan, directly relating to the changeset of linting configuration files.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ze/frontend-linting

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (1)
.pre-commit-config.yaml (1)

27-31: 💤 Low value

djlint hooks use the old Riverside-Healthcare organisation URL.

The djLint documentation shows the canonical pre-commit repo as https://github.com/djlint/djLint, reflecting the project's GitHub organisation rename. GitHub currently redirects the old URL, but updating to the canonical form avoids any future broken-redirect surprises.

🔧 Proposed fix
-  - repo: https://github.com/Riverside-Healthcare/djLint
+  - repo: https://github.com/djlint/djLint
     rev: v1.36.4
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.pre-commit-config.yaml around lines 27 - 31, Update the pre-commit hook
repo URL in .pre-commit-config.yaml: replace the old organization URL string
"https://github.com/Riverside-Healthcare/djLint" with the canonical repository
"https://github.com/djlint/djLint" so the djlint hooks (ids
djlint-reformat-django and djlint-django) point to the upstream source and avoid
relying on GitHub redirects.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.eslintrc.js:
- Line 1: Add an ignorePatterns entry to the exported ESLint config object so
the config file itself is skipped by linting; update the object exported via
module.exports to include ignorePatterns (e.g., include ".eslintrc.js") to
prevent the no-undef false positive on module, or alternatively enable env.node
at the top of the exported config if you prefer to allow CommonJS in the config
file.

In `@package.json`:
- Line 18: package.json declares "eslint": "^8.57.1" but your pre-commit hook
lists additional_dependencies: eslint@8.56.0, causing a version mismatch; update
the .pre-commit-config.yaml additional_dependencies entry for eslint to match
the package.json minimum (e.g., change eslint@8.56.0 to eslint@8.57.1) so both
npm and pre-commit run the same ESLint version.

---

Nitpick comments:
In @.pre-commit-config.yaml:
- Around line 27-31: Update the pre-commit hook repo URL in
.pre-commit-config.yaml: replace the old organization URL string
"https://github.com/Riverside-Healthcare/djLint" with the canonical repository
"https://github.com/djlint/djLint" so the djlint hooks (ids
djlint-reformat-django and djlint-django) point to the upstream source and avoid
relying on GitHub redirects.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 8c4e9820-9b34-44b2-9bdf-33301c706668

📥 Commits

Reviewing files that changed from the base of the PR and between 0245f06 and c74858b.

⛔ Files ignored due to path filters (2)
  • package-lock.json is excluded by !**/package-lock.json
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (4)
  • .eslintrc.js
  • .pre-commit-config.yaml
  • package.json
  • pyproject.toml

Comment thread .eslintrc.js
Comment thread package.json
@zandre-eng zandre-eng force-pushed the ze/frontend-linting branch from c74858b to 0d42702 Compare May 7, 2026 12:42
@zandre-eng zandre-eng changed the base branch from main to ze/frontend-linting-reformat May 7, 2026 12:43
@zandre-eng zandre-eng force-pushed the ze/frontend-linting-reformat branch from 6b3ec61 to acef025 Compare May 7, 2026 13:04
@zandre-eng zandre-eng force-pushed the ze/frontend-linting branch from 41a568d to 32da122 Compare May 7, 2026 13:05
Copy link
Copy Markdown
Contributor

@mkangia mkangia left a comment

Choose a reason for hiding this comment

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

I don't have enough knowledge to review this but if you can share what each setting/file is specifically for, that would be helpful.

Comment thread .eslintrc.js
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you help understand the different settings added in this file, what they mean?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I have updated the PR description to give a breakdown summary of what each of the settings in eslintrc.js mean.

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.

5 participants