Skip to content

Commit 4e81e9c

Browse files
ryansolidyumemi-thomascursoragent
committed
fix(signals): repeat() handles disjoint window jumps (#2853, from #2854)
updateRepeat modeled every window move as clear-end + shift-front + fill, valid only when the old and new windows overlap. Disjoint moves walked _nodes at negative local indices: forward jumps larger than the window created and leaked every gap row (owners/effects alive, invisible to dispose bookkeeping), first render with nonzero `from` mapped the whole prefix, and backward disjoint jumps threw on _nodes[negative].dispose() and froze the list. Detect disjointness up front (from >= prevTo || to <= offset — the exact complement of overlap) and replace the window wholesale; overlapping slides (the #2784 fix) are untouched. Co-authored-by: yumemi-thomas <yumemi-thomas@users.noreply.github.com> Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent c4ba526 commit 4e81e9c

3 files changed

Lines changed: 92 additions & 0 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>` leaking live row scopes and crashing on disjoint window jumps. When a reactive `from` moves the window to indices that don't overlap the previous window, the shift-and-fill update walked `_nodes` at negative local indices: a forward jump larger than the window created every gap row and left them alive (owners, effects, and `onCleanup`s never disposed), and a backward disjoint jump threw `Cannot read properties of undefined (reading 'dispose')` and froze the list. The first render with a nonzero `from` mapped the whole `0..from+count` prefix for the same reason. Disjoint windows are now detected and replaced wholesale; overlapping slides (the #2784 fix) are unchanged.

packages/solid-signals/src/map.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -346,6 +346,20 @@ function updateRepeat<MappedItem>(this: RepeatData<MappedItem>): any[] {
346346
// remove fallback
347347
if (this._len === 0 && this._nodes[0]) this._nodes[0].dispose();
348348

349+
// Disjoint windows have no rows to retain; replace them wholesale.
350+
if (from >= prevTo || to <= this._offset) {
351+
for (let i = 0; i < this._len; i++) this._nodes[i].dispose();
352+
this._nodes = new Array(newLen);
353+
this._mappings = new Array(newLen);
354+
for (let i = 0; i < newLen; i++)
355+
this._mappings[i] = runWithOwner<MappedItem>((this._nodes[i] = createOwner()), () =>
356+
this._map(from + i)
357+
);
358+
this._offset = from;
359+
this._len = newLen;
360+
return;
361+
}
362+
349363
// clear the end
350364
for (let i = to; i < prevTo; i++) this._nodes[i - this._offset].dispose();
351365

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

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -346,3 +346,76 @@ it("slides the window backwards: disposes departing rows and retains the overlap
346346
// only the departing rows were disposed
347347
expect(disposed.sort()).toEqual([4, 5]);
348348
});
349+
350+
describe("disjoint window jumps", () => {
351+
function windowed() {
352+
const [from, setFrom] = createSignal(0);
353+
let created = 0;
354+
let cleaned = 0;
355+
let view!: () => number[];
356+
createRoot(() => {
357+
view = repeat(
358+
() => 3,
359+
i => {
360+
created++;
361+
onCleanup(() => cleaned++);
362+
return i;
363+
},
364+
{ from }
365+
);
366+
});
367+
flush();
368+
return {
369+
view,
370+
jump: (n: number) => {
371+
setFrom(n);
372+
flush();
373+
},
374+
live: () => created - cleaned
375+
};
376+
}
377+
378+
it("a forward jump larger than the window creates only the window's rows", () => {
379+
const w = windowed();
380+
expect(w.view()).toEqual([0, 1, 2]);
381+
expect(w.live()).toBe(3);
382+
w.jump(10);
383+
expect(w.view()).toEqual([10, 11, 12]);
384+
expect(w.live()).toBe(3);
385+
});
386+
387+
it("repeated disjoint jumps do not accumulate live scopes", () => {
388+
const w = windowed();
389+
w.jump(10);
390+
w.jump(20);
391+
expect(w.view()).toEqual([20, 21, 22]);
392+
expect(w.live()).toBe(3);
393+
});
394+
395+
it("a backward disjoint jump maps the new window without crashing", () => {
396+
const w = windowed();
397+
w.jump(20);
398+
expect(() => w.jump(0)).not.toThrow();
399+
expect(w.view()).toEqual([0, 1, 2]);
400+
expect(w.live()).toBe(3);
401+
});
402+
403+
it("initial render with a nonzero `from` maps only the window", () => {
404+
const [from] = createSignal(10);
405+
let created = 0;
406+
let view!: () => number[];
407+
createRoot(() => {
408+
view = repeat(
409+
() => 3,
410+
i => {
411+
created++;
412+
return i;
413+
},
414+
{ from }
415+
);
416+
});
417+
flush();
418+
expect(view()).toEqual([10, 11, 12]);
419+
expect(created).toBe(3);
420+
});
421+
});

0 commit comments

Comments
 (0)