fix flaky re-scroll test timing issue

apps/app/src/client/util/watch-rendering-and-rescroll.spec.tsx

@@ -1,8 +1,24 @@
+import { setImmediate as realSetImmediate } from 'node:timers/promises';
 import { GROWI_IS_CONTENT_RENDERING_ATTR } from '@growi/core/dist/consts';
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
 
 import { watchRenderingAndReScroll } from './watch-rendering-and-rescroll';
 
+// happy-dom captures the real setTimeout at module load, before vitest's
+// fake timers are installed. Its MutationObserver callbacks therefore fire
+// on the REAL event loop, not on the fake timer clock. Yielding to the real
+// event loop (via node:timers/promises) flushes them.
+// Yield twice because happy-dom batches zero-delay timeouts and dispatch
+// across two real-timer ticks (the batcher + the listener's own timer).
+// NOTE: happy-dom v15 stores the MO listener callback in a `WeakRef`
+// (see MutationObserverListener.cjs), so GC between `observe()` and the
+// first mutation can silently drop delivery. Tests that assert MO-driven
+// behavior should rely on retry to absorb that rare collection window.
+const flushMutationObservers = async () => {
+  await realSetImmediate();
+  await realSetImmediate();
+};
+
 describe('watchRenderingAndReScroll', () => {
   let container: HTMLDivElement;
   let scrollToTarget: ReturnType<typeof vi.fn>;
@@ -56,7 +72,7 @@ describe('watchRenderingAndReScroll', () => {
     // Trigger a DOM mutation mid-timer
     const child = document.createElement('span');
     container.appendChild(child);
-    await vi.advanceTimersByTimeAsync(0);
+    await flushMutationObservers();
 
     // The timer should NOT have been reset — 2 more seconds should fire it
     vi.advanceTimersByTime(2000);
@@ -65,26 +81,30 @@ describe('watchRenderingAndReScroll', () => {
     cleanup();
   });
 
-  it('should detect rendering elements added after initial check via observer', async () => {
-    const cleanup = watchRenderingAndReScroll(container, scrollToTarget);
+  // Retry absorbs rare happy-dom MO WeakRef GC drops (see file-top note).
+  it(
+    'should detect rendering elements added after initial check via observer',
+    { retry: 3 },
+    async () => {
+      const cleanup = watchRenderingAndReScroll(container, scrollToTarget);
 
-    vi.advanceTimersByTime(3000);
-    expect(scrollToTarget).not.toHaveBeenCalled();
+      vi.advanceTimersByTime(3000);
+      expect(scrollToTarget).not.toHaveBeenCalled();
 
-    // Add a rendering element later (within 10s timeout)
-    const renderingEl = document.createElement('div');
-    renderingEl.setAttribute(GROWI_IS_CONTENT_RENDERING_ATTR, 'true');
-    container.appendChild(renderingEl);
+      // Add a rendering element later (within 10s timeout)
+      const renderingEl = document.createElement('div');
+      renderingEl.setAttribute(GROWI_IS_CONTENT_RENDERING_ATTR, 'true');
+      container.appendChild(renderingEl);
 
-    // Flush microtasks so MutationObserver callback fires
-    await vi.advanceTimersByTimeAsync(0);
+      // Flush MO so it schedules the poll timer
+      await flushMutationObservers();
 
-    // Timer should be scheduled — fires after 5s
-    await vi.advanceTimersByTimeAsync(5000);
-    expect(scrollToTarget).toHaveBeenCalledTimes(1);
+      await vi.advanceTimersByTimeAsync(5000);
+      expect(scrollToTarget).toHaveBeenCalledTimes(1);
 
-    cleanup();
-  });
+      cleanup();
+    },
+  );
 
   it('should scroll once when multiple rendering elements exist simultaneously', () => {
     const renderingEl1 = document.createElement('div');
@@ -154,27 +174,35 @@ describe('watchRenderingAndReScroll', () => {
     expect(scrollToTarget).not.toHaveBeenCalled();
   });
 
-  it('should not schedule further re-scrolls after rendering elements complete', async () => {
-    const renderingEl = document.createElement('div');
-    renderingEl.setAttribute(GROWI_IS_CONTENT_RENDERING_ATTR, 'true');
-    container.appendChild(renderingEl);
+  // Retry absorbs rare happy-dom MO WeakRef GC drops (see file-top note).
+  it(
+    'should perform a final re-scroll when rendering completes after the first poll',
+    { retry: 3 },
+    async () => {
+      const renderingEl = document.createElement('div');
+      renderingEl.setAttribute(GROWI_IS_CONTENT_RENDERING_ATTR, 'true');
+      container.appendChild(renderingEl);
 
-    const cleanup = watchRenderingAndReScroll(container, scrollToTarget);
+      const cleanup = watchRenderingAndReScroll(container, scrollToTarget);
 
-    // First timer fires at 5s — re-scroll executes
-    vi.advanceTimersByTime(5000);
-    expect(scrollToTarget).toHaveBeenCalledTimes(1);
+      // First timer fires at 5s — re-scroll executes
+      vi.advanceTimersByTime(5000);
+      expect(scrollToTarget).toHaveBeenCalledTimes(1);
 
-    // Rendering completes — attribute toggled to false
-    renderingEl.setAttribute(GROWI_IS_CONTENT_RENDERING_ATTR, 'false');
-    await vi.advanceTimersByTimeAsync(0);
+      // Rendering completes — attribute toggled to false. MO observes the
+      // transition and triggers a final re-scroll to compensate for the
+      // trailing layout shift.
+      renderingEl.setAttribute(GROWI_IS_CONTENT_RENDERING_ATTR, 'false');
+      await flushMutationObservers();
+      expect(scrollToTarget).toHaveBeenCalledTimes(2);
 
-    // No further re-scrolls should be scheduled
-    vi.advanceTimersByTime(10000);
-    expect(scrollToTarget).toHaveBeenCalledTimes(1);
+      // No further scrolls afterward — the MO cleared the next poll timer.
+      vi.advanceTimersByTime(10000);
+      expect(scrollToTarget).toHaveBeenCalledTimes(2);
 
-    cleanup();
-  });
+      cleanup();
+    },
+  );
 
   it('should scroll exactly once when rendering completes before the first timer fires', () => {
     const renderingEl = document.createElement('div');
@@ -183,12 +211,13 @@ describe('watchRenderingAndReScroll', () => {
 
     const cleanup = watchRenderingAndReScroll(container, scrollToTarget);
 
-    // Rendering completes before the first poll timer fires
+    // Rendering completes before the first poll timer fires (no async flush,
+    // so the MO does not deliver before the timer).
     renderingEl.setAttribute(GROWI_IS_CONTENT_RENDERING_ATTR, 'false');
 
-    // Poll timer fires at 5s — detects no rendering elements.
-    // wasRendering is reset in the timer callback BEFORE scrollToTarget so that
-    // the subsequent checkAndSchedule call does not trigger a redundant extra scroll.
+    // wasRendering is reset in the timer callback BEFORE scrollToTarget so
+    // the subsequent checkAndSchedule call does not trigger a redundant
+    // extra scroll.
     vi.advanceTimersByTime(5000);
     expect(scrollToTarget).toHaveBeenCalledTimes(1);
 

apps/app/src/client/util/watch-rendering-and-rescroll.ts

@@ -8,7 +8,9 @@ export const WATCH_TIMEOUT_MS = 10000;
 
 /**
  * Watch for elements with in-progress rendering status in the container.
- * Periodically calls scrollToTarget while rendering elements remain.
+ * Periodically calls scrollToTarget while rendering elements remain, and
+ * performs a final re-scroll when the last rendering element completes
+ * to compensate for the trailing layout shift (Requirements 3.1–3.3).
  * Returns a cleanup function that stops observation and clears timers.
  */
 export const watchRenderingAndReScroll = (