destroy() never stops the render loop — every destroyed PageFlip keeps a requestAnimationFrame loop running forever

[NCN]

Version: 2.0.7 (also present on current master)

Summary

Render.start() schedules an unconditional, self-perpetuating requestAnimationFrame loop:

// src/Render/Render.ts
public start(): void {
    this.update();

    const loop = (timer: number): void => {
        this.render(timer);
        requestAnimationFrame(loop);
    };

    requestAnimationFrame(loop);
}

Nothing ever cancels it. PageFlip.destroy() tears down the UI and removes the block element, but the render loop it started keeps running:

// src/PageFlip.ts
public destroy(): void {
    this.ui.destroy();
    this.block.remove();
}

So every PageFlip instance ever created keeps calling render() → drawFrame() once per animation frame for the life of the page, even after destroy().

Impact

• Any app that rebuilds its book (SPA route changes, responsive re-inits, changing usePortrait, replacing pages) accumulates one live render loop per rebuild. After N rebuilds there are N+1 loops doing per-frame work against detached state. On the canvas render this is N+1 full drawFrame() passes per frame.
• If destroy() is called while a flip animation is in flight, the orphaned loop later runs the remaining animation frames and fires onAnimateEnd, so a destroyed instance emits flip/state events into application handlers.
• Calling start() twice on the same render stacks a second loop on the same instance.

Reproduction

<div id="host"></div>
<script src="page-flip.browser.js"></script>
<script>
  // Count how many rAF callbacks are scheduled per frame.
  let scheduled = 0;
  const raf = window.requestAnimationFrame;
  window.requestAnimationFrame = (cb) => { scheduled++; return raf(cb); };

  function build() {
    const book = document.createElement('div');
    document.getElementById('host').appendChild(book);
    const pages = [1, 2].map((n) => {
      const p = document.createElement('div');
      p.className = 'page';
      p.textContent = n;
      book.appendChild(p);
      return p;
    });
    const pf = new St.PageFlip(book, { width: 200, height: 300 });
    pf.loadFromHTML(pages);
    return pf;
  }

  for (let i = 0; i < 10; i++) build().destroy();

  // All 10 instances are destroyed, yet ~10 rAF callbacks are scheduled
  // every frame, forever:
  setInterval(() => { console.log(scheduled); scheduled = 0; }, 1000);
</script>

Expected: after destroy(), no further requestAnimationFrame callbacks from that instance. Actual: one loop per destroyed instance runs until the page is unloaded.

Suggested fix

Track the pending frame id, guard the loop with a running flag, and stop the renderer from destroy():

// src/Render/Render.ts
/** Current requestAnimationFrame id (null when no frame is scheduled) */
private animationFrameId: number = null;
/** true while the render loop is allowed to run */
private isRunning = false;

public start(): void {
    if (this.isRunning) return; // don't stack a second loop

    this.isRunning = true;
    this.update();
    if (!this.isRunning) return; // stop() was called from update()

    const loop = (timer: number): void => {
        this.animationFrameId = null;
        if (!this.isRunning) return;

        this.render(timer);

        if (this.isRunning && this.animationFrameId === null) {
            this.animationFrameId = requestAnimationFrame(loop);
        }
    };

    this.animationFrameId = requestAnimationFrame(loop);
}

/**
 * Stop the render loop and drop any animation in progress
 */
public stop(): void {
    this.isRunning = false;

    if (this.animationFrameId !== null) {
        cancelAnimationFrame(this.animationFrameId);
        this.animationFrameId = null;
    }

    this.animation = null;
}

// src/PageFlip.ts
public destroy(): void {
    this.render.stop();
    this.ui.destroy();
    this.block.remove();
}

Notes on the fix:

• stop() deliberately discards an in-flight animation without calling onAnimateEnd — after destroy() those callbacks would emit events from a dead instance (the current orphaned-loop behavior does exactly that, one frame later).
• The re-check after this.update() matters: update() can call back into application code via updateOrientation, which may destroy the instance synchronously.
• The animationFrameId === null check inside the loop keeps a re-entrant start() from double-scheduling.

I've been running this patch in production (applied to the vendored 2.0.7 bundle) with a regression test covering repeated rebuilds, destroy() from inside a frame callback, and stop() during start()'s update().