Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 69 additions & 4 deletions .github/scripts/frontend_checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
"""Automated frontend checks for CourtListener PR reviews.

Runs checks on changed template and CSS files to enforce frontend
conventions. Called by the frontend-lint GitHub Actions workflow.
conventions, and checks that changes to vendored JS are recorded in that
directory's README. Called by the frontend-lint GitHub Actions workflow.

Input file must be in git --name-status format (STATUS\\tPATH per line).
"""
Expand Down Expand Up @@ -120,6 +121,24 @@ def is_input_css(path: str) -> bool:
return path == "cl/assets/tailwind/input.css"


# ---------------------------------------------------------------------------
# Vendored JS directories
# ---------------------------------------------------------------------------

# Each directory holds upstream code and a README listing every package
# vendored there, with its version and source.
VENDORED_JS_DIRS = (
"cl/assets/static-global/js/third_party/",
"cl/assets/static-global/js/alpine/",
)

# Our own code inside a vendored directory; changing it needs no README entry.
VENDORED_JS_OWN_CODE = (
"cl/assets/static-global/js/alpine/components/",
"cl/assets/static-global/js/alpine/composables/",
)


# ---------------------------------------------------------------------------
# Multiline tag helper
# ---------------------------------------------------------------------------
Expand Down Expand Up @@ -964,6 +983,46 @@ def run_checks(
return findings


def check_vendored_js_readme(diff_paths: list[str]) -> list[Finding]:
"""Fail when upstream files in a vendored JS directory change but that
directory's README doesn't.

``diff_paths`` is every path in the diff, including the old side of a
rename, so moving a file into or out of a directory counts for both.
"""
findings = []
for vendored_dir in VENDORED_JS_DIRS:
readme = f"{vendored_dir}README.md"
upstream_changes = sorted(
{
path
for path in diff_paths
if path.startswith(vendored_dir)
and path != readme
and not path.startswith(VENDORED_JS_OWN_CODE)
}
)
if not upstream_changes or readme in diff_paths:
continue
shown = ", ".join(
path.removeprefix(vendored_dir) for path in upstream_changes[:3]
)
if len(upstream_changes) > 3:
shown += f" and {len(upstream_changes) - 3} more"
findings.append(
Finding(
readme,
1,
"check_vendored_js_readme",
FAIL,
f"Upstream files in {vendored_dir} changed ({shown}) but "
f"{readme} did not — record each package's version and "
"source there",
)
)
return findings


def _swap_template_prefix(path: str, *, add_v2: bool) -> str | None:
"""Swap between legacy and v2_ template paths.

Expand Down Expand Up @@ -1115,6 +1174,7 @@ def main() -> int:
raw = Path(args.changed_files).read_text()

changed_files = []
diff_paths: list[str] = []
file_statuses: dict[str, str] = {}
for line in raw.splitlines():
line = line.strip()
Expand All @@ -1131,6 +1191,10 @@ def main() -> int:
path = parts[-1]
file_statuses[path] = status
changed_files.append(path)
diff_paths.append(path)
# A rename also removes its old path; a copy leaves it untouched.
if status.startswith("R") and len(parts) == 3:
diff_paths.append(parts[1])

# Filter to relevant files
relevant = [
Expand All @@ -1140,10 +1204,11 @@ def main() -> int:
and not any(fnmatch.fnmatch(f, g) for g in args.skip_files)
]

if not relevant:
findings = check_vendored_js_readme(diff_paths)
if not relevant and not findings:
return 0

findings = run_checks(relevant, repo_root, file_statuses)
if relevant:
findings += run_checks(relevant, repo_root, file_statuses)

if not findings:
print("All frontend checks passed.")
Expand Down
123 changes: 123 additions & 0 deletions .github/scripts/test_frontend_checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,13 @@

from __future__ import annotations

import contextlib
import io
import tempfile
import textwrap
import unittest
from pathlib import Path
from unittest import mock

import frontend_checks

Expand Down Expand Up @@ -266,5 +269,125 @@ def test_modified_v2_template_needs_no_registration(self) -> None:
self.assertEqual(findings, [])


THIRD_PARTY = "cl/assets/static-global/js/third_party/"
ALPINE = "cl/assets/static-global/js/alpine/"


def _vendored_findings(diff_paths: list[str]) -> list[tuple[str, str]]:
"""``(file, check)`` per finding from the vendored-README check."""
return [
(f.file, f.check)
for f in frontend_checks.check_vendored_js_readme(diff_paths)
]


class VendoredJsReadmeTest(unittest.TestCase):
"""Upstream JS changes must come with an update to that directory's README."""

def test_upstream_change_without_readme_fails(self) -> None:
"""A changed upstream file with no README change is a FAIL."""
findings = frontend_checks.check_vendored_js_readme(
[f"{THIRD_PARTY}htmx.js"]
)
self.assertEqual(
[(f.file, f.severity) for f in findings],
[(f"{THIRD_PARTY}README.md", frontend_checks.FAIL)],
)

def test_upstream_change_with_readme_passes(self) -> None:
"""Touching the same directory's README is enough."""
self.assertEqual(
_vendored_findings(
[f"{THIRD_PARTY}htmx.js", f"{THIRD_PARTY}README.md"]
),
[],
)

def test_nested_upstream_file_counts(self) -> None:
"""Files in subdirectories, like flatpickr plugins, count too."""
self.assertEqual(
_vendored_findings(
[f"{THIRD_PARTY}flatpickr/plugins/confirmDate.js"]
),
[(f"{THIRD_PARTY}README.md", "check_vendored_js_readme")],
)

def test_each_directory_needs_its_own_readme(self) -> None:
"""The other directory's README doesn't cover a change."""
self.assertEqual(
_vendored_findings(
[f"{ALPINE}alpinejscsp.js", f"{THIRD_PARTY}README.md"]
),
[(f"{ALPINE}README.md", "check_vendored_js_readme")],
)

def test_alpine_plugins_are_upstream(self) -> None:
"""Official Alpine plugins live in the vendored directory."""
self.assertEqual(
_vendored_findings([f"{ALPINE}plugins/focus.js"]),
[(f"{ALPINE}README.md", "check_vendored_js_readme")],
)

def test_our_alpine_code_is_ignored(self) -> None:
"""components/ and composables/ are our code, not upstream."""
self.assertEqual(
_vendored_findings(
[
f"{ALPINE}components/date_selector.js",
f"{ALPINE}composables/focus_trap.js",
]
),
[],
)

def test_readme_only_change_passes(self) -> None:
"""Editing just the README is fine."""
self.assertEqual(_vendored_findings([f"{ALPINE}README.md"]), [])

def test_other_js_is_out_of_scope(self) -> None:
"""Legacy scripts outside the two directories are never checked."""
self.assertEqual(
_vendored_findings(["cl/assets/static-global/js/base.js"]), []
)


class VendoredJsReadmeMainTest(unittest.TestCase):
"""``main()`` reads renames from ``--name-status`` and runs the check
even when no template or CSS file changed."""

def _main(self, name_status: str) -> int:
with tempfile.TemporaryDirectory() as tmp:
changed = Path(tmp) / "changed_files.txt"
changed.write_text(name_status, encoding="utf-8")
argv = ["frontend_checks.py", "--repo-root", tmp]
argv += ["--changed-files", str(changed)]
with (
mock.patch("sys.argv", argv),
contextlib.redirect_stdout(io.StringIO()),
):
return frontend_checks.main()

def test_rename_inside_vendored_dir_fails(self) -> None:
"""An R100 rename with no README change makes the job fail."""
old = f"{THIRD_PARTY}flatpickr/flatpickr@4.6.13.js"
new = f"{THIRD_PARTY}flatpickr/flatpickr.js"
self.assertEqual(self._main(f"R100\t{old}\t{new}\n"), 1)

def test_moving_a_file_out_counts_for_the_old_directory(self) -> None:
"""The old side of a rename is checked as well as the new one."""
old = f"{THIRD_PARTY}htmx.js"
self.assertEqual(
self._main(f"R100\t{old}\tcl/assets/static-global/js/htmx.js\n"),
1,
)

def test_rename_with_readme_passes(self) -> None:
"""The same rename plus a README change exits cleanly."""
old = f"{THIRD_PARTY}flatpickr/flatpickr@4.6.13.js"
new = f"{THIRD_PARTY}flatpickr/flatpickr.js"
name_status = f"R100\t{old}\t{new}\nM\t{THIRD_PARTY}README.md\n"
self.assertEqual(self._main(name_status), 0)


if __name__ == "__main__":
unittest.main()
2 changes: 2 additions & 0 deletions .github/workflows/frontend-lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ on:
paths:
- '**/*.html'
- 'cl/assets/tailwind/input.css'
- 'cl/assets/static-global/js/third_party/**'
- 'cl/assets/static-global/js/alpine/**'

permissions:
contents: read
Expand Down
13 changes: 13 additions & 0 deletions cl/assets/static-global/js/third_party/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,16 @@ Current version: **2.0.11**
| `htmx.min.js` | https://cdn.jsdelivr.net/npm/htmx.org@2.0.11/dist/htmx.min.js |

Legacy templates run htmx 1.7.0 from `/js/` and are not to be upgraded; that copy retires with legacy.

## flatpickr

Current version: **4.6.13**

| File | CDN URL |
|------|---------|
| `flatpickr/flatpickr.js` | https://cdn.jsdelivr.net/npm/flatpickr@4.6.13/dist/flatpickr.js |
| `flatpickr/flatpickr.min.js` | https://cdn.jsdelivr.net/npm/flatpickr@4.6.13/dist/flatpickr.min.js |
| `flatpickr/plugins/confirmDate.js` | https://cdn.jsdelivr.net/npm/flatpickr@4.6.13/dist/plugins/confirmDate/confirmDate.js |
| `flatpickr/plugins/confirmDate.min.js` | https://cdn.jsdelivr.net/npm/flatpickr@4.6.13/dist/plugins/confirmDate/confirmDate.min.js |

The unminified files in this repo have been reformatted by Prettier; the code is unchanged.
2 changes: 1 addition & 1 deletion cl/opinion_page/templates/cotton/docket_filter/index.html
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{% load component_tags svg_tags %}
{% require_script "js/alpine/components/docket_filter.js" %}
{% require_script "js/third_party/flatpickr/flatpickr@4.6.13" %}
{% require_script "js/third_party/flatpickr/flatpickr" %}
{# frontend-checks-skip: check_hardcoded_ids #}

<c-vars
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{% load svg_tags component_tags %}
{% require_script "js/third_party/flatpickr/flatpickr@4.6.13" %}
{% require_script "js/third_party/flatpickr/plugins/confirmDate@4.6.13" %}
{% require_script "js/third_party/flatpickr/flatpickr" %}
{% require_script "js/third_party/flatpickr/plugins/confirmDate" %}
{% require_script "js/alpine/components/date_selector.js" %}
{% require_script "js/alpine/plugins/focus" defer=True %}

Expand Down
Loading