Skip to content

refactor: replace moment.js with luxon and remove the bundled library#4937

Draft
SteveGilvarry wants to merge 1 commit into
ZoneMinder:masterfrom
SteveGilvarry:replace-moment-with-luxon
Draft

refactor: replace moment.js with luxon and remove the bundled library#4937
SteveGilvarry wants to merge 1 commit into
ZoneMinder:masterfrom
SteveGilvarry:replace-moment-with-luxon

Conversation

@SteveGilvarry

Copy link
Copy Markdown
Member

Draft — not ready for merge. The library swap is done and conversions are parity-verified in node, but each impacted UI area still needs manual/browser testing (checklist below).

Summary

luxon is already bundled and loaded globally (functions.php), so the remaining moment.js call sites are migrated to it and the library is removed (moment.js, moment.min.js, moment.js.URL, and the <script> include). Resolves the moment.js CodeQL alert.

Changes

  • skin.js convertLabelFormat — strftime tokens now map to luxon tokens (YYYYyyyy, DDdd, Aa, DDDDooo, …). The %N monitor name is emitted as a luxon literal (each char single-quoted, '' for an apostrophe) so token characters and apostrophes in names survive.
  • skin.js addVideoTimingTrack — parses StartDateTime with DateTime.fromSQL (ISO fallback) and advances via the immutable plus({seconds:1}). Removed the dead commented WebVTT block that also used moment.
  • montagereview.jssecs2inputstr/secs2dbstr use DateTime.fromMillis().toFormat(); filter date normalisation uses fromISO with a fromSQL fallback.
  • report_event_audit.js — min/max inputs parsed via a tolerant fromISO/fromSQL helper and compared as luxon DateTimes.

Chart.js (report.php) only uses its category axis, not the moment-backed time scale, so it is unaffected.

Verification done

  • eslint clean on all three JS files; php -l clean; node --check clean.
  • Parity test in node (moment vs luxon, same inputs): byte-identical output for epoch→format, YYYY-MM-DD HH:mm:ss parse, and label-format conversion across representative formats — including the apostrophe-name edge case (which caught and fixed a %N escaping bug).

Manual testing checklist (each impacted area)

  • Event video playback — the timing/subtitle track labels render correctly for several monitor Label Format strings (date, 12h %p, weekday %a, %N name) and for a monitor whose name contains an apostrophe.
  • Montage Review — time scrubber/inputs populate correctly (secs2inputstr/secs2dbstr), and applying a date/time filter builds the correct API URL (YYYY-MM-DD HH:mm:ss).
  • Report → Event Audit — entering min > max flags the error and blocks submit; a valid range submits.
  • No regressions from the removed global — spot-check console on montage/event/report pages for any moment is not defined.

luxon is already bundled and loaded globally, so the remaining moment.js
usages are migrated to it and the library is removed.

- skin.js convertLabelFormat: map strftime tokens to luxon format tokens
  (YYYY->yyyy, DD->dd, A->a, ...) and emit the %N monitor name as a luxon
  literal (each char single-quoted, '' for an apostrophe) so token
  characters and apostrophes in names survive.
- skin.js addVideoTimingTrack: parse StartDateTime with
  DateTime.fromSQL (ISO fallback) and advance with the immutable
  plus({seconds:1}); drop the dead commented WebVTT block that also used
  moment.
- montagereview.js secs2inputstr/secs2dbstr: DateTime.fromMillis().toFormat();
  filter normalisation: fromISO with fromSQL fallback.
- report_event_audit.js: parse min/max inputs via a tolerant
  fromISO/fromSQL helper and compare luxon DateTimes.
- functions.php: drop the moment.min.js script tag.
- remove web/skins/classic/js/moment.js, moment.min.js, moment.js.URL.

Chart.js (report.php) only uses its category axis, not the moment-backed
time scale, so it is unaffected. Resolves the moment.js CodeQL alert.

Parity verified in node: luxon output is byte-identical to moment for the
epoch/format, parse and label-format conversions (incl. apostrophe names).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.

1 participant