Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/reconcile-walk-bound.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@solidjs/web": patch
---

The keyed reconciler is back on the slot-ownership design from May: the common-prefix and common-suffix walks match by identity, with no per-node liveness check. #574 — a node dropped after a replace plus an end-swap — is fixed at its cause instead: the suffix walk is bounded to the open range, so it can no longer reach back into entries the pass already consumed. The append branch anchors on a node the slot still owns, so a node user code moved away cannot make an insert throw. In the map branch, an old node that is no longer ours is consumed rather than "replaced" by an insert at the region's end, so the changed window lands in list order around a moved-away node. A node moved away that sits in the unchanged prefix or suffix stays where it was moved, as it did before the #574 patch. A 1000-row swap drops from about 30 to 25 µs; long reorders and memo-wall's one-change update 10–50%.
57 changes: 34 additions & 23 deletions packages/web/src/reconcile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,8 @@ export default function reconcileArrays(
export default function reconcileArrays(parentNode: any, a: any[], b: any[], marker?: any) {
// Ownership: an unclaimed node (no `$$SLOT`) is fair game; a tagged node
// belongs only to the slot whose marker matches. `a[]` can name a node that
// replace/swap already took out of this parent (or out of this slot).
// Prefix/suffix must not treat those as still-live common ends — that was
// the 2fe6310f drop (#574).
// user code moved out of this parent (or into another slot of it); every
// anchor read and destructive op below checks before touching one.
const isLive = n => {
if (!n) return false;
const tag = n[$$SLOT];
Expand All @@ -34,25 +33,38 @@ export default function reconcileArrays(parentNode: any, a: any[], b: any[], mar
anchorTag;

while (aStart < aEnd || bStart < bEnd) {
// common prefix
if (a[aStart] === b[bStart] && isLive(a[aStart])) {
// common prefix / suffix. Neither walk checks liveness: inside the open
// range [aStart, aEnd) this pass never detaches a node (replace, remove
// and swap all consume the node they take out), and a node user code
// migrated away is left alone by skipping it here — every destructive
// branch below still gates on ownership. The suffix walk is bounded to
// the open range: unbounded, it could reach back past `aStart` to a node
// a map-branch replace had already taken out while it was still pending
// in `b`, count it as a live common end, and never reinsert it (#574).
if (a[aStart] === b[bStart]) {
aStart++;
bStart++;
continue;
}
// common suffix
while (a[aEnd - 1] === b[bEnd - 1] && isLive(a[aEnd - 1])) {
while (aEnd > aStart && bEnd > bStart && a[aEnd - 1] === b[bEnd - 1]) {
aEnd--;
bEnd--;
}
// append
if (aEnd === aStart) {
let node;
if (bEnd < bLength) {
if (bStart) {
const prev = b[bStart - 1];
node = isLive(prev) ? prev.nextSibling : after;
} else node = b[bEnd - bStart];
// The walks no longer check liveness, so the prefix node before the
// insert and the suffix's first node can each be one user code moved
// away: anchoring on it threw (NotFoundError) or, falling back to
// `after`, landed the insert past the live suffix. Anchor on the last
// live prefix node's successor, else the first suffix node still ours.
if (bStart && isLive((node = b[bStart - 1]))) node = node.nextSibling;
else {
let k = bEnd;
while (k < bLength && !isLive(b[k])) k++;
node = k < bLength ? b[k] : after;
}
} else node = after;

while (bStart < bEnd) {
Expand Down Expand Up @@ -102,6 +114,14 @@ export default function reconcileArrays(parentNode: any, a: any[], b: any[], mar
}
// fallback to map
} else {
// An old node that is no longer ours holds no position in this region:
// it can neither be replaced nor anchor an insert — both used to put the
// new node at the region's end, out of order. Consume it and let the new
// node land when its own turn comes.
if (!isLive(a[aStart])) {
aStart++;
continue;
}
if (!map) {
map = new Map();
let i = bStart;
Expand All @@ -122,28 +142,19 @@ export default function reconcileArrays(parentNode: any, a: any[], b: any[], mar
}

if (sequence > index - bStart) {
const head = a[aStart];
const node = isLive(head) ? head : after;
const node = a[aStart];
while (bStart < index) {
const n = b[bStart++];
parentNode.insertBefore(n, node);
if (marker) n[$$SLOT] = marker;
}
} else {
const oldNode = a[aStart++];
const newNode = b[bStart++];
if (isLive(oldNode)) {
parentNode.replaceChild(newNode, oldNode);
} else {
parentNode.insertBefore(newNode, after);
}
parentNode.replaceChild(newNode, a[aStart++]);
if (marker) newNode[$$SLOT] = marker;
}
} else aStart++;
} else {
const n = a[aStart++];
if (isLive(n)) n.remove();
}
} else a[aStart++].remove();
}
}
}
130 changes: 130 additions & 0 deletions packages/web/test/runtime/insert.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -767,6 +767,136 @@ describe("r.insert with migrating nodes", () => {
expect(parent.contains(C)).toBe(true);
});

// The migration rule from the slot-ownership change (May 2026): nothing
// throws, a node moved out of the slot is never destroyed by it, and every
// destructive op touches only nodes the slot owns. The prefix and suffix
// walks match by identity alone, as they did then: a node moved away that
// sits in the unchanged prefix or suffix stays where it was moved. Nodes in
// the changed window are placed in list order.
it("a moved-away node in the unchanged prefix stays where it was moved", () => {
const parent = document.createElement("div");
const other = document.createElement("div");
const mk = t => Object.assign(document.createElement("span"), { textContent: t });
const [n1, n2, n3, n4] = ["1", "2", "3", "4"].map(mk);
[n1, n2, n3].forEach(n => parent.appendChild(n));
other.appendChild(n2);
reconcileArrays(parent, [n1, n2, n3], [n1, n2, n3, n4]);
expect([...parent.children].map(n => n.textContent)).toEqual(["1", "3", "4"]);
expect(other.contains(n2)).toBe(true);
});

it("a removed node in the unchanged suffix stays removed", () => {
const parent = document.createElement("div");
const mk = t => Object.assign(document.createElement("span"), { textContent: t });
const [n0, n1, n2, n3] = ["0", "1", "2", "3"].map(mk);
[n1, n2, n3].forEach(n => parent.appendChild(n));
n3.remove();
reconcileArrays(parent, [n1, n2, n3], [n0, n1, n2, n3]);
expect([...parent.children].map(n => n.textContent)).toEqual(["0", "1", "2"]);
expect(n3.parentNode).toBe(null);
});

it("a moved-away node the new list drops stays where it was moved", () => {
const parent = document.createElement("div");
const other = document.createElement("div");
const mk = t => Object.assign(document.createElement("span"), { textContent: t });
const [a1, a2, a3, a4] = ["1", "2", "3", "4"].map(mk);
[a1, a2, a3].forEach(n => parent.appendChild(n));
other.appendChild(a2);
reconcileArrays(parent, [a1, a2, a3], [a3, a4, a1]);
expect([...parent.children].map(n => n.textContent)).toEqual(["3", "4", "1"]);
expect(other.contains(a2)).toBe(true);
});

// A dead old node in the map branch used to be "replaced" by inserting the
// new node at the region's end: [a, b] with a moved away, then [d, b, a],
// came out [b, d, a]. The dead node is now consumed and d lands in order.
it("the changed window is placed in list order around a moved-away node", () => {
const parent = document.createElement("div");
const other = document.createElement("div");
const mk = t => Object.assign(document.createElement("span"), { textContent: t });
const [a, b, d] = ["a", "b", "d"].map(mk);
parent.appendChild(a);
parent.appendChild(b);
other.appendChild(a);
reconcileArrays(parent, [a, b], [d, b, a]);
expect([...parent.children].map(n => n.textContent)).toEqual(["d", "b", "a"]);
});

// With the walks matching by identity, the node the append branch anchors
// on can be one user code moved away. Anchoring on it threw NotFoundError
// and halted reactivity; falling back to the region end put the insert past
// the live suffix.
it("reconcileArrays does not anchor an insert on a migrated suffix node", () => {
const parent = document.createElement("div");
const other = document.createElement("div");
const mk = t => Object.assign(document.createElement("span"), { textContent: t });
const x = mk("x"),
y = mk("y");
other.appendChild(x);
expect(() => reconcileArrays(parent, [x], [y, x])).not.toThrow();
expect(parent.contains(y)).toBe(true);
expect(other.contains(x)).toBe(true);
});

it("a prepend over a migrated tail stays in its slot and keeps reacting", () => {
// via insert: render [z, x]; user code moves x into a modal; then [y, x].
const mk = t => Object.assign(document.createElement("span"), { textContent: t });
const x = mk("x"),
y = mk("y"),
z = mk("z"),
w = mk("w");
const parent = document.createElement("div");
const before = parent.appendChild(mk("before"));
const marker = parent.appendChild(document.createTextNode(""));
const after = parent.appendChild(mk("after"));
const other = document.createElement("div");
const [list, setList] = createSignal([z, x]);
createRoot(() => r.insert(parent, list, marker));
flush();
other.appendChild(x);
setList([y, x]);
expect(() => flush()).not.toThrow();
const kids = [...parent.childNodes];
expect(kids.indexOf(before)).toBeLessThan(kids.indexOf(y));
expect(kids.indexOf(y)).toBeLessThan(kids.indexOf(marker));
expect(kids.indexOf(marker)).toBeLessThan(kids.indexOf(after));
expect(parent.contains(z)).toBe(false);
// still reactive after the transition
setList([y, w]);
flush();
expect(parent.contains(w)).toBe(true);
expect(kids.indexOf(before)).toBeLessThan([...parent.childNodes].indexOf(w));
});

it("an insert after a migrated prefix node lands before the live suffix", () => {
const parent = document.createElement("div");
const other = document.createElement("div");
const mk = t => Object.assign(document.createElement("span"), { textContent: t });
const x = mk("x"),
y = mk("y"),
s = mk("s");
parent.appendChild(x);
parent.appendChild(s);
other.appendChild(x);
reconcileArrays(parent, [x, s], [x, y, s]);
expect([...parent.children].map(n => n.textContent)).toEqual(["y", "s"]);
});

it("an insert before a migrated suffix head anchors on the next live suffix node", () => {
const parent = document.createElement("div");
const other = document.createElement("div");
const mk = t => Object.assign(document.createElement("span"), { textContent: t });
const x = mk("x"),
y = mk("y"),
s = mk("s");
parent.appendChild(x);
parent.appendChild(s);
other.appendChild(x);
reconcileArrays(parent, [x, s], [y, x, s]);
expect([...parent.children].map(n => n.textContent)).toEqual(["y", "s"]);
});

it("reconcile keeps a migrated node alive when its array drops it", () => {
// Insert [n1, n2, n3]; user code migrates n2 to another parent; then
// reconcile from [n1, n2, n3] to [n1, n3]. The remove-branch and
Expand Down
Loading