2.0.0-beta.15: `<Repeat>` leaks live rows and can crash on disjoint window jumps

[yumemi-thomas]

Describe the bug

repeat() / <Repeat> with a reactive from handles small window slides, but not jumps where the old and new windows do not overlap.

Think of count={3} as a moving window:

from=0   -> rows 0,1,2
from=10  -> rows 10,11,12

On a correct implementation, clicking jump +10 from the initial state would leave exactly three live row scopes:

from=10, count=3
created=6  cleaned=3  LIVE SCOPES=3
calls: cleanup(0) cleanup(1) cleanup(2) map(10) map(11) map(12)

What actually happens is that <Repeat> creates every gap row too:

from=10, count=3
created=13  cleaned=3  LIVE SCOPES=10
calls: cleanup(0) cleanup(1) cleanup(2) map(3) map(4) map(5) map(6) map(7) map(8) map(9) map(10) map(11) map(12)

Rows 3..9 never appear in the DOM, but their owners/effects stay alive because they are stored outside the normal window bookkeeping. Repeating the jump leaks more rows. Jumping backward across a disjoint gap can also throw before the window is replaced.

So the bug has three visible forms:

1. Initial nonzero from maps too much. from=10,count=3 maps 0..12 instead of 10..12.
2. Forward disjoint jumps leak invisible row scopes. 0..2 -> 10..12 maps and leaks 3..9.
3. Backward disjoint jumps can crash. 20..22 -> 0..2 throws while trying to dispose a row at a negative local index.

This is a normal virtualized-list shape: dragging the scrollbar or jumping to an index moves from by more than count all the time.

Related to but distinct from #2767 (closed, fixed by #2784 / d8921ac): that fix covered the empty-window reset and overlapping backward slides, and those cases still work correctly. The disjoint geometries (prevTo < from and to <= offset) were never handled.

The invariant is simple: after any update, created - cleaned should equal count, and the map function should only run for indices in [from, from + count). Disjoint jumps violate both sides of that contract.

Your Example Website or App

https://stackblitz.com/edit/solidjs-templates-de3jtq5s?file=src%2FApp.tsx

The rendered list always looks correct — the leaked rows never enter the DOM, only their owners/effects stay alive — so the panel shows created/cleaned/live counts and the exact map/cleanup calls each action makes. (It's written with raw DOM writes because the backward jump crashes and freezes reactivity — a reactive panel couldn't render the final state.) Validated against 2.0.0-beta.15.

import { createSignal, onCleanup, Repeat, flush } from 'solid-js';

export default function App() {
  const [from, setFrom] = createSignal(0);
  let created = 0;
  let cleaned = 0;
  let calls: string[] = [];
  let panel!: HTMLPreElement;

  // Raw DOM write: the backward jump crashes and freezes reactivity, so the
  // panel must not depend on a reactive read to show the final leaked state.
  const report = (action: string, note = '') => {
    panel.textContent =
      `${action}
` +
      `from=${from()}, count=3 — the DOM list shows only these 3 rows
` +
      `created=${created}  cleaned=${cleaned}  LIVE SCOPES=${
        created - cleaned
      } (should be 3)
` +
      (note ? note + '' : '') +
      `calls: ${calls.join(' ') || '(none)'}`;
  };

  const go = (to: number, label: string) => {
    calls = [];
    try {
      setFrom(to);
      flush();
      report(label);
    } catch (e) {
      report(label, `CRASH: ${e} — UI frozen, buttons dead`);
    }
  };

  return (
    <main style={{ 'font-family': 'system-ui', padding: '16px' }}>
      <h2>&lt;Repeat&gt; leaks live row scopes on disjoint window jumps</h2>
      <button onClick={() => go(from() + 1, 'step +1')}>step +1</button>{' '}
      <button onClick={() => go(from() + 10, 'jump +10')}>jump +10</button>{' '}
      <button onClick={() => go(0, 'jump to 0 (disjoint backward)')}>
        jump to 0
      </button>
      <ul>
        <Repeat count={3} from={from()}>
          {(i: number) => {
            created++;
            calls.push(`map(${i})`);
            onCleanup(() => {
              cleaned++;
              calls.push(`cleanup(${i})`);
            });
            return <li>row {i}</li>;
          }}
        </Repeat>
      </ul>
      {/* the pre's ref callback fires the initial report once it mounts */}
      <pre ref={(el) => ((panel = el), report('mount'))} />
    </main>
  );
}

Steps to Reproduce the Bug or Issue

1. On mount the panel reads created=3 cleaned=0 LIVE SCOPES=3 and the list shows rows 0–2. Correct.
2. Click "jump +10". This changes the requested window from 0..2 to 10..12.

Expected panel:

jump +10
from=10, count=3 — the DOM list shows only these 3 rows
created=6  cleaned=3  LIVE SCOPES=3 (should be 3)
calls: cleanup(0) cleanup(1) cleanup(2) map(10) map(11) map(12)

Actual panel:

jump +10
from=10, count=3 — the DOM list shows only these 3 rows
created=13  cleaned=3  LIVE SCOPES=10 (should be 3)
calls: cleanup(0) cleanup(1) cleanup(2) map(3) map(4) map(5) map(6) map(7) map(8) map(9) map(10) map(11) map(12)

