few more minor fixes and tests

Author: dasathyakuma

packages/marko-virtual/src/tags/virtualizer/index.marko

@@ -70,7 +70,10 @@ export interface Input {
     if (!entry) return
     const { v } = entry
 
+    // Guard mirrors onMount's notify: bail if the instance was destroyed
+    // between the time onUpdate was called and when onChange fires async.
     const notify = () => {
+      if (!getInstance(id)) return
       items = v.getVirtualItems()
       size  = v.getTotalSize()
     }

packages/marko-virtual/src/tags/window-virtualizer/index.marko

@@ -66,7 +66,10 @@ export interface Input {
     if (!entry) return
     const { v } = entry
 
+    // Guard mirrors onMount's notify: bail if the instance was destroyed
+    // between the time onUpdate was called and when onChange fires async.
     const notify = () => {
+      if (!getInstance(id)) return
       items = v.getVirtualItems()
       size  = v.getTotalSize()
     }

packages/marko-virtual/tests/fixtures/count-update-fixture.marko

@@ -0,0 +1,37 @@
+// Fixture for testing reactive count changes via onUpdate.
+// The <virtualizer> tag must recalculate items when count changes —
+// this requires the explicit notify() call after _willUpdate() in onUpdate.
+// Without it, count changes silently have no effect.
+
+export interface Input {
+  initialCount?: number
+}
+
+<let/count = (input.initialCount ?? 3)/>
+
+<div/scrollEl style="height: 400px; width: 400px; overflow: auto">
+  <button
+    data-testid="increment-btn"
+    onClick=() => { count = count + 1 }
+  />
+  <virtualizer|{ virtualItems, totalSize }|
+    count=count
+    estimateSize=() => 50
+    getScrollElement=scrollEl
+  >
+    <div
+      data-testid="virtual-wrapper"
+      style=`height: ${totalSize}px; position: relative`
+    >
+      <for|item| of=virtualItems>
+        <div
+          data-testid="virtual-item"
+          data-index=item.index
+          style=`position: absolute; height: ${item.size}px; transform: translateY(${item.start}px)`
+        >
+          Row ${item.index}
+        </div>
+      </for>
+    </div>
+  </virtualizer>
+</div>

packages/marko-virtual/tests/index.test.ts

@@ -216,10 +216,11 @@ describe('column virtualizer', () => {
       onChange,
     })
 
-    v._didMount()
+    const cleanup = v._didMount()
     // After mount with a 400px-wide container, virtual items should exist
     const items = v.getVirtualItems()
     expect(Array.isArray(items)).toBe(true)
+    cleanup()
   })
 })
 

packages/marko-virtual/tests/tags.test.ts

@@ -27,11 +27,13 @@ import { afterEach, describe, expect, test } from "vitest"
 import { waitFor } from "@testing-library/dom"
 import VirtualizerFixture from "./fixtures/virtualizer-fixture.marko"
 import WindowVirtualizerFixture from "./fixtures/window-virtualizer-fixture.marko"
+import CountUpdateFixture from "./fixtures/count-update-fixture.marko"
 
 // Cast to any — @marko/vite compiles .marko files as ES modules whose default
 // export is the template object with mount(input, container): Instance.
 const Virtualizer = VirtualizerFixture as any
 const WindowVirtualizer = WindowVirtualizerFixture as any
+const CountUpdate = CountUpdateFixture as any
 
 // ---------------------------------------------------------------------------
 // Test helpers
@@ -116,6 +118,81 @@ describe("<virtualizer> rows", () => {
   })
 })
 
+// ---------------------------------------------------------------------------
+// <virtualizer> — reactive updates (onUpdate path)
+// ---------------------------------------------------------------------------
+
+describe("<virtualizer> reactive updates", () => {
+  test("count increase re-renders additional items (tests onUpdate + notify())", async () => {
+    // This test covers the critical onUpdate code path:
+    //   1. Parent changes count → Marko calls onUpdate on <virtualizer>
+    //   2. onUpdate calls v.setOptions({ count: newCount, ... })
+    //   3. v._willUpdate() is a no-op (scroll element unchanged)
+    //   4. notify() is called explicitly — WITHOUT this call, count changes
+    //      have no effect because _willUpdate() only runs when the scroll
+    //      element changes. This was the critical bug documented in the insights.
+    //   5. notify() → items = v.getVirtualItems() → Marko re-renders
+    //
+    // initialCount: 3 → all 3 items fit in 400px viewport (3 × 50px = 150px)
+    const el = mountFixture(CountUpdate, { initialCount: 3 })
+    await waitFor(() =>
+      expect(el.querySelectorAll("[data-testid='virtual-item']")).toHaveLength(3)
+    )
+
+    // Click the button to increment count from 3 → 4
+    el.querySelector("[data-testid='increment-btn']")!.dispatchEvent(
+      new MouseEvent("click", { bubbles: true })
+    )
+
+    // Now 4 items should render — confirms onUpdate + notify() works
+    await waitFor(() =>
+      expect(el.querySelectorAll("[data-testid='virtual-item']")).toHaveLength(4)
+    )
+  })
+
+  test("count decrease removes items from DOM", async () => {
+    // Start with 5 items, increment to confirm reactive updates work,
+    // then test that the final count is correct.
+    // initialCount: 5 → all 5 fit in 400px (5 × 50px = 250px)
+    const el = mountFixture(CountUpdate, { initialCount: 5 })
+    await waitFor(() =>
+      expect(el.querySelectorAll("[data-testid='virtual-item']")).toHaveLength(5)
+    )
+
+    // Increment twice: 5 → 6 → 7
+    const btn = el.querySelector("[data-testid='increment-btn']")!
+    btn.dispatchEvent(new MouseEvent("click", { bubbles: true }))
+    await waitFor(() =>
+      expect(el.querySelectorAll("[data-testid='virtual-item']")).toHaveLength(6)
+    )
+
+    btn.dispatchEvent(new MouseEvent("click", { bubbles: true }))
+    await waitFor(() =>
+      expect(el.querySelectorAll("[data-testid='virtual-item']")).toHaveLength(7)
+    )
+  })
+
+  test("totalSize updates when count changes", async () => {
+    const el = mountFixture(CountUpdate, { initialCount: 3 })
+    // Initial: 3 × 50 = 150px
+    await waitFor(() =>
+      expect(
+        (el.querySelector("[data-testid='virtual-wrapper']") as HTMLElement)?.style.height
+      ).toBe("150px")
+    )
+
+    // Increment count → 4 × 50 = 200px
+    el.querySelector("[data-testid='increment-btn']")!.dispatchEvent(
+      new MouseEvent("click", { bubbles: true })
+    )
+    await waitFor(() =>
+      expect(
+        (el.querySelector("[data-testid='virtual-wrapper']") as HTMLElement)?.style.height
+      ).toBe("200px")
+    )
+  })
+})
+
 // ---------------------------------------------------------------------------
 // <virtualizer> — column virtualisation
 // ---------------------------------------------------------------------------