FOLLOW-UP: animatedElements.add() consumes once-per-lifetime slot before animation validation (from PR #69)

[adnaan]

Context

This follow-up task was identified during the Round 8 review of PR #69 by the Claude bot.

Source PR: #69
PR Title: fix: cross-handler SPA nav, infinite scroll race, animation cleanup
Merged commit: e40f4b1
Suggested by: @claude[bot]
Comment type: PR review comment (post-merge)
Original comment: #69 (comment)

The bug

In dom/directives.ts (current main), the handleAnimateDirectives flow adds the element to the animatedElements WeakSet before resolving the animation shorthand:

// dom/directives.ts ~L198-220
if (animatedElements.has(htmlElement)) break;
animatedElements.add(htmlElement);                 // ← added unconditionally

const duration = parseInt(/* ... */);
const animation = config || "fade";

let animationValue = "";
switch (animation) {
  case "fade":  animationValue = `lvt-fade-in ${duration}ms ease-out`;  break;
  case "slide": animationValue = `lvt-slide-in ${duration}ms ease-out`; break;
  case "scale": animationValue = `lvt-scale-in ${duration}ms ease-out`; break;
  default:
    console.warn(`Unknown lvt-fx:animate mode: ${animation}`);
}
if (!animationValue) break;                        // ← bails AFTER set.add()
htmlElement.style.animation = animationValue;

When a user writes a typo like lvt-fx:animate="bounse" (meant to be "bounce", which doesn't exist, or meant "scale"), the flow is:

1. animatedElements.has(el) → false
2. animatedElements.add(el) → consumes the once-per-lifetime slot
3. default branch logs console.warn("Unknown lvt-fx:animate mode: bounse")
4. animationValue is ""
5. if (!animationValue) break; → exits before applying any animation

The element is now permanently recorded as "already animated" even though nothing actually played. If the template is corrected on the server (e.g. the user fixes the typo to lvt-fx:animate="scale") and the same DOM node is reused via morphdom, the corrected animation will silently never fire, because animatedElements.has(el) returns true.

Why this matters

• Silent correctness issue — no error, no warning on the retry path.
• Painful to debug — developers will add lvt-fx:animate="fade" to verify the directive works, see nothing animate, and have no indication that a prior typo consumed the slot.
• Affects the exact case animatedElements was designed to protect: valid animations on fresh DOM nodes.

Fix

Move animatedElements.add(htmlElement) to just before htmlElement.style.animation = animationValue, so only elements that actually animate consume the slot:

if (animatedElements.has(htmlElement)) break;

const duration = parseInt(/* ... */);
const animation = config || "fade";

let animationValue = "";
switch (animation) { /* ... */ }
if (!animationValue) break;                        // fail fast before add()

animatedElements.add(htmlElement);                 // ← moved
htmlElement.style.animation = animationValue;

Test to add

Regression test in tests/directives.test.ts:

1. Apply lvt-fx:animate="bounse" to an element.
2. Run directive handler → assert console.warn was called, no animation applied.
3. Correct the attribute to lvt-fx:animate="fade".
4. Run directive handler again on the same element → assert the fade animation IS applied.

Original comment excerpt

1. animatedElements.add() before validating animation type (dom/directives.ts ~L53-80)

The element is added to the WeakSet before the if (!animationValue) break guard fires. An element with an unknown animation type (e.g. a typo like lvt-fx:animate="bounce") gets silently consumed — it logs a console.warn but then never animates, and because it's in the set it won't retry even if the type is corrected via a re-render on the same node. Consider moving animatedElements.add() to just before htmlElement.style.animation = animationValue so the set only gates elements that actually animated.

---

This issue was automatically created by /prmonitor from the Round 8 bot review on PR #69.