Rows 3..9 were created, never cleaned, and stay live — invisible in the DOM. Click "jump +10" again and LIVE SCOPES grows to 17.

3. Click "jump to 0". This is a backward disjoint jump.

Expected panel:

jump to 0 (disjoint backward)
from=0, count=3 — the DOM list shows only these 3 rows
created=9  cleaned=6  LIVE SCOPES=3 (should be 3)
calls: cleanup(10) cleanup(11) cleanup(12) map(0) map(1) map(2)

Actual panel:

CRASH: Error: Cannot read properties of undefined (reading 'dispose') — UI frozen, buttons dead

The list stays frozen on the old rows and every button is dead from then on.

4. Control: reload and use "step +1" instead. That move overlaps the old window, so everything stays correct and LIVE SCOPES remains 3.

5. Bonus: change the initial signal to createSignal(10) and reload — the mount panel reads created=13 for a 3-row window (the whole 0..12 prefix is mapped).

Expected behavior

Rows are only created inside the requested window, every row that leaves the window is cleaned up, and backward jumps work:

after any jump:  created - cleaned == 3, list shows the 3 requested rows
jump to 0:       no crash — rows 0–2 render, LIVE SCOPES=3

Screenshots or Videos

No response

Platform

• OS: macOS
• Browser: Chrome
• Version: 2.0.0-beta.15 (re-verified at next @ 61722cbe, which includes the fix(signals): reset Repeat offset when window goes empty (closes #2767) #2784 fix; all three symptoms and the exact counts still reproduce)

Additional context

Exact map/cleanup call sequences

Instrumenting the map function with console.log shows precisely which rows each jump creates and disposes:

mount:        map(0) map(1) map(2)                                    ✓
jump to 10:   cleanup(0..2), then map(3) map(4) ... map(12)           ← 10 created for a 3-row window;
                                                                        rows 3..9 never get a cleanup
jump to 20:   cleanup(10..12), then map(13) ... map(22)               ← 7 more leaked (13..19);
                                                                        23 mapped, 6 cleaned = 17 live
jump to 0:    nothing — no map, no cleanup, StatusError (see above)

Quick matrix:

action                        expected calls                   actual calls / result
initial from=10,count=3       map(10) map(11) map(12)          map(0) ... map(12)
0..2 -> 10..12                cleanup(0..2), map(10..12)       cleanup(0..2), map(3..12), leaks 3..9
10..12 -> 0..2                cleanup(10..12), map(0..2)       throws before replacement
overlap, 0..2 -> 1..3         cleanup(0), map(3)               works

Root cause

The row-creation loop in updateRepeat (packages/solid-signals/src/map.ts:377) assumes the old and new windows overlap:

for (let i = prevTo; i < to; i++) {
  this._mappings[i - from] = runWithOwner<MappedItem>(
    (this._nodes[i - from] = createOwner()),  // i - from is NEGATIVE for every i < from
    () => this._map(i)
  );
}

When prevTo < from (first render with nonzero from, where prevTo is 0, or any forward jump larger than the window), the loop starts at prevTo and walks the entire gap. Every i < from writes its owner at a negative _nodes index — invisible to all the dispose/splice bookkeeping, which only operates on local indices 0..len-1. That is symptoms 1 and 2.

Symptom 3 is the mirror image in the end-clear loop (map.ts:350):

for (let i = to; i < prevTo; i++) this._nodes[i - this._offset].dispose();

On a disjoint backward jump (to <= this._offset), i - this._offset goes negative, this._nodes[-7] is undefined, and .dispose() throws — aborting the update before anything is created or cleaned. The _offset > from shift branch just below (map.ts:360-375) has the same overlap assumption: on a disjoint backward move it would overwrite live owners in _nodes without disposing them.

Why #2784 / d8921ac don't cover this

That fix resets _offset when the window goes empty and corrects which rows get disposed on overlapping slides from a nonzero offset. All the loops above still model window movement as "shift + fill the difference", which is only valid when old ∩ new ≠ ∅. Disjoint jumps never take a sane path.

Suggested fix direction

Detect disjoint windows up front — from >= prevTo || to <= this._offset — and treat them as a full window replacement: dispose all current rows (local indices 0..len-1), reset _nodes/_mappings, and create exactly the rows in [from, to). That condition is the exact complement of "the windows overlap" (from < prevTo && to > offset), so partial-overlap slides still take the existing preserving path and keep shared rows' identity; only truly disjoint moves are replaced wholesale (which is unavoidable work — no shared rows to preserve). It also subsumes the nonzero-first-render case, where prevTo is 0.

Working patch with tests in hand (fix PR to follow): the ~15-line guard above fixes all three symptoms with no regression to the overlapping paths, and the common overlapping-slide path benchmarks unchanged (the two added comparisons are free). No loop-clamping is needed — the guard's classification is exact, so the negative-index accesses become unreachable rather than merely clamped.

Does this exist in Solid 1.x?

Not applicable — new 2.0 API. repeat()/<Repeat> has no Solid 1.x counterpart (the closest 1.x pattern, <Index> over a sliced range array, has no windowing bookkeeping and no equivalent failure mode).