}} state */
function teardownRepeat(state) {
- for (const inst of state.map.values()) {
- disposeInstance(inst);
- removeBetween(inst.startNode, inst.endNode);
+ // Same delete-as-you-go shape as the leftover loop in `reconcileRepeat`,
+ // for the same reason: a throw part-way must not leave already-removed
+ // instances in the map. The trailing `clear()` stays as a no-op safety net.
+ for (const [k, inst] of [...state.map]) {
+ state.map.delete(k);
+ try {
+ disposeInstance(inst);
+ } finally {
+ removeBetween(inst.startNode, inst.endNode);
+ }
}
state.map.clear();
}
@@ -1977,42 +2055,107 @@ function reconcileArray(part, value) {
const old = state.items;
/** @type {ArrayItem[]} */
const next = [];
+ // How many slots of `old` are fully processed. Tracked rather than inferred
+ // from `next.length`, because the shrink loop below advances through `old`
+ // while `next` stops growing, so the two part company there. The catch is
+ // the only reader.
+ let consumed = 0;
- for (let i = 0; i < value.length; i++) {
- const v = value[i];
- const o = old[i];
- if (isTemplate(v)) {
- const tr = /** @type any */ (v);
- if (o && o.type === 'tpl' && o.inst.strings === tr.strings) {
- updateInstance(o.inst, tr.values);
- next.push(o);
- continue;
- }
- } else if (v != null && v !== false && v !== true) {
- if (o && o.type === 'text') {
- const str = String(v);
- if (o.node.data !== str) o.node.data = str;
- next.push(o);
+ try {
+ for (let i = 0; i < value.length; i++) {
+ const v = value[i];
+ const o = old[i];
+ if (isTemplate(v)) {
+ const tr = /** @type any */ (v);
+ if (o && o.type === 'tpl' && o.inst.strings === tr.strings) {
+ updateInstance(o.inst, tr.values);
+ next.push(o);
+ consumed = i + 1;
+ continue;
+ }
+ } else if (v != null && v !== false && v !== true) {
+ if (o && o.type === 'text') {
+ const str = String(v);
+ if (o.node.data !== str) o.node.data = str;
+ next.push(o);
+ consumed = i + 1;
+ continue;
+ }
+ } else {
+ // Empty slot: drop any prior nodes that occupied this position.
+ // Deliberately NOT reordered like the branch below. That reorder
+ // exists to keep a slot that was already BUILT and INSERTED tracked,
+ // and this branch builds and inserts nothing, so pushing first would
+ // only mean a throw from the removal leaves a phantom empty slot at
+ // this index AND `old[i]` spliced in at the next one, shifting every
+ // later slot by one in a POSITIONAL reconciler. Removing first, a
+ // throw here leaves `old[i]` describing its own position, which is
+ // still exactly where its nodes are.
+ if (o) removeArrayItem(o);
+ next.push({ type: 'empty' });
+ consumed = i + 1;
continue;
}
- } else {
- // Empty slot: drop any prior nodes that occupied this position.
+ // Shape changed, or the array grew past the old length. Build fresh,
+ // insert at this position (before the current / next still-attached
+ // old node, else the marker), then drop the old slot it replaced.
+ // The push sits BEFORE the removal, which is a pure reordering on the
+ // success path and means a slot that has already been built and
+ // inserted is never untracked at any throw point.
+ const { item, frag } = buildArrayItem(v);
+ if (frag) parent.insertBefore(frag, nextArrayAnchor(old, i, marker));
+ next.push(item);
if (o) removeArrayItem(o);
- next.push({ type: 'empty' });
- continue;
+ consumed = i + 1;
}
- // Shape changed, or the array grew past the old length. Build fresh,
- // insert at this position (before the current / next still-attached
- // old node, else the marker), then drop the old slot it replaced.
- const { item, frag } = buildArrayItem(v);
- if (frag) parent.insertBefore(frag, nextArrayAnchor(old, i, marker));
- if (o) removeArrayItem(o);
- next.push(item);
- }
- // Shrink: remove slots beyond the new length.
- for (let i = value.length; i < old.length; i++) removeArrayItem(old[i]);
- state.items = next;
+ // Shrink: remove slots beyond the new length.
+ for (let i = value.length; i < old.length; i++) {
+ removeArrayItem(old[i]);
+ consumed = i + 1;
+ }
+ state.items = next;
+ } catch (err) {
+ // `state.items` is committed only after the whole walk, so a throw part
+ // way through discards `next` entirely: the map of slots keeps describing
+ // positions whose nodes were already removed, while the freshly built and
+ // inserted ones are in the document tracked by nothing. Nothing is logged
+ // after the first throw, and the orphan outlives even a render of an EMPTY
+ // array, because the only code that could remove it walks `state.items`.
+ //
+ // Splice the untouched tail of `old` onto what `next` accumulated, so the
+ // bookkeeping describes the DOM again. The invariant that holds at any
+ // throw point: every live node is described by exactly one slot, the
+ // slots below `next.length` being the rebuilt or reused ones and the rest
+ // the part of `old` this pass never reached. Index alignment survives
+ // because this reconciler is POSITIONAL, so a slot's index IS its
+ // identity, which is also why the boundary has to come from `consumed`
+ // rather than `next.length`: during the shrink loop those differ, and
+ // splicing from `next.length` would re-describe slots already removed.
+ // A later render that grew the array would then match a live value
+ // against a DETACHED slot with the same `strings`, update it in place,
+ // and that row would silently never appear.
+ //
+ // Deliberately NOT a teardown-and-rebuild of the region, for the reason
+ // recorded on `reconcileRepeat`'s catch above: it discards node identity
+ // for every row, which cancels an in-progress native drag and drops focus
+ // and scroll.
+ //
+ // The residual is a throw from the removal step itself, which takes a
+ // throwing DOM to reach (the teardown it calls is total, so only
+ // `removeBetween` is left, and that calls `removeChild` solely on nodes
+ // the renderer owns). State it rather than deny it: `removeBetween`
+ // takes the start marker first and then early-returns for good once that
+ // marker is gone, so a row whose removal refused part-way can never be
+ // removed afterwards. Its remaining nodes stay in the document, and an
+ // EMPTY render will not clear them. Tracked or not, they are there for
+ // the life of the region, the same residual `reconcileRepeat`'s catch
+ // names. What this repair buys is that there is only ONE such row and
+ // every other slot still reconciles, where before the whole pass was
+ // discarded.
+ state.items = next.concat(old.slice(consumed));
+ throw err;
+ }
}
/**
diff --git a/packages/core/test/rendering/browser/directive-commit-throw.test.js b/packages/core/test/rendering/browser/directive-commit-throw.test.js
index a2e7301c4..ea27437f3 100644
--- a/packages/core/test/rendering/browser/directive-commit-throw.test.js
+++ b/packages/core/test/rendering/browser/directive-commit-throw.test.js
@@ -16,7 +16,7 @@
import { html } from '../../../src/html.js';
import { render } from '../../../src/render-client.js';
import { repeat } from '../../../src/repeat.js';
-import { watch } from '../../../src/directives.js';
+import { watch, ref } from '../../../src/directives.js';
import { signal } from '../../../src/signal.js';
import { WebComponent } from '../../../src/component.js';
@@ -215,6 +215,108 @@ suite('directive commit throws (browser)', () => {
assert.strictEqual(after[2], before[2]);
});
+ test('a throwing ref unbind removes the row, and re-adding it builds a new element', () => {
+ // The identity facts linkedom cannot prove: the survivor is MOVED rather
+ // than rebuilt, and the resurrected key is a genuinely new element rather
+ // than the disposed instance handed back.
+ const boom = { set value(v) { if (v === undefined) throw new Error('ref-boom'); }, get value() { return null; } };
+ const refRows = (items) => html`${repeat(
+ items,
+ (it) => it.id,
+ (it) => html`- ${it.n}
`,
+ )}
`;
+
+ render(refRows([{ id: 1, n: 'a' }, { id: 9, n: 'doomed' }]), container);
+ const before = [...container.querySelectorAll('li')];
+
+ render(refRows([{ id: 1, n: 'a' }]), container);
+ assert.deepEqual([...container.querySelectorAll('li')].map((li) => li.textContent), ['a']);
+ assert.strictEqual(container.querySelector('li'), before[0]);
+
+ render(refRows([{ id: 1, n: 'a' }, { id: 9, n: 'again' }]), container);
+ const after = [...container.querySelectorAll('li')];
+ assert.deepEqual(after.map((li) => li.textContent), ['a', 'again']);
+ assert.strictEqual(after[0], before[0]);
+ assert.ok(after[1] !== before[1], 'the disposed instance must not be resurrected');
+ });
+
+ test('a refused DOM removal keeps that row keyed, and does not duplicate it', () => {
+ // The ref-unbind case above cannot reach the removal loop's own shape,
+ // because the guard makes that step unable to throw at all. This drives
+ // the throw from the DOM removal instead, in a real browser, where node
+ // identity is the thing that separates "reused the row already there"
+ // from "built a second one beside it".
+ const idRows = (items) => html`${repeat(
+ items,
+ (it) => it.id,
+ (it) => html`- ${it.n}
`,
+ )}
`;
+
+ render(idRows([{ id: 1, n: 'one' }, { id: 2, n: 'two' }, { id: 3, n: 'three' }]), container);
+ const [liOne, liTwo, liThree] = [...container.querySelectorAll('li')];
+
+ const ul = container.querySelector('ul');
+ const origRemove = ul.removeChild.bind(ul);
+ ul.removeChild = (node) => {
+ if (node === liThree) throw new Error('rm-boom');
+ return origRemove(node);
+ };
+ throwsMatching(() => { render(idRows([{ id: 1, n: 'one' }]), container); }, /rm-boom/);
+ ul.removeChild = origRemove;
+
+ render(idRows([{ id: 1, n: 'one' }, { id: 2, n: 'two' }]), container);
+ const after = [...container.querySelectorAll('li')];
+
+ // Key 1 never left the map, so it is the same element. Key 2 left the map
+ // together with its row, so it MISSES and rebuilds rather than having a
+ // disposed instance handed back.
+ assert.strictEqual(after.filter((li) => li === liOne).length, 1);
+ assert.ok(!after.includes(liTwo), 'a removed row must not be resurrected');
+ assert.strictEqual(after.filter((li) => li.textContent === 'two').length, 1);
+
+ // `liThree` is the named residual: the DOM removal itself refused, so
+ // those nodes stayed, and its key was already dropped, so nothing tracks
+ // them. What the trade buys is that the region still RECONCILES, which is
+ // the assertion that matters and the one only a real browser settles.
+ assert.strictEqual(liThree.parentNode, ul);
+ render(idRows([{ id: 1, n: 'one' }, { id: 2, n: 'two' }, { id: 4, n: 'four' }]), container);
+ assert.deepEqual(
+ [...container.querySelectorAll('li')].map((li) => li.textContent).filter((t) => t !== 'three'),
+ ['one', 'two', 'four'],
+ );
+ });
+
+ test('a plain .map() array recovers a shape-changed row without losing identity', () => {
+ // The non-keyed reconciler updates in place, so the rows that were NOT
+ // rebuilt must survive the recovery as the same elements. linkedom can
+ // show the markup is right; only a real DOM can show it was repaired
+ // rather than rebuilt.
+ const view = (items) => html`${items.map((it) => (
+ it.kind === 'a' ? html`
${it.v}
` : html`
${it.v}`
+ ))}
${repeat(
+ items,
+ (it) => it.id,
+ (it) => html`- ${it.n}
`,
+ )}
`;
+
+ render(rows([{ id: 1, n: 'a' }, { id: 9, n: 'doomed' }]), container);
+ assert.deepEqual(rowTexts(container), ['a', 'doomed']);
+ const beforeA = container.querySelector('li');
+
+ // The app asked for a one-row list. It used to get a two-row list led by
+ // the row it deleted, because the unbind threw out of the removal loop.
+ render(rows([{ id: 1, n: 'a' }]), container);
+ assert.deepEqual(rowTexts(container), ['a']);
+
+ // And the survivor is the same element, not a rebuild.
+ assert.equal(container.querySelector('li'), beforeA);
+
+ // Still reconciling normally afterwards, rather than wedged.
+ render(rows([{ id: 1, n: 'A' }]), container);
+ assert.deepEqual(rowTexts(container), ['A']);
+});
+
+test('repeat: re-adding a key dropped through a throwing unbind builds a fresh row', () => {
+ const container = document.createElement('div');
+ const rows = (items) => html`${repeat(
+ items,
+ (it) => it.id,
+ (it) => html`- ${it.n}
`,
+ )}
`;
+
+ render(rows([{ id: 1, n: 'a' }, { id: 9, n: 'doomed' }]), container);
+ const doomed = [...container.querySelectorAll('li')][1];
+
+ render(rows([{ id: 1, n: 'a' }]), container);
+ render(rows([{ id: 1, n: 'a' }, { id: 9, n: 'again' }]), container);
+
+ assert.deepEqual(rowTexts(container), ['a', 'again']);
+ // A disposed, detached instance must never come back: it is unmapped, so
+ // the key misses and builds fresh.
+ const readded = [...container.querySelectorAll('li')][1];
+ assert.notEqual(readded, doomed);
+});
+
+test('repeat: a throw INSIDE the removal loop leaves no leftover still mapped', () => {
+ // Drives the throw from the loop's OTHER step, so this covers the loop's
+ // shape independently of the ref guard above (which makes the dispose step
+ // unable to throw at all). Nothing here reaches into module internals: it
+ // patches the rows' parent so one DOM removal refuses.
+ const container = document.createElement('div');
+ const rows = (items) => html`${repeat(
+ items,
+ (it) => it.id,
+ (it) => html`- ${it.n}
`,
+ )}
`;
+
+ render(rows([{ id: 1, n: 'one' }, { id: 2, n: 'two' }, { id: 3, n: 'three' }]), container);
+ const [, liTwo, liThree] = [...container.querySelectorAll('li')];
+
+ const ul = container.querySelector('ul');
+ const origRemove = ul.removeChild.bind(ul);
+ ul.removeChild = (node) => {
+ if (node === liThree) throw new Error('rm-boom');
+ return origRemove(node);
+ };
+
+ // Drop keys 2 and 3. Key 2 is processed cleanly; key 3's DOM removal
+ // refuses part-way.
+ assert.throws(() => { render(rows([{ id: 1, n: 'one' }]), container); }, /rm-boom/);
+ ul.removeChild = origRemove;
+
+ // `liThree` is the named residual: `removeBetween` itself refused, so those
+ // nodes stayed, and the key was already dropped, so nothing tracks them.
+ assert.equal(liThree.parentNode, ul);
+
+ // Re-add BOTH dropped keys in ONE render, with no render in between. That
+ // ordering is load-bearing rather than incidental: any render that treats
+ // key 3 as a leftover again unmaps it under EITHER unmap ordering (its
+ // start marker is gone, so `removeBetween` early-returns and the delete
+ // runs), which collapses the difference this test exists to catch. Re-added
+ // immediately, key 3 is already unmapped, so it MISSES and builds a fresh
+ // row beside the remnant. Unmapping after the removal instead would keep
+ // that key pointing at a half-removed row, the re-add would take the reuse
+ // branch, and `moveRange` would re-attach the lone start marker AFTER its
+ // own end marker.
+ render(rows([{ id: 1, n: 'one' }, { id: 2, n: 'two' }, { id: 3, n: 'three' }]), container);
+ const at = (text) => [...container.querySelectorAll('li')].filter((li) => li.textContent === text);
+
+ assert.equal(at('one').length, 1);
+ assert.equal(at('two').length, 1, 'exactly one row for the re-added key');
+ assert.notEqual(at('two')[0], liTwo, 'a disposed instance must not be resurrected');
+ assert.equal(at('three').length, 2, 'a fresh row for the re-added key, beside the remnant');
+
+ // And the region is still alive. Under the other ordering the removal below
+ // walks off the end of the mis-ordered range and takes the repeat part's
+ // own marker with it, after which no render ever lands again.
+ render(rows([{ id: 1, n: 'one' }, { id: 2, n: 'two' }, { id: 4, n: 'four' }]), container);
+ assert.deepEqual(
+ [...container.querySelectorAll('li')].map((li) => li.textContent).filter((t) => t !== 'three'),
+ ['one', 'two', 'four'],
+ 'the region still reconciles rather than being dead',
+ );
+});
+
+test('clearInstance: a throwing ref unbind does not wedge template swaps', () => {
+ const container = document.createElement('div');
+ render(html`A
`, container);
+ assert.equal(container.querySelector('p')?.textContent, 'A');
+
+ // A template-SHAPE swap runs the container-level teardown. The throw used
+ // to skip the replaceChildren() at the end of it, so the old DOM stayed and
+ // the instance was never replaced. `lastTarget` is cleared only after the
+ // throwing write, so it was permanent: every later swap threw at the same
+ // part, which is why this asserts TWO swaps.
+ render(html`B`, container);
+ assert.equal(container.querySelector('b')?.textContent, 'B');
+ assert.equal(container.querySelector('p'), null);
+
+ render(html`C`, container);
+ assert.equal(container.querySelector('i')?.textContent, 'C');
+});
+
test('a plain template child hole recovers too (not just repeat)', () => {
const container = document.createElement('div');
const view = (x) => html`${x}
`;
@@ -178,6 +316,111 @@ test('a plain template child hole recovers too (not just repeat)', () => {
assert.equal(container.querySelector('span').textContent, 'ok');
});
+// --- plain .map() arrays (the non-keyed child reconciler) ---
+
+/** Rendered markup of the array region, with the renderer's markers stripped. */
+function regionHTML(container) {
+ return container.querySelector('div').innerHTML.replace(//g, '');
+}
+
+// The REPLACE branch is the destructive one: it inserts the replacement and
+// removes the old slot BEFORE the walk can finish. An in-place update touches
+// only values and cannot reach it. Three different things route there, and
+// the cases below cover more than one, because narrowing this to "the
+// template shape changed" would leave the others untested: the item's
+// template shape changed, its slot KIND changed (text, template, empty), or
+// the array GREW past the old length, where there is no old slot to compare
+// against at all.
+
+test('array: a mid-walk throw does not strand a row on the next valid render', () => {
+ const container = document.createElement('div');
+ const view = (items) => html`${items.map((it) => (
+ it.kind === 'a' ? html`
${it.v}
` : html`
${it.v}`
+ ))}
1
2
');
+
+ // Item 0 changes shape (destructive) and item 1's child hole then throws.
+ assert.throws(() => {
+ render(view([{ kind: 'b', v: '1' }, { kind: 'a', v: poison }]), container);
+ }, /boom/);
+
+ // The freshly built used to be tracked by nothing, so it survived
+ // alongside a rebuilt copy of itself: 112
.
+ render(view([{ kind: 'b', v: '1' }, { kind: 'a', v: '2' }]), container);
+ assert.equal(regionHTML(container), '12
');
+
+ // Not merely delayed by one render.
+ render(view([{ kind: 'b', v: '1' }, { kind: 'a', v: '2' }]), container);
+ assert.equal(regionHTML(container), '12
');
+});
+
+test('array: a GROWN array reaches the same branch, with no shape change at all', () => {
+ // Every item here is the same template shape, so nothing about this render
+ // is a "shape change". The new tail index simply has no old slot to reuse,
+ // which routes it to the same build-insert-remove branch.
+ const container = document.createElement('div');
+ const view = (items) => html`${items.map((v) => html`
${v}
`)}
1
2
3
');
+ render(view([]), container);
+ assert.equal(regionHTML(container), '');
+});
+
+test('array: an EMPTY render after a throw leaves nothing behind', () => {
+ const container = document.createElement('div');
+ const view = (items) => html`${items.map((it) => (
+ it.kind === 'a' ? html`
${it.v}
` : html`
${it.v}`
+ ))}
${items.map((v) => html`
${v}
`)}
A directive that throws mid-commit stays consistent
The component boundary above also covers watch(signal) and until(), which commit outside the update cycle, so a throw from either reaches renderError() rather than the window. It reaches the component whose template holds the binding, which is not always the element the binding sits inside: a watch() written between a child component's tags belongs to the parent that wrote it. asyncAppend / asyncReplace are not covered, in two ways: that path logs its own iteration throw and continues on purpose, and a watch() or until() nested inside a chunk it commits still reaches the window.
- Beyond reporting the error, the directive's own state is left describing the DOM that actually exists, which is what makes the NEXT render correct. That matters because the failure is otherwise silent: the renders that expose it are fully valid and log nothing. The hole whose commit threw is marked so the next render re-applies it instead of skipping it as unchanged, which is what used to leave a region blank for good. repeat() additionally repairs its key map so it describes the DOM again, and the next render is an ordinary reconcile that repositions every row (the symptom was a permanently duplicated row); it deliberately does not rebuild the region, which would throw away the node identity keyed reconciliation exists to preserve. guard() records its new deps only once the commit succeeds, so a later render with those deps re-renders the region instead of skipping past one the throw had blanked; until() advances its resolved priority only after its commit succeeds, so a failed high-priority resolution does not refuse the lower-priority one behind it.
+ Beyond reporting the error, the directive's own state is left describing the DOM that actually exists, which is what makes the NEXT render correct. That matters because the failure is otherwise silent: the renders that expose it are fully valid and log nothing. The hole whose commit threw is marked so the next render re-applies it instead of skipping it as unchanged, which is what used to leave a region blank for good. Both list reconcilers additionally repair their own bookkeeping so it describes the DOM again, and the next render is an ordinary reconcile rather than a rebuild of the region, which would throw away the node identity they exist to preserve. repeat() re-unites its key map and repositions every row (the symptom was a permanently duplicated row). A plain .map() array splices back the part of its slot list the failed pass never reached, which is what a slot REPLACED rather than updated in place needs (its template shape changed, its kind changed between text, template and empty, or the array grew past its old length), since that is the branch that inserts the replacement before removing what it replaced (the symptom was a stranded row that outlived even a render of an empty array). guard() records its new deps only once the commit succeeds, so a later render with those deps re-renders the region instead of skipping past one the throw had blanked; until() advances its resolved priority only after its commit succeeds, so a failed high-priority resolution does not refuse the lower-priority one behind it.
+ Tearing content back out is covered too, and it has to be, because a teardown has no next render to repair it. Unbinding a ref while a row is removed can never abort the removal of the rest of the list, and repeat() drops each leftover key from its map before touching that row, so the map never describes a row that has already been removed. Without that, a throw part-way through left the row you had DELETED on screen, reordered the survivors, and let a later render that re-added the key reinsert the disposed instance. The cost is that a ref whose object value setter throws is swallowed on teardown, matching the ref callback, which was already swallowed everywhere (lit guards neither and propagates from both, so this is a deliberate divergence). It applies to teardown only: on the COMMIT path a throwing object-ref setter still reaches renderError().
Server action errors
Errors thrown from server actions are sanitized in production: the client gets a generic "Internal server error" message plus a short digest, never the raw thrown message or the stack trace. The full error is logged server-side keyed by that digest, so a client-reported digest maps back to the server log line. A redirect() / notFound() control-flow throw passes through. To surface a specific user-facing message, return an ActionResult { success: false, error } envelope instead of throwing.