Skip to content

refactor: fix functional lint violations in animation package #158

Description

@cnotv

Summary

The packages/animation/src/ files have lint warnings from the functional/immutable-data and functional/no-let ESLint rules. These are currently set to 'warn' (non-blocking) in the animation package override in eslint.config.js because the timeline manager fundamentally relies on mutable state.

Affected files

TimelineManager.ts

The TimelineState object holds mutable Map, Set, and Array that are mutated throughout the manager's lifecycle:

  • state.timeline.push(actionWithId) — L70
  • state.activeActions.set(id, actionWithId) — L71
  • state.timeline.splice(index, 1) — L87
  • state.activeActions.delete(id) — L88
  • state.completedActions.add(id) — L89, L175
  • state.activeActions.set(id, updatedAction) — L110
  • state.timeline[index] = updatedAction — L109
  • state.completedActions.forEach(...) with inner splice / delete — L145-151
  • state.timeline.length = 0 — L157
  • state.activeActions.clear() — L158
  • state.completedActions.clear() — L159

TimelineLogger.ts

  • logs.push(fullEntry) — L74 (uses let logs)
  • started.add(entry.actionId) — L114
  • completed.add(entry.actionId) — L115
  • categories[cat] = (categories[cat] || 0) + 1 — L141
  • Magic number 8 in entry.actionId.substring(0, 8) — L55

What needs to be done

Option A — Full refactor (preferred long term)

Rewrite TimelineManager to use immutable state transitions: instead of mutating state.timeline, state.activeActions, etc., return new state objects on each operation. This aligns with functional/immutable-data intent.

Challenges:

  • TimelineState is shared across multiple closures (buildActionMethods, buildQueryMethods) via reference mutation — switching to immutable state requires threading the new state through every call, or using a single top-level closure with let state = ... reassignment
  • Performance: rebuilding Map and Set on every add/remove may be noticeable for large timelines

Option B — Targeted suppression (minimal effort)

Keep the current 'warn' override and additionally suppress functional/no-let for the logger's let logs pattern. Accept that this module has justified mutable state (it's a stateful cache/registry by nature).

Option C — Extract constants, fix easy violations

Fix only the low-effort violations:

  • Extract const ACTION_ID_DISPLAY_LENGTH = 8 in TimelineLogger.ts
  • Replace logs.push(fullEntry) with logs = [...logs, fullEntry]
  • Replace started.add() / completed.add() with a reduce building new Sets
  • Replace categories[cat] = ... with reduce building a new object

Leave TimelineManager.ts mutations as-is under 'warn' since restructuring the state model is a larger refactor.

Current state

The ESLint override in eslint.config.js (lines 453-460) already sets these to 'warn' so CI is not blocked. This issue tracks resolving them properly.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions