kimai-plugin-heatmap/.planning/phases/06-renderer-architecture/06-REVIEW.md

phase	reviewed	depth	files_reviewed	files_reviewed_list	findings	status
06-renderer-architecture	2026-04-09T14:00:00Z	standard	10	assets/src/heatmap.ts assets/src/renderers/registry.ts assets/src/renderers/types.ts assets/src/renderers/year.ts assets/src/shared/color-scale.ts assets/src/shared/date-utils.ts assets/src/shared/stats.ts assets/src/shared/tooltip.ts assets/src/state.ts assets/src/types.ts	critical warning info total 0 3 2 5	issues_found

Phase 06: Code Review Report

Reviewed: 2026-04-09T14:00:00Z Depth: standard Files Reviewed: 10 Status: issues_found

Summary

The renderer architecture refactor is clean and well-structured. The extraction of shared utilities (tooltip, color-scale, stats, date-utils) into focused modules is solid, and the renderer registry pattern is simple and appropriate. No security vulnerabilities or critical bugs found. The issues below are defensive coding gaps and a minor cleanup item.

Warnings

WR-01: Unchecked JSON.parse can crash init

File: assets/src/heatmap.ts:21 Issue: JSON.parse(projectsJson) will throw a SyntaxError if the data-projects attribute contains malformed JSON. This would crash the entire init() function, leaving the heatmap unrendered with no user-facing error. Fix:

let projects: ProjectOption[] = [];
if (projectsJson) {
  try {
    projects = JSON.parse(projectsJson);
  } catch {
    console.error('KimaiHeatmap: invalid data-projects JSON');
  }
}

WR-02: Resize listener never removed -- leaks on repeated init()

File: assets/src/heatmap.ts:107-112 Issue: The resize event listener is added but never removed. If init() is called multiple times on the same or different containers (e.g., SPA navigation, widget reload), each call adds another listener. The old listeners reference stale closures and DOM elements. Fix: Return a cleanup function or store the listener reference for removal:

const onResize = () => {
  clearTimeout(resizeTimer);
  resizeTimer = setTimeout(() => {
    if (state.data) doRender();
  }, 200);
};
window.addEventListener('resize', onResize);
// Store for cleanup: return { destroy: () => window.removeEventListener('resize', onResize) };

WR-03: innerHTML usage in stats renders data into DOM unsanitized

File: assets/src/shared/stats.ts:79 Issue: statsDiv.innerHTML = parts.join('') injects content built from DISPLAY_FORMAT(d) output and numeric values. Currently safe because the data sources are controlled (d3 date formatter output and numbers), but the innerHTML pattern is fragile -- if a future change introduces user-controlled strings into parts, it becomes an XSS vector. The same pattern exists in tooltip.ts:18 with tip.innerHTML = html. Fix: Consider using textContent with structured DOM creation instead of innerHTML, or document the safety invariant with a comment so future maintainers know the constraint.

Info

IN-01: Unused import -- timeMonth in date-utils.ts

File: assets/src/shared/date-utils.ts:1 Issue: timeMonth is imported from d3-time but never used in this module. It is used in renderers/year.ts which imports it directly. Fix: Remove timeMonth from the import:

import { timeMonday, timeSunday, timeDay } from 'd3-time';

IN-02: console.error calls as runtime logging

File: assets/src/heatmap.ts:14,95,125 Issue: Three console.error calls serve as runtime error reporting. These are appropriate for a Kimai plugin context (no structured logging available), but worth noting as intentional rather than debug artifacts. Fix: No action needed. If the project later adopts a logging abstraction, consolidate these.

---

Reviewed: 2026-04-09T14:00:00Z Reviewer: Claude (gsd-code-reviewer) Depth: standard