Skip to content

Commit d8921ac

Browse files
fix: repeat disposes wrong rows when from advances from non-zero offset
The front-clear loop indexed the local _nodes array with a global index, so sliding a window (e.g. 1-3 -> 3-5) disposed rows that stayed visible and leaked rows that left. Dispose the correct local positions instead. Fixes #2767. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent db88de1 commit d8921ac

3 files changed

Lines changed: 37 additions & 5 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@solidjs/signals": patch
3+
---
4+
5+
Fix `repeat` (`<Repeat>`) disposing the wrong row owners when `from` advances from a non-zero offset. The front-clear loop indexed the local `_nodes` array with a global index, so a sliding window (e.g. rows 1-3 → 3-5) disposed rows that stayed visible and leaked rows that left. It now disposes the correct local positions.

packages/solid-signals/src/map.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -345,12 +345,13 @@ function updateRepeat<MappedItem>(this: RepeatData<MappedItem>): any[] {
345345
for (let i = to; i < prevTo; i++) this._nodes[i - this._offset].dispose();
346346

347347
if (this._offset < from) {
348-
// clear beginning
349-
let i = this._offset;
350-
while (i < from && i < this._len) this._nodes[i++].dispose();
348+
// clear beginning — `_nodes` is local-indexed, so the rows leaving the
349+
// front sit at local positions 0..(from - _offset), clamped to old len.
350+
const removed = from - this._offset;
351+
for (let i = 0; i < removed && i < this._len; i++) this._nodes[i].dispose();
351352
// shift indexes
352-
this._nodes.splice(0, from - this._offset);
353-
this._mappings.splice(0, from - this._offset);
353+
this._nodes.splice(0, removed);
354+
this._mappings.splice(0, removed);
354355
} else if (this._offset > from) {
355356
// shift indexes
356357
let i = prevTo - this._offset - 1;

packages/solid-signals/tests/repeat.test.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import {
44
createSignal,
55
createStore,
66
flush,
7+
onCleanup,
78
repeat
89
} from "../src/index.js";
910

@@ -183,6 +184,31 @@ it("should compute map when key by index", () => {
183184
expect(computed).toHaveBeenCalledTimes(4);
184185
});
185186

187+
it("disposes the rows that leave the window when `from` advances from a non-zero offset", () => {
188+
const cleanups: number[] = [];
189+
const [from, setFrom] = createSignal(1);
190+
191+
const map = createRoot(() =>
192+
repeat(
193+
() => 3,
194+
index => {
195+
onCleanup(() => cleanups.push(index));
196+
return index;
197+
},
198+
{ from }
199+
)
200+
);
201+
202+
expect(map()).toEqual([1, 2, 3]);
203+
204+
// slide window 1-3 -> 3-5: rows 1 and 2 leave, row 3 stays
205+
setFrom(3);
206+
flush();
207+
208+
expect(map()).toEqual([3, 4, 5]);
209+
expect(cleanups.sort((a, b) => a - b)).toEqual([1, 2]);
210+
});
211+
186212
it("should retain instances when only `offset` changes", () => {
187213
const [source] = createStore<Array<{ id: string }>>([
188214
{ id: "a" },

0 commit comments

Comments
 (0)