chore(eslint): fix @eslint-react/purity warning — move new Date() to useMemo (#1471)

[steilerDev]

Summary

Fixes #1471
Part of #1455

Problem

The @eslint-react/purity rule was flagging new Date() called directly in the render body of CriticalPathCard. Date construction via new Date() is non-deterministic (reads the system clock) and is considered an impure operation in a React component's render phase.

Fix

Moved the new Date() call (and surrounding date arithmetic) into a useMemo hook in CriticalPathCard:

// BEFORE (flagged — new Date() in render body)
const today = new Date();
today.setHours(0, 0, 0, 0);
let daysRemaining = 0;
if (deadline) { ... }

// AFTER — moved into useMemo
const daysRemaining = useMemo(() => {
  if (!deadline) return 0;
  const today = new Date();
  today.setHours(0, 0, 0, 0);
  ...
  return Math.ceil(diff / (1000 * 60 * 60 * 24));
}, [deadline]);

Structural Change

Because React hooks must be called unconditionally before any early returns, the component was restructured to hoist all derived state computation (incompleteCritical, nextItem, deadline) and the useMemo above the two guard early-return paths. Functional behaviour is identical.

Verification

Confirmed 0 @eslint-react/purity violations across all client/src files after the fix.

Files Changed

• client/src/components/TimelineStatusCards/CriticalPathCard.tsx — hoist hooks, wrap new Date() in useMemo

[steilerDev]

[frontend-developer PR Review]

Verdict: ✅ APPROVED

React Hook Rules ✅

• useMemo called unconditionally before early returns
• Correct import and placement at component top level
• No conditional hook calls

Dependency Array ✅

• Complete and correct: [deadline] is the only external dependency
• deadline is captured and used in the callback
• No stale closures — all internal variables are local to the callback

Functional Equivalence ✅

• Before: daysRemaining initialized as 0, conditionally set if deadline exists
• After: useMemo returns 0 if !deadline, otherwise performs identical calculation
• Type improved: const instead of let (safer semantics)

Early Returns & Hook Discipline ✅

All derived state hoisted unconditionally above guard checks:

• incompleteCritical ← filtered/sorted before returns
• nextItem ← derived before returns
• deadline ← extracted before returns
• daysRemaining ← computed via useMemo before returns

Guard condition correctly updated: if (!nextItem) replaces if (incompleteCritical.length === 0)

Performance & Re-renders ✅

• useMemo dependency on deadline prevents unnecessary re-computation across renders
• No regression in re-render frequency — all state changes trigger appropriate updates

Code Quality

• Addresses ESLint @eslint-react-purity violation correctly
• Moves impure new Date() into deterministic useMemo boundary
• Type safety improved with ?? over !

Ready to merge. No concerns.

[steilerDev]

[ux-designer]

Design Review — Visual Parity Verification ✅

This is a logic-only refactor. I performed a line-by-line comparison of the rendered JSX tree (before and after) and verified that the visual output is completely unchanged.

Structural / JSX analysis

The entire return tree (both early-return paths and the full card) is byte-for-byte identical between main and the PR branch. Only the position and wrapping of the computation logic changed.

Concern	Finding
daysRemaining value	Identical computation — same arithmetic, same Math.ceil, same midnight normalisation. Wrapped in useMemo([deadline]) instead of an inline if block; result is the same.
incompleteCritical / nextItem logic	Moved above the first early-return; semantically equivalent. Old code only reached this block when criticalItems.length > 0, and the new code computes it unconditionally but the result is unused if the early return fires — no observable difference.
nextItem null safety	Old: incompleteCritical[0]! (non-null assertion after length guard). New: incompleteCritical[0] ?? null. The !nextItem guard that follows is equivalent to the old incompleteCritical.length === 0 guard.
deadline type change	Old: string | undefined (from nextItem.endDate). New: string | null (via ?? null). formatDate(deadline) handles both null and undefined identically (returns '—') — verified in formatters.test.ts.
daysRemaining when no deadline	Old: initialised to 0, stays 0 if no deadline. New: useMemo returns 0 when !deadline. Same value.
Health badge colours / labels	Driven entirely by the same daysRemaining thresholds (< 0, < 7, <= 14). Unchanged.
Dark mode / responsive behaviour	No CSS, no style props, no class names were modified. Dark mode and responsive behaviour are entirely unaffected.
Hooks-before-returns rule	The refactor correctly moves useMemo above both early returns, fixing the ESLint @eslint-react/purity violation without altering rendered output.

Verdict

All rendered paths produce identical output. The refactor is a correct fix for the hooks-before-early-returns rule. APPROVED (self-authored PR — comment used instead of formal approve).

[steilerDev]

[dev-team-lead]

Code Review — CriticalPathCard.tsx ESLint purity fix

VERDICT: APPROVED ✅

Summary: This is a clean, correct structural refactor. The new Date() call is properly wrapped in useMemo and all derived state is hoisted above early returns to satisfy Rules of Hooks. Functional behaviour is preserved.

---

Hooks / Rules of Hooks ✅

• useMemo is now called unconditionally before any early returns — Rules of Hooks satisfied.
• @eslint-react/purity warning resolved: new Date() no longer exists in the raw render body.

Functional Equivalence ✅

• Empty critical path (criticalItems.length === 0): hoisted filter/sort on an empty array returns [], nextItem becomes null, deadline becomes null, useMemo short-circuits to 0 — then the early return fires. Identical output.
• All completed (!nextItem): incompleteCritical[0] ?? null gives null exactly when the original incompleteCritical.length === 0 check was true — equivalent guard.
• Normal render path: computation of daysRemaining is byte-for-byte identical to the original logic; only its placement and useMemo wrapper differ.

useMemo dependency array ✅

• [deadline] is correct and complete. today is computed inside the memo body (no stale closure risk). deadline is a string | null derived from props — it changes exactly when a recompute is needed.

Test compatibility ✅

• Existing tests use jest.setSystemTime() with fake timers. new Date() inside useMemo correctly picks up the mocked system time — no test changes needed and the test file is untouched.

No regressions or new warnings identified.

[steilerDev]

[product-architect] Architecture review — APPROVED ✅ (posted as comment because GitHub blocks self-approval on this PR)

Scope is narrowly focused on the @eslint-react/purity warning in a single component, with no schema, API contract, or cross-cutting impact.

Findings

Hook correctness ✅

• useMemo is hoisted above both guard early returns (criticalItems.length === 0 and !nextItem), so all hooks are called unconditionally on every render. Rules of Hooks honored.
• Dependency array [deadline] is correct — it's the sole reactive input to the computation. today is intentionally recomputed inside the memo body and only stabilizes when deadline changes, which is acceptable for a "days until deadline" display that doesn't need to tick in real time.

Behavioral parity ✅

• incompleteCritical[0] ?? null + if (!nextItem) cleanly replaces the post-guard ! non-null assertion that the previous code needed. Same semantics, less unsafe.
• daysRemaining defaults to 0 when deadline is null inside the memo, matching the original let daysRemaining = 0 initialization. Health-class branching downstream is unchanged.

Pattern consistency ✅

• client/src/components/GanttChart/GanttChart.tsx:183 already uses const today = useMemo(() => new Date(), []) — this PR extends an established pattern rather than inventing a new one.
• Sibling timeline cards (UpcomingMilestonesCard, WorkItemProgressCard) don't construct Date objects in the render body, so there's no drift to harmonize.

Wiki / ADR — none required. This is internal refactoring of a single component; no architectural decision is being made or changed.

Informational (not blocking)

• criticalItems on line 18 is still recomputed each render. It's a pure derivation so not a purity-rule violation, and out of scope for this PR, but worth noting if this card ever shows up in profiling.

CI signal: Static Analysis and all 6 test shards green. No concerns from an architecture standpoint — safe to merge once other required reviews land.

[steilerDev]

[orchestrator] Consolidated /review-pr 1640 verdict: ✅ APPROVED (all 4 reviewers)

Reviewer	Verdict	Highlights
dev-team-lead	APPROVED	Hooks compliance ✅, functional equivalence ✅, useMemo deps correct ✅, test compatibility ✅ (fake timers respected), no new ESLint warnings
product-architect	APPROVED	Hook ordering correct, behavioral parity preserved, pattern consistent with GanttChart.tsx:183 which already uses useMemo(() => new Date(), []) — extends an established pattern, no architectural drift
frontend-developer	APPROVED	React hook rules ✅, deps [deadline] complete, no stale closures, no re-render regressions; let → const is a type-safety upgrade
ux-designer	APPROVED	JSX tree byte-for-byte identical; thresholds (<0, <7, <=14) + badge classes unchanged; dark mode / responsive unaffected; deadline: string | undefined → string | null is benign (formatDate handles both)

Informational (non-blocking)

• product-architect noted: criticalItems is still computed unmemoized in the render body. Pure derivation, not a purity violation — out of scope for this PR.

Status

• Retargeted: main → beta (was incorrectly targeting main).
• Mergeability: MERGEABLE against beta.
• Blocked on CI infra: Quality Gates is failing for systemic reasons (E2E Cache Warmup hits the 6h job timeout because playwright install hangs on cache-miss). WP-0 is opening a fix (fix/ci-playwright-install-timeout). This PR is agent-review-clean and will be merged once that infra fix lands on beta.

Closes #1471 · Part of #1455