diff --git a/packages/core/src/router-client.js b/packages/core/src/router-client.js index 6781186c6..5914b1f22 100644 --- a/packages/core/src/router-client.js +++ b/packages/core/src/router-client.js @@ -16,7 +16,10 @@ import './webjs-suspense.js'; import { scanSeeds } from './action-seed-client.js'; // Slot-runtime constants for re-projecting page-authored slotted content of a // reused hydrated light-DOM component across a soft nav (#908). -import { SLOT_STATE, LIGHT_SLOT_ATTR, PROJECTION_ATTR, PROJECTION_ACTUAL } from './slot.js'; +import { + SLOT_STATE, LIGHT_SLOT_ATTR, PROJECTION_ATTR, PROJECTION_ACTUAL, + applyActualAssignment, applyFallback, fireSlotChange, +} from './slot.js'; /** The content type a content-negotiated stream-action response carries (#248). */ const STREAM_MIME = 'text/vnd.webjs-stream.html'; @@ -2751,6 +2754,40 @@ function ownActualLightSlots(host) { return byName; } +/** + * The component's own light slot for `name`, whatever its projection state + * (first-wins by document order). Used to resolve the slot to fill on a + * fallback->actual transition, where the target slot is currently `fallback` + * so it is absent from `ownActualLightSlots`. Nested-child slots are excluded. + * + * @param {Element} host + * @param {string|null} name + * @returns {HTMLSlotElement | null} + */ +function ownLightSlotForName(host, name) { + for (const slot of host.querySelectorAll(`slot[${LIGHT_SLOT_ATTR}]`)) { + const s = /** @type {HTMLSlotElement} */ (slot); + if (!isOwnLightSlot(s, host)) continue; + if ((s.getAttribute('name') || null) === name) return s; + } + return null; +} + +/** + * Per-index node-identity equality of two node lists. Used to decide whether a + * slot's assigned-node SET actually changed (so `slotchange` should fire); an + * in-place text edit that reuses the same node is NOT a set change. + * + * @param {ArrayLike} a + * @param {ArrayLike} b + * @returns {boolean} + */ +function sameNodeList(a, b) { + if (a.length !== b.length) return false; + for (let i = 0; i < a.length; i++) if (a[i] !== b[i]) return false; + return true; +} + /** * Re-project the page-authored slotted content of a REUSED hydrated light-DOM * component across a soft nav (#908), without touching its render-owned @@ -2764,12 +2801,20 @@ function ownActualLightSlots(host) { * the live DOM and the incoming SSR HTML carry the same slot markers * (render-server emits them), so slots pair up by name + document order. * - * Scope: this handles the actual->actual case (the projected content changed). - * The two boundary transitions where a slot crosses between actual and fallback - * (content fully REMOVED, or ADDED where there was none) are deferred to #912: - * a slot's fallback is render-owned, so restoring or replacing it must go - * through the slot runtime, not a raw reconcile. Both are skipped here (see the - * two `continue`/iteration guards below), so neither can touch a lit-html part. + * Three cases, by how a slot's projection state changes across the nav: + * - actual->actual (content changed): identity-preserving `reconcileChildren` + * on the page-authored slot children, exactly as #908 shipped. + * - actual->fallback (content REMOVED) and fallback->actual (content ADDED): + * a slot's fallback is RENDER-OWNED (the compiled fallback template held by + * the slot-part), so these are NOT a raw reconcile. Drive them through the + * slot runtime's own primitives (`applyFallback` / `applyActualAssignment`) + * on the ONE resolved own slot, which restore or swap the render-owned + * fallback without reconciling any lit-html part (#912). This is applied + * surgically to the target slot, NOT via a whole-host `projectChildren` + * pass: `projectChildren` selects EVERY `data-webjs-light` slot in the + * subtree (including a nested child component's), so running it here would + * let this component's assignment reach into a nested child's same-named + * slot and clobber its render-owned nodes (the #906 hazard, one level down). * * @param {Element} dst Live hydrated component host. * @param {Element} src Incoming SSR copy of the same component. @@ -2782,31 +2827,48 @@ function reprojectSlottedContent(dst, src) { const state = /** @type {any} */ (dst)[SLOT_STATE]; if (!state) return; - // Iterate the LIVE component's actual slots only. A slot the live component - // currently shows as FALLBACK is absent here, so the fallback->actual - // direction (incoming ADDED content) is skipped: filling it would replace - // render-owned fallback nodes, which is the #906 hazard (deferred to #912). const liveSlots = ownActualLightSlots(dst); - if (liveSlots.size === 0) return; const incSlots = ownActualLightSlots(src); + if (liveSlots.size === 0 && incSlots.size === 0) return; - for (const [name, liveSlot] of liveSlots) { + // A slot name is `actual` on the live side, the incoming side, or both. Walk + // the union so a boundary transition (present on only one side) is handled. + const names = new Set([...liveSlots.keys(), ...incSlots.keys()]); + for (const name of names) { + const liveSlot = liveSlots.get(name); const incSlot = incSlots.get(name); - // The actual->fallback direction: incoming REMOVED this slot's content (now - // shows fallback), so leave the live projection as-is rather than touch - // render-owned fallback nodes. Conservative, no #906 regression (#912). - if (!incSlot) continue; - - // The slot's children are page-authored, so reconcileChildren is safe: - // it preserves node identity where it can and never touches lit-html parts. - reconcileChildren(liveSlot, incSlot); - - // Keep the slot runtime's assignment bookkeeping in sync with the new - // children so a later component re-render's projection pass materialises - // THESE nodes, not the stale ones it captured at hydration. - const children = [...liveSlot.childNodes]; - state.assignedByName.set(name, children); - state.lastSnapshot.set(liveSlot, children.slice()); + if (liveSlot && incSlot) { + // actual->actual: the slot's children are page-authored, so + // reconcileChildren is safe: it preserves node identity where it can and + // never touches lit-html parts. Keep the slot runtime's assignment + // bookkeeping in sync so a later re-render materialises THESE nodes. + const prevAssigned = state.lastSnapshot.get(liveSlot) || []; + reconcileChildren(liveSlot, incSlot); + const children = [...liveSlot.childNodes]; + state.assignedByName.set(name, children); + state.lastSnapshot.set(liveSlot, children.slice()); + // Fire slotchange on an assigned-node-SET change (add / remove / reorder), + // not an in-place text edit that reuses the same node: that matches the + // spec and keeps this branch uniform with the boundary branches below. + if (!sameNodeList(prevAssigned, children)) fireSlotChange(liveSlot); + } else if (liveSlot && !incSlot) { + // actual->fallback: incoming REMOVED this slot's content. Clear the + // assignment and flip THIS slot to fallback; applyFallback restores the + // render-owned fallback from the slot-part holding fragment. + state.assignedByName.delete(name); + if (applyFallback(state, liveSlot)) fireSlotChange(liveSlot); + } else { + // fallback->actual: incoming ADDED content where the live slot shows + // fallback. The target slot is currently fallback, so resolve it directly + // (it is absent from the actual-only map) and fill it with the imported + // page-authored nodes; applyActualAssignment tucks the render-owned + // fallback into the holding fragment first. + const targetSlot = ownLightSlotForName(dst, name); + if (!targetSlot) continue; + const nodes = [...incSlot.childNodes].map((n) => document.importNode(n, true)); + state.assignedByName.set(name, nodes); + if (applyActualAssignment(state, targetSlot, nodes)) fireSlotChange(targetSlot); + } } } diff --git a/packages/core/src/slot.js b/packages/core/src/slot.js index 61cdb8789..d8610c9cd 100644 --- a/packages/core/src/slot.js +++ b/packages/core/src/slot.js @@ -709,7 +709,7 @@ function handleSlotRemoved(state, slot) { * @returns {boolean} True if the slot's assignment changed compared to * its last snapshot (so slotchange should fire). */ -function applyActualAssignment(state, slot, assigned) { +export function applyActualAssignment(state, slot, assigned) { const wasFallback = slot.getAttribute(PROJECTION_ATTR) !== PROJECTION_ACTUAL; const prev = state.lastSnapshot.get(slot) || []; const equal = !wasFallback && arraysEqual(prev, assigned); @@ -747,7 +747,7 @@ function applyActualAssignment(state, slot, assigned) { * @returns {boolean} True if the slot transitioned from actual to * fallback this pass. */ -function applyFallback(state, slot) { +export function applyFallback(state, slot) { const wasActual = slot.getAttribute(PROJECTION_ATTR) === PROJECTION_ACTUAL; slot.setAttribute(PROJECTION_ATTR, PROJECTION_FALLBACK); if (!wasActual) { @@ -841,7 +841,7 @@ export function moveSlotChildrenToPending(host, slot) { // --------------------------------------------------------------------------- /** Fire a `slotchange` event on the slot (bubbles, not composed; per spec). */ -function fireSlotChange(slot) { +export function fireSlotChange(slot) { slot.dispatchEvent(new Event('slotchange', { bubbles: true, composed: false })); } diff --git a/packages/core/test/routing/browser/reconcile-hydrated-component.test.js b/packages/core/test/routing/browser/reconcile-hydrated-component.test.js index de9fe1c93..3944eedd9 100644 --- a/packages/core/test/routing/browser/reconcile-hydrated-component.test.js +++ b/packages/core/test/routing/browser/reconcile-hydrated-component.test.js @@ -367,4 +367,222 @@ suite('Client router: reconcile does not corrupt a hydrated component (#906)', ( live.remove(); }); + + test('re-projects an actual->fallback transition: incoming removed the content (#912)', async () => { + // The live component projects page-authored content; the incoming nav + // supplies NONE, so its slot shows fallback. The reused component must flip + // back to its own compiled fallback (render-owned, restored through the slot + // runtime, NOT a raw reconcile) rather than keep the stale content. + const tag = `rc-af-${counter++}`; + class RcAf extends WebComponent({ on: Boolean }) { + constructor() { super(); this.on = false; } + render() { + return html`
FALLBACK
`; + } + } + customElements.define(tag, RcAf); + + const live = document.createElement(tag); + live.innerHTML = 'REAL'; + document.body.appendChild(live); + await live.updateComplete; + await Promise.resolve(); + assert.equal(live.querySelector('slot').textContent, 'REAL', 'initial actual projection'); + assert.equal(live.querySelector('slot').getAttribute('data-projection'), 'actual', 'starts actual'); + + // Incoming SSR shows the slot as fallback (page authored no content). + const incoming = document.createElement(tag); + incoming.innerHTML = + '
FALLBACK
'; + _diffElementInPlace(live, incoming); + await Promise.resolve(); + + assert.equal(live.querySelector('slot').textContent, 'FALLBACK', + 'removed content must flip back to the compiled fallback'); + assert.equal(live.querySelector('slot').getAttribute('data-projection'), 'fallback', + 'slot marked fallback after the transition'); + + live.querySelector('button').click(); + await live.updateComplete; + assert.equal(live.on, true, 'reactive state still updates after actual->fallback'); + + live.remove(); + }); + + test('re-projects a fallback->actual transition: incoming added content (#912)', async () => { + // The live component shows its fallback (page authored nothing); the incoming + // nav supplies content. The reused component must project the new page-authored + // content in place of its fallback. + const tag = `rc-fa-${counter++}`; + class RcFa extends WebComponent({ on: Boolean }) { + constructor() { super(); this.on = false; } + render() { + return html`
FALLBACK
`; + } + } + customElements.define(tag, RcFa); + + const live = document.createElement(tag); + // No authored children: the slot shows its fallback. + document.body.appendChild(live); + await live.updateComplete; + await Promise.resolve(); + assert.equal(live.querySelector('slot').textContent, 'FALLBACK', 'initial fallback projection'); + assert.equal(live.querySelector('slot').getAttribute('data-projection'), 'fallback', 'starts fallback'); + + // Incoming SSR supplies actual content. + const incoming = document.createElement(tag); + incoming.innerHTML = + '
ADDED
'; + _diffElementInPlace(live, incoming); + await Promise.resolve(); + + assert.equal(live.querySelector('slot').textContent, 'ADDED', + 'added content must project in place of the fallback'); + assert.equal(live.querySelector('slot').getAttribute('data-projection'), 'actual', + 'slot marked actual after the transition'); + + live.querySelector('button').click(); + await live.updateComplete; + assert.equal(live.on, true, 'reactive state still updates after fallback->actual'); + + live.remove(); + }); + + test('a boundary transition must not clobber a nested child component sharing a slot name (#912)', async () => { + // The corruption risk: the outer component projects an actual->fallback + // transition on its OWN default slot while a nested light-DOM child (also + // using a default slot) sits elsewhere in the outer's rendered tree. The + // transition must touch ONLY the outer's own slot, never the nested child's + // same-named slot (a whole-host projection would push the child to fallback + // and detach its render-owned nodes: #906, one level down). + // A fixed child tag (defined once) so the host can reference it STATICALLY + // in its render template (an html tag position cannot be a `${}` hole). + const childTag = 'rc-clobber-child'; + if (!customElements.get(childTag)) { + class RcChild extends WebComponent({ n: Number }) { + constructor() { super(); this.n = 0; } + render() { + return html`childfb`; + } + } + customElements.define(childTag, RcChild); + } + + const outerTag = `rc-host-${counter++}`; + // The outer renders its OWN default slot AND embeds a nested child (which has + // its own default, same-named slot) as a sibling in its render output. + class RcHost extends WebComponent({ on: Boolean }) { + constructor() { super(); this.on = false; } + render() { + return html` +
hostfb
+ KEEP`; + } + } + customElements.define(outerTag, RcHost); + + const live = document.createElement(outerTag); + live.innerHTML = 'HOSTREAL'; + document.body.appendChild(live); + await live.updateComplete; + await Promise.resolve(); + const liveChild = live.querySelector(childTag); + await liveChild.updateComplete; + await Promise.resolve(); + // Drive the nested child's own state so a clobber-to-fallback would be visible. + liveChild.querySelector('button').click(); + await liveChild.updateComplete; + assert.equal(live.querySelector('section slot').textContent, 'HOSTREAL', 'outer own slot actual'); + assert.equal(liveChild.querySelector('em slot').textContent, 'KEEP', 'child slot projects its content'); + assert.equal(liveChild.querySelector('button').textContent, 'c 1', 'child client state'); + + // Incoming removes the OUTER's slotted content (its own slot -> fallback). + // The nested child is unchanged (same key/position, same projected content). + const incoming = document.createElement(outerTag); + incoming.innerHTML = + '' + + '
hostfb
' + + `<${childTag}>KEEP`; + _diffElementInPlace(live, incoming); + await Promise.resolve(); + + // Outer's own slot flipped to fallback. + assert.equal(live.querySelector('section slot').textContent, 'hostfb', + 'outer own slot flipped to its fallback'); + + // The nested child was NOT touched: still the SAME element, still projecting + // its content (not clobbered to childfb), still interactive. + assert.equal(live.querySelector(childTag), liveChild, 'nested child reused, not recreated'); + assert.equal(liveChild.querySelector('em slot').textContent, 'KEEP', + 'nested child slot must NOT be clobbered by the outer transition'); + assert.equal(liveChild.querySelector('em slot').getAttribute('data-projection'), 'actual', + 'nested child slot must stay actual'); + liveChild.querySelector('button').click(); + await liveChild.updateComplete; + assert.equal(liveChild.querySelector('button').textContent, 'c 2', + 'nested child still interactive (its render-owned nodes were never detached)'); + + live.remove(); + }); + + test('fires slotchange on an assigned-node-set change and boundary transitions, not on a text edit (#912)', async () => { + // The reproject must fire slotchange when the assigned-node SET changes + // (add / remove / boundary) but NOT when a text node is edited in place + // (same node identity), matching the DOM spec. + const tag = `rc-sc-${counter++}`; + class RcSc extends WebComponent({ on: Boolean }) { + constructor() { super(); this.on = false; } + render() { + return html`
FB
`; + } + } + customElements.define(tag, RcSc); + + function mount(innerHTML) { + const el = document.createElement(tag); + if (innerHTML != null) el.innerHTML = innerHTML; + document.body.appendChild(el); + return el; + } + function countSlotChanges(el) { + let n = 0; + el.querySelector('slot').addEventListener('slotchange', () => { n++; }); + return () => n; + } + + // 1. In-place text edit (same single text node reused): NO slotchange. + const a = mount('OLD'); + await a.updateComplete; await Promise.resolve(); + const aCount = countSlotChanges(a); + const aInc = document.createElement(tag); + aInc.innerHTML = '
NEW
'; + _diffElementInPlace(a, aInc); + await Promise.resolve(); + assert.equal(a.querySelector('slot').textContent, 'NEW', 'text updated'); + assert.equal(aCount(), 0, 'no slotchange on an in-place text edit (same node)'); + a.remove(); + + // 2. Set change (add a node): slotchange fires. + const b = mount('X'); + await b.updateComplete; await Promise.resolve(); + const bCount = countSlotChanges(b); + const bInc = document.createElement(tag); + bInc.innerHTML = '
XY
'; + _diffElementInPlace(b, bInc); + await Promise.resolve(); + assert.ok(bCount() >= 1, 'slotchange fires when a node is added to the set'); + b.remove(); + + // 3. actual->fallback boundary: slotchange fires. + const c = mount('REAL'); + await c.updateComplete; await Promise.resolve(); + const cCount = countSlotChanges(c); + const cInc = document.createElement(tag); + cInc.innerHTML = '
FB
'; + _diffElementInPlace(c, cInc); + await Promise.resolve(); + assert.ok(cCount() >= 1, 'slotchange fires on the actual->fallback transition'); + c.remove(); + }); });