diff --git a/.agents/skills/webjs/references/components.md b/.agents/skills/webjs/references/components.md index aae9eab63..1f98b20d7 100644 --- a/.agents/skills/webjs/references/components.md +++ b/.agents/skills/webjs/references/components.md @@ -183,7 +183,7 @@ The boundary also covers `watch(signal)` (its notify microtask) and `until()` (i **A commit that throws leaves the directive's own state consistent, so the NEXT valid render is correct.** This matters because the corruption is otherwise silent: the renders that expose it are fully valid and log nothing after the first throw. The hole whose commit threw is marked so the next render re-applies it rather than skipping it as unchanged (its recorded value is never advanced past a throw, and would otherwise match exactly what the recovering render supplies, leaving a child 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 discard the node identity the reconcilers exist to preserve. `repeat()` re-unites its key map and repositions every row (the failure was a permanently duplicated row). A plain `.map()` array splices the part of its slot list the failed pass never reached back on, which matters whenever a slot is REPLACED rather than updated in place (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 failure 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 same deps re-renders the region instead of short-circuiting past a region the throw had blanked; `until()` advances its resolved priority only after the commit succeeds, so a failed high-priority resolution does not refuse the lower-priority one behind it. -**Teardown is total as well.** Removing a row is not a commit and has no retry, so a throw while tearing one down cannot be allowed to abandon the rest. Unbinding a `ref` during teardown can never abort the removal of the remaining rows, 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 (which used to leave the row the app DELETED on screen, reorder the survivors, and let a later render that re-added that key reinsert the disposed instance). To make that hold, a `ref` whose object `value` setter throws is now SWALLOWED on teardown, matching the ref CALLBACK, which was already swallowed everywhere. That is a deliberate divergence from lit, which guards neither and propagates from both. It applies to teardown only: on the COMMIT path a throwing object-ref setter still reaches `renderError()`, because there the boundary can report it and the next render can repair it. +**Teardown is total as well.** Removing a row is not a commit and has no retry, so a throw while tearing one down cannot be allowed to abandon the rest. Unbinding a `ref` during teardown can never abort the removal of the remaining rows, 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 (which used to leave the row the app DELETED on screen, reorder the survivors, and let a later render that re-added that key reinsert the disposed instance). To make that hold, a `ref` whose object `value` setter throws is now SWALLOWED on teardown, matching the ref CALLBACK, which was already swallowed everywhere. That is a deliberate divergence from lit, which guards neither and propagates from both. It applies to teardown only: on the COMMIT path a throwing object-ref setter still reaches `renderError()`, because there the boundary can report it and the next render can repair it. Total also means a removal takes the row's own boundary markers with it, so a list that grows and shrinks all day is net zero on the nodes the renderer added, rather than accruing one invisible comment per removed row for the life of the region. Decision rules. Use `async render()` for request-time server data that should be in the first paint (the default). Add `renderFallback()` when a client re-fetch's stale content would mislead. Use `Task` / signals for genuinely client-only data (a click, viewport, live updates). For SLOW data where blocking the first byte hurts, wrap the region in `` to stream it (the only way to show a first-paint fallback; see `client-router-and-streaming.md`). Do NOT fetch in `connectedCallback` for data knowable server-side, and do NOT prop-drill what a leaf can fetch itself. diff --git a/packages/core/src/render-client.js b/packages/core/src/render-client.js index e4012c8c1..0613ef7e4 100644 --- a/packages/core/src/render-client.js +++ b/packages/core/src/render-client.js @@ -1701,16 +1701,46 @@ function nodesToFrag(nodes) { return frag; } -/** @param {Node} start @param {Node} end */ +/** + * Remove a template instance's whole range, its bookend markers INCLUDED. + * + * Every caller discards the instance right after: the map entry or slot that + * held it is dropped, and a replacement, where there is one, is built with + * markers of its own. So this is a REMOVE, never lit's clear-and-reuse. A + * caller that wants to keep the bookends and render into them again needs its + * OWN function, because the two want opposite answers for the end marker. + * + * `parent` is read BEFORE the walk because the walk removes `start` on its + * first iteration, which nulls `start.parentNode`. Reading it afterwards + * compared the end marker's live parent against `null`, so the guard could + * never fire and every teardown left one `wjm-e` comment in the document, + * unbounded for the life of the region. + * + * The `end.parentNode === parent` comparison is a refusal, not a formality. A + * marker moved under a different parent is not this region's to remove, and + * `parent.removeChild(end)` on it throws NotFoundError from inside a teardown + * that has to stay total. + * + * The walk assumes the range is INTACT: it steps `nextSibling` from `start` + * and stops on `end`, so a range whose end no longer follows its start runs + * off the child list and takes the part's own marker with it. That is not + * something this function defends against, before or after the parent capture, + * and the consequence is spelled out where it bites, on `reconcileRepeat`'s + * catch below. The guard is the narrower promise: whatever the walk did, a + * marker that is somewhere else is left alone. + * + * @param {Node} start @param {Node} end + */ function removeBetween(start, end) { - if (!start.parentNode) return; + const parent = start.parentNode; + if (!parent) return; let n = start; while (n && n !== end) { const next = n.nextSibling; n.parentNode?.removeChild(n); n = next; } - if (end.parentNode === start.parentNode) end.parentNode?.removeChild(end); + if (end.parentNode === parent) parent.removeChild(end); } /* ================================================================ diff --git a/packages/core/test/directives/browser/directives-cache.test.js b/packages/core/test/directives/browser/directives-cache.test.js index 4b5328107..bf9986a44 100644 --- a/packages/core/test/directives/browser/directives-cache.test.js +++ b/packages/core/test/directives/browser/directives-cache.test.js @@ -22,8 +22,11 @@ import { cache } from '../../../src/directives.js'; import { assert } from '../../../../../test/browser-assert.js'; /** - * Strip webjs marker comments (the framework injects `` - * style comments around dynamic parts; tests assert plain HTML). + * Strip the renderer's marker comments (it brackets each instance with a + * `wjm-s` / `wjm-e` pair and marks each hole with a `wjm-N`; tests here assert + * plain HTML). Note this hides marker ACCOUNTING by construction, so it cannot + * be used to show markers are balanced. Assertions about that live in + * `rendering/marker-leak-on-teardown.test.js`, which counts them directly. */ function stripExpressionComments(s) { return s.replace(//g, ''); diff --git a/packages/core/test/rendering/browser/marker-leak-on-teardown.test.js b/packages/core/test/rendering/browser/marker-leak-on-teardown.test.js new file mode 100644 index 000000000..3dcc498d4 --- /dev/null +++ b/packages/core/test/rendering/browser/marker-leak-on-teardown.test.js @@ -0,0 +1,173 @@ +/** + * Real-browser assertions for the teardown that takes its own end marker. + * + * The unit suite proves the accounting under linkedom, driving the renderer + * with bare `render()` calls. Two things are left for a real engine. A + * component reaches the same teardown through the update pipeline instead, so + * the leak is shown on the path an app actually takes. And `removeChild` runs + * a custom element's `disconnectedCallback` synchronously, which means author + * code lands part-way through the removal walk; that is worth pinning on the + * engines people run rather than on a shim, and no unit case covers it. + * + * (linkedom does fire `disconnectedCallback`, so the second one is a question + * of fidelity rather than of capability. Do not write the stronger claim.) + */ +import { html } from '../../../src/html.js'; +import { MARKER } from '../../../src/html.js'; +import { repeat } from '../../../src/repeat.js'; +import { WebComponent } from '../../../src/component.js'; + +import { assert } from '../../../../../test/browser-assert.js'; + +let uid = 0; +const tagName = (p) => `${p}-${uid++}`; + +const tick = () => new Promise((r) => queueMicrotask(() => queueMicrotask(r))); + +/** Count renderer bookends under `root`, at any depth. */ +function countBookends(root) { + let s = 0; + let e = 0; + const walk = (n) => { + for (const c of n.childNodes) { + if (c.nodeType === 8) { + if (c.data === `${MARKER}s`) s++; + else if (c.data === `${MARKER}e`) e++; + } else walk(c); + } + }; + walk(root); + return { s, e }; +} + +const rows = (n) => Array.from({ length: n }, (_, i) => ({ id: i, t: `row ${i}` })); + +suite('teardown takes its own end marker', () => { + test('a component churning a repeat() list stays flat on marker count', async () => { + const tag = tagName('marker-churn'); + class C extends WebComponent({ items: Array }) { + constructor() { + super(); + this.items = rows(4); + } + render() { + return html``; + } + } + C.register(tag); + + const host = document.createElement(tag); + document.body.appendChild(host); + await host.updateComplete; + await tick(); + + try { + const baseline = countBookends(host); + assert.equal(baseline.s, baseline.e, 'baseline is paired'); + assert.ok(baseline.e > 0, 'the list really rendered bookends'); + const survivor = host.querySelector('li'); + + for (let i = 0; i < 5; i++) { + host.items = rows(2); + await host.updateComplete; + host.items = rows(4); + await host.updateComplete; + } + await tick(); + + const got = countBookends(host); + assert.equal(got.s, got.e, `bookends unpaired: ${got.s} start, ${got.e} end`); + assert.equal(got.e, baseline.e, 'end markers drifted from baseline'); + assert.equal(host.querySelectorAll('li').length, 4, 'all four rows rendered'); + assert.equal(host.querySelector('li'), survivor, 'a row that never left kept its identity'); + } finally { + host.remove(); + } + }); + + test('a disconnectedCallback running mid-walk does not derail the removal', async () => { + // This reds on the reverted fix like the case above (it counts bookends + // too), but that is not what it is for. It exists to prove the removal + // finishes, and the region stays renderable, while a `disconnectedCallback` + // runs synchronously in the middle of the walk, on the engines people + // actually run. + // + // The callback here writes OUTSIDE the range being torn down, which is what + // ordinary author code does. It deliberately does not move a node the walk + // is about to step onto: a range mutated underneath the walk overruns, that + // is true before and after this fix, and this case is not the place to + // claim otherwise. + const seen = []; + // A fixed tag, because a tag NAME is not a hole position in an `html` + // template; only attribute and child positions are. + const cellTag = 'marker-leak-cell'; + class Cell extends WebComponent({ label: String }) { + disconnectedCallback() { + super.disconnectedCallback?.(); + // Author code, running synchronously from inside `removeChild`, that + // writes to a container OUTSIDE the range being torn down. + seen.push(this.label); + const note = document.createElement('i'); + note.className = 'gone'; + note.textContent = this.label; + document.querySelector('#marker-sink')?.appendChild(note); + } + render() { + return html`${this.label}`; + } + } + Cell.register(cellTag); + + const tag = tagName('marker-host'); + class C extends WebComponent({ items: Array }) { + constructor() { + super(); + this.items = rows(4); + } + render() { + return html``; + } + } + C.register(tag); + + const sink = document.createElement('div'); + sink.id = 'marker-sink'; + document.body.appendChild(sink); + const host = document.createElement(tag); + document.body.appendChild(host); + await host.updateComplete; + await tick(); + + try { + const baseline = countBookends(host); + assert.equal(baseline.s, baseline.e, 'baseline is paired'); + assert.ok(baseline.e > 0, 'the list really rendered bookends to count'); + + host.items = rows(1); + await host.updateComplete; + await tick(); + + assert.ok(seen.length > 0, 'a disconnectedCallback really ran during the removal'); + assert.equal(host.querySelectorAll('li').length, 1, 'the shrink completed'); + assert.equal(host.querySelector('li span').textContent, 'row 0', 'the survivor is intact'); + + // The region is still usable: the part marker survived the walk, so a + // later grow renders back into the same place. + host.items = rows(4); + await host.updateComplete; + await tick(); + + assert.equal(host.querySelectorAll('li').length, 4, 'the region still renders after the walk'); + const got = countBookends(host); + assert.equal(got.s, got.e, `bookends unpaired: ${got.s} start, ${got.e} end`); + assert.equal(got.e, baseline.e, 'end markers drifted from baseline'); + } finally { + host.remove(); + sink.remove(); + } + }); +}); diff --git a/packages/core/test/rendering/marker-leak-on-teardown.test.js b/packages/core/test/rendering/marker-leak-on-teardown.test.js new file mode 100644 index 000000000..a8f72f6d7 --- /dev/null +++ b/packages/core/test/rendering/marker-leak-on-teardown.test.js @@ -0,0 +1,279 @@ +/** + * Tearing an instance out takes its OWN bookend markers with it. + * + * Every rendered template instance is bracketed by a `wjm-s` / `wjm-e` comment + * pair, and `removeBetween` is what takes a range back out. It used to remove + * the start marker, the content, and then skip the end marker, because the + * guard deciding whether to remove it read `start.parentNode` AFTER the walk + * had already detached `start`. One orphan comment per teardown, accumulating + * for the life of the region. + * + * The leak is invisible to `textContent` and to `querySelectorAll`, which is + * exactly why nothing caught it for so long. So the six LEAK cases count + * comment nodes directly, and assert BOTH halves of the accounting: the pair + * count is balanced (`s === e`), and it is back to the baseline the first + * render established. Each also asserts the rendered output, so none of them + * can pass by rendering nothing at all. They cover every `removeBetween` + * caller, since each reaches it through a different path and a repair on one + * branch says nothing about the others (the two array cases share a caller, + * one shrinking the list and one emptying it). + * + * The LAST case is not one of those and does not count markers at all. It + * pins the guard, which is a separate decision from the leak, and it is green + * with or without the fix by design. Its own comment says so. + */ +import { test, before } from 'node:test'; +import assert from 'node:assert/strict'; +import { parseHTML } from 'linkedom'; + +before(() => { + const { window } = parseHTML(''); + globalThis.document = window.document; + globalThis.DocumentFragment = window.DocumentFragment; + globalThis.Node = window.Node; + globalThis.Element = window.Element; + globalThis.Comment = window.Comment; + globalThis.Text = window.Text; + globalThis.NodeFilter = window.NodeFilter; + globalThis.HTMLElement = window.HTMLElement; +}); + +let html, render, repeat, MARKER; +before(async () => { + ({ html, MARKER } = await import('../../src/html.js')); + ({ render } = await import('../../src/render-client.js')); + ({ repeat } = await import('../../src/repeat.js')); +}); + +/** + * Count renderer bookends under `root`, at any depth. + * + * It reads MARKER for the prefix but spells the `s` / `e` suffix here, so it + * tracks only half of the marker text and a rename of the other half would + * leave it counting zero. That is what `baselineBookends` is for: nothing in + * this file compares two counts without first proving the baseline is + * non-zero, so a counter that has stopped seeing anything fails instead of + * passing on `0 === 0`. + */ +function countBookends(root) { + let s = 0; + let e = 0; + const walk = (n) => { + for (const c of n.childNodes) { + if (c.nodeType === 8) { + if (c.data === `${MARKER}s`) s++; + else if (c.data === `${MARKER}e`) e++; + } else walk(c); + } + }; + walk(root); + return { s, e }; +} + +/** + * Capture the post-first-render count every leak case measures against, and + * refuse a baseline that is empty or unpaired. Without the non-zero arm a + * counter that matches nothing makes every later comparison `0 === 0`, and + * the case passes while observing exactly nothing. + */ +function baselineBookends(el, label) { + const got = countBookends(el); + assert.equal(got.s, got.e, `${label}: baseline unpaired (${got.s} start, ${got.e} end)`); + assert.ok(got.e > 0, `${label}: baseline counted no bookends, so nothing below can fail`); + return got; +} + +/** Assert the bookends are paired AND back to `baseline`. */ +function assertBookends(el, baseline, label) { + const got = countBookends(el); + assert.equal(got.s, got.e, `${label}: bookends unpaired (${got.s} start, ${got.e} end)`); + assert.deepEqual(got, baseline, `${label}: bookend count drifted from baseline`); +} + +const texts = (el, sel) => [...el.querySelectorAll(sel)].map((n) => n.textContent); +const rows = (n) => Array.from({ length: n }, (_, i) => ({ id: i, t: `row ${i}` })); + +/* ------------------------------------------------------------------ * + * U1: the leftover-removal loop in reconcileRepeat. + * ------------------------------------------------------------------ */ + +test('repeat(): rows removed across many cycles leave no orphan markers', () => { + const el = document.createElement('div'); + const view = (items) => + html``; + + render(view(rows(4)), el); + const baseline = baselineBookends(el, 'repeat'); + // The row at index 0 is present in every render below, so keyed + // reconciliation must hand back the SAME element each time. This is the + // teardown-only guarantee: removing rows 2 and 3 must not perturb the rows + // that stayed. + const survivor = el.querySelector('li'); + + for (let i = 0; i < 5; i++) { + render(view(rows(2)), el); + render(view(rows(4)), el); + } + + assertBookends(el, baseline, 'repeat after 5 shrink/grow cycles'); + assert.deepEqual(texts(el, 'li'), ['row 0', 'row 1', 'row 2', 'row 3']); + assert.equal(el.querySelector('li'), survivor, 'a row that never left kept its node identity'); +}); + +/* ------------------------------------------------------------------ * + * U2: the shape-mismatch branch in reconcileRepeat, where a key SURVIVES + * but its template shape changes, so the old instance is torn out and a + * fresh one built under the same key. + * ------------------------------------------------------------------ */ + +test('repeat(): a same-key template swap leaves no orphan markers', () => { + const el = document.createElement('div'); + const view = (flip) => + html``; + + render(view(false), el); + const baseline = baselineBookends(el, 'same-key swap'); + + for (let i = 0; i < 5; i++) { + render(view(true), el); + render(view(false), el); + } + + assertBookends(el, baseline, 'repeat after 5 same-key shape swaps'); + assert.deepEqual(texts(el, 'li b'), ['row 0', 'row 1', 'row 2']); +}); + +/* ------------------------------------------------------------------ * + * U3: removeArrayItem, the plain .map() path. A different reconciler + * (positional, not keyed) reaching the same helper. + * ------------------------------------------------------------------ */ + +test('plain .map() array: shrinking the array leaves no orphan markers', () => { + const el = document.createElement('div'); + const view = (n) => html``; + + render(view(4), el); + const baseline = baselineBookends(el, 'array shrink'); + + for (let i = 0; i < 5; i++) { + render(view(1), el); + render(view(4), el); + } + + assertBookends(el, baseline, 'array after 5 shrink/grow cycles'); + assert.deepEqual(texts(el, 'li'), ['row 0', 'row 1', 'row 2', 'row 3']); +}); + +test('plain .map() array: emptying it entirely leaves no orphan markers', () => { + const el = document.createElement('div'); + const view = (n) => html``; + + render(view(3), el); + const baseline = baselineBookends(el, 'array empty'); + + for (let i = 0; i < 5; i++) { + render(view(0), el); + render(view(3), el); + } + + assertBookends(el, baseline, 'array after 5 empty/refill cycles'); + assert.deepEqual(texts(el, 'li'), ['row 0', 'row 1', 'row 2']); +}); + +/* ------------------------------------------------------------------ * + * U4: applyChildInnerRaw, a template-shape swap at a plain child hole + * (no list involved). + * ------------------------------------------------------------------ */ + +test('child hole: swapping template shape leaves no orphan markers', () => { + const el = document.createElement('div'); + // ONE outer template site, so the outer instance UPDATES in place and the + // swap really reaches the child-hole teardown. Rendering two separate outer + // literals instead would give the container two different `strings` + // identities, rebuild the whole instance each time, and wipe the leak with + // `replaceChildren` before this could ever see it. + const view = (inner) => html`
${inner}
`; + const a = () => html`A`; + const b = () => html`B`; + + render(view(a()), el); + const baseline = baselineBookends(el, 'child hole swap'); + + for (let i = 0; i < 5; i++) { + render(view(b()), el); + render(view(a()), el); + } + + assertBookends(el, baseline, 'child hole after 5 shape swaps'); + assert.equal(el.querySelector('span').textContent, 'A'); + assert.equal(el.querySelector('em'), null); +}); + +/* ------------------------------------------------------------------ * + * U5: a hole that stops being a list, and becomes one again. Each direction + * takes a different route: list to single tears the repeat down through + * `applyChildInnerRaw`'s own branch, and single to list removes the plain + * instance through `teardownChild`'s `strings` branch. `teardownChild`'s + * repeat branch is NOT what this reaches, and nothing in this file does. + * ------------------------------------------------------------------ */ + +test('child hole: swapping a repeat() out for a single template leaves no orphan markers', () => { + const el = document.createElement('div'); + // One outer template site, for the reason spelled out on the case above. + const view = (inner) => html`
${inner}
`; + const list = () => repeat(rows(3), (it) => it.id, (it) => html`

${it.t}

`); + const single = () => html`solo`; + + render(view(list()), el); + const baseline = baselineBookends(el, 'child hole list/single'); + + for (let i = 0; i < 5; i++) { + render(view(single()), el); + assert.equal(el.querySelector('span').textContent, 'solo'); + render(view(list()), el); + } + + assertBookends(el, baseline, 'child hole after 5 list/single swaps'); + assert.deepEqual(texts(el, 'p'), ['row 0', 'row 1', 'row 2']); +}); + +/* ------------------------------------------------------------------ * + * U6: the guard itself. This one pins a DECISION rather than the fix. + * ------------------------------------------------------------------ */ + +test('a marker moved under a foreign parent is refused, not reached into', () => { + // The removal is deliberately NOT an unconditional `end.remove()`. A marker + // that has been moved somewhere else is not this region's to remove, and + // removing it would rip a node out of a tree the renderer does not own. + // + // What this case is NOT claiming: that the walk copes with a desynced + // range. It does not. It still runs off the end of the child list here, + // taking following siblings with it, before this fix and after it. + // `reconcileRepeat`'s catch describes that same overrun, as the reason it + // rejects an alternative that would leave a start marker stranded past its + // end. Out of scope either way, because the guard's job starts after the + // walk, not during it. + // + // Both assertions below discriminate. linkedom throws for a `removeChild` + // of a node under a different parent, exactly as a browser does, so an + // unguarded `parent.removeChild(end)` reds the first arm, and an + // unconditional `end.remove()` reds the second by stealing the comment. + const el = document.createElement('div'); + const view = (n) => html``; + render(view(2), el); + + const ul = el.querySelector('ul'); + const stolen = [...ul.childNodes].find((n) => n.nodeType === 8 && n.data === `${MARKER}e`); + assert.ok(stolen, 'found an end marker to move'); + + const foreign = document.createElement('section'); + el.appendChild(foreign); + foreign.appendChild(stolen); + + assert.doesNotThrow(() => render(view(0), el)); + assert.equal(stolen.parentNode, foreign, 'the moved marker was left where it was moved to'); +}); diff --git a/packages/core/test/slots/browser/record-self-heal.test.js b/packages/core/test/slots/browser/record-self-heal.test.js index b96b52f87..b725a5d5d 100644 --- a/packages/core/test/slots/browser/record-self-heal.test.js +++ b/packages/core/test/slots/browser/record-self-heal.test.js @@ -8,7 +8,7 @@ * Runs in a REAL browser via WTR + Playwright. */ import { WebComponent } from '../../../src/component.js'; -import { html } from '../../../src/html.js'; +import { html, MARKER } from '../../../src/html.js'; import { repeat, cache, asyncAppend } from '../../../src/directives.js'; import { projectAuthored, isAuthoredContentSlot } from '../../../src/slot.js'; @@ -131,6 +131,61 @@ suite('Record self-heal + overlay coherence (review round 16)', () => { } }); + test('a list churning inside the slot leaves no bookends behind and does not perturb the record', async () => { + // A teardown removes the row's own boundary markers, and inside a slot that + // removal is seen by the backstop as one extra `removedNodes` entry per + // row. Those comments were never in the authored record (they sit between + // the host instance's own bookends, so `instanceOwns` claims them for the + // renderer), so churning the list must leave the record and the assignment + // exactly where they started. The marker count is asserted directly, + // because the leak is invisible to `querySelectorAll`. + ensureFixedShell(); + const parentTag = tagName('heal-churn'); + const rows = (n) => Array.from({ length: n }, (_, i) => `i${i}`); + class P extends WebComponent({ items: Array }) { + constructor() { super(); this.items = rows(4); } + render() { + return html`${this.items.map( + (i) => html`

${i}

`, + )}`; + } + } + P.register(parentTag); + const parent = document.createElement(parentTag); + document.body.appendChild(parent); + await tick(); + try { + const shell = parent.querySelector(fixedShell); + const slot = shell.querySelector('slot[data-webjs-light]'); + // Reads MARKER for the prefix, but spells the `e` suffix here, so it + // still tracks only half the marker text. The non-zero assertion below + // is what actually stops a counter that has gone blind from passing on + // 0 against 0; reading MARKER on its own would not. + const endMarkers = (root) => + [...root.childNodes].filter((n) => n.nodeType === 8 && n.data === `${MARKER}e`).length; + + assert.equal(slot.querySelectorAll('.item').length, 4, 'four items projected'); + const baseMarkers = endMarkers(slot); + assert.ok(baseMarkers > 0, 'the projected rows really carry end markers to count'); + const baseAssigned = slot.assignedNodes().length; + + for (let i = 0; i < 5; i++) { + parent.items = rows(1); + await parent.updateComplete; + await tick(); + parent.items = rows(4); + await parent.updateComplete; + await tick(); + } + + assert.equal(endMarkers(slot), baseMarkers, 'end markers under the slot drifted'); + assert.equal(slot.querySelectorAll('.item').length, 4, 'all four items are projected again'); + assert.equal(slot.assignedNodes().length, baseAssigned, 'the assigned set drifted'); + } finally { + parent.remove(); + } + }); + test('a library write into the assigned container survives the next apply', async () => { const tag = tagName('lib-write'); const host = await mount(tag, () => html`
`); diff --git a/website/app/docs/error-handling/page.ts b/website/app/docs/error-handling/page.ts index 16f616945..46db8ce7e 100644 --- a/website/app/docs/error-handling/page.ts +++ b/website/app/docs/error-handling/page.ts @@ -128,7 +128,7 @@ export default function GlobalError({ error }: { error: Error }) {

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 covered the same way: a chunk's own commit throw, and a watch() or until() nested inside a chunk, both reach the owning component's renderError(). A chunk's own commit throw also stops the stream, since the boundary is about to render an error state and appending into a region it may have replaced is not a recovery; a nested directive throws from its own handler outside that loop, so it reaches the boundary without stopping the stream. Your own code is the exception: a throw from the iterable or from a mapper you passed alongside it is a generator failing rather than a render, so it is still logged to the console and you are expected to handle it. That ends the stream too.

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().

+

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(). Covered also means a removal takes the row's own boundary markers with it, so a list that grows and shrinks all day is net zero on the nodes the renderer added, rather than accruing one invisible comment per removed row for the life of the region.

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.