Skip to content

Commit e99c33a

Browse files
committed
fix(core): 6 browser failures fixed, all 26 browser tests pass
Four real bugs surfaced in the browser test suite and are fixed: 1. captureAuthoredChildren ran on every connectedCallback, including on re-connection of a previously-mounted element. The second capture wrongly hoovered up the host's existing rendered template (the marker comments + the <div> with its slot) into the assignment table, then the re-render's clientRender no-op'd because INSTANCE was cached. Result: blank slot, lost projection. Fix: skip capture and adoption when hasSlotState(this) is already true (re-connection path). State preserved through disconnect by detachSlotObservers (only the observers pause). 2. adoptSSRAssignments ran AFTER _performRender, but _performRender's clientRender calls createInstance which calls replaceChildren and wipes the SSR-placed children before adoption ever sees them. Fix: move adoptSSRAssignments to BEFORE _performRender so the assignment table holds Node references to the SSR-placed children. createInstance still wipes them from the host, but they remain alive via the state map, and projection re-attaches them to the freshly-cloned slot. DOM identity is preserved through the hydration round-trip. 3. Child slot=" " attribute changes were observed only with subtree:false, so children already projected into slot elements (which sit one level deeper than the host) became invisible to the observer. Fix: subtree:true on the MutationObserver, with defensive checks in the callback. childList handler still filters to node.parentElement === host so projection's own slot.appendChild deep in the tree does not re-fire the capture path. removedNodes still skips when host.contains(node) so projection moves into a slot are not mistaken for user removals. 4. The slot-part apply hook only ran inside createInstance, never inside applyChild's nested-template branch. A <slot> inside `${cond ? html\`<slot/>\` : ''}` is part of an inner template compiled and bound separately when the outer's child-position hole resolves; its parts never saw the slot-apply tail loop. Fix: add the same slot-apply tail loop after the nested-template's insertBefore so the slot's microtask retry fires and projection runs. Conditional collapse + re-expansion now preserves projected child Node identity through the pending-fragment hand-off. Also: applyFallback no longer pushes children to the pending map. That code was a leftover from the conditional-collapse design; the correct path for that case lives in moveSlotChildrenToPending (called from clearInstance). applyFallback's push was an unrelated correctness hazard for user-initiated child removals. The slot-part apply now retries on the next microtask when findSlotHost returns null at first (the slot is still inside an unattached fragment for nested templates). The retry fires after the outer's replaceChildren, at which point parent-walk reaches the host. Tests pass: 26 of 26 browser cases in test/browser/slot.test.js 51 of 51 unit cases in test/component-slot.test.js 178 server-side unit cases across the broader sweep (component, render-client, render-server, directives, registry, css, html, context, task, suspense, repeat, testing, blog-smoke, json-negotiation, light-dom-ssr) Remaining for Task 14: - e2e tests covering SSR + page/layout + client router (per user's most recent direction) - Convention check rule (Task 15)
1 parent 95d2d37 commit e99c33a

3 files changed

Lines changed: 82 additions & 33 deletions

File tree

packages/core/src/component.js

Lines changed: 31 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
attachSlotObservers,
88
detachSlotObservers,
99
ensureSlotState,
10+
hasSlotState,
1011
} from './slot.js';
1112

1213
const isBrowser = typeof window !== 'undefined' && typeof HTMLElement !== 'undefined';
@@ -388,19 +389,32 @@ export class WebComponent extends Base {
388389
`For light DOM, use global CSS or <style> in render().`
389390
);
390391
}
391-
// Light-DOM slot lifecycle phase one. Capture authored children
392-
// into the slot-state's assignment table BEFORE _performRender
393-
// runs, since the renderer's first clientRender call will
394-
// replaceChildren() on the host and would otherwise destroy them.
392+
// Light-DOM slot lifecycle phase one. Three sub-paths:
395393
//
396-
// Detect SSR hydration via the framework's <!--webjs-hydrate-->
397-
// marker: in that case the SSR pipeline already placed projected
398-
// children inside <slot data-webjs-light data-projection="actual">
399-
// elements, so we ensure slot state exists (so findSlotHost
400-
// succeeds) and defer to adoptSSRAssignments after the renderer
401-
// has hydrated the template. Otherwise capture in place.
402-
if (this.__isHydrating()) {
394+
// a. Reconnection. Slot state already exists from a prior mount.
395+
// The host still carries the rendered template DOM (plus
396+
// projected children) from before the disconnect. Skip
397+
// capture (would wrongly hoover up rendered nodes) and skip
398+
// SSR adoption. clientRender will see the existing INSTANCE
399+
// and updateInstance instead of recreating; the DOM stays.
400+
//
401+
// b. SSR hydration (first mount, <!--webjs-hydrate--> marker
402+
// present). Children are already projected into
403+
// <slot data-webjs-light data-projection="actual"> elements
404+
// by injectDSD. Adopt those assignments BEFORE _performRender
405+
// so we retain references to the SSR'd nodes; the renderer's
406+
// createInstance().replaceChildren() will detach them, but
407+
// projection re-attaches by moving the same Node refs into
408+
// the freshly-cloned slot. DOM identity preserved through the
409+
// hydration round-trip.
410+
//
411+
// c. First mount, no SSR. Move authored children into the
412+
// assignment table before _performRender wipes the host.
413+
if (hasSlotState(this)) {
414+
// (a) Reconnection. State already populated; nothing to do here.
415+
} else if (this.__isHydrating()) {
403416
ensureSlotState(this);
417+
adoptSSRAssignments(this);
404418
} else {
405419
captureAuthoredChildren(this);
406420
}
@@ -418,16 +432,13 @@ export class WebComponent extends Base {
418432
this._performRender();
419433

420434
// Light-DOM slot lifecycle phase two. With the rendered template
421-
// (and therefore the live <slot> elements) now in the DOM, hook up
422-
// the mutation observers so future authored-child mutations and
423-
// slot-name changes drive incremental projection. For hydrated
424-
// hosts, also adopt the SSR-placed slot assignments so the first
425-
// projection pass is a no-op.
435+
// (and therefore the live <slot> elements) now in the DOM, attach
436+
// mutation observers so future authored-child mutations and
437+
// slot-name changes drive incremental projection. Adoption of
438+
// SSR-placed assignments has already happened in phase one (before
439+
// _performRender) so the renderer's replaceChildren cannot destroy
440+
// those nodes; projection moves them into the freshly-cloned slots.
426441
if (this._renderRoot === this) {
427-
if (this.__hydratedAtActivate) {
428-
adoptSSRAssignments(this);
429-
this.__hydratedAtActivate = false;
430-
}
431442
attachSlotObservers(this);
432443
}
433444
}

packages/core/src/render-client.js

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -603,10 +603,25 @@ function applyPart(part, value, _prev, allValues) {
603603
// been inserted into the host's render root (so the slot can find
604604
// its host by walking parents). Subsequent re-renders are
605605
// observer-driven from slot.js, not value-driven.
606+
//
607+
// For NESTED templates (a slot inside `${cond ? html`<slot/>` : ''}`),
608+
// the slot's parent chain at apply time leads up through an
609+
// unattached fragment that has not yet been inserted into the
610+
// host's render root by the outer createInstance. findSlotHost
611+
// returns null in that case. We retry on the next microtask, by
612+
// which point the outer's replaceChildren has placed the entire
613+
// tree (including this nested slot) into the host.
606614
if (part.applied) break;
607615
part.applied = true;
608-
const host = findSlotHost(part.slotEl);
609-
if (host) scheduleProjection(host);
616+
const directHost = findSlotHost(part.slotEl);
617+
if (directHost) {
618+
scheduleProjection(directHost);
619+
} else {
620+
queueMicrotask(() => {
621+
const h = findSlotHost(part.slotEl);
622+
if (h) scheduleProjection(h);
623+
});
624+
}
610625
break;
611626
}
612627
case 'noop':
@@ -710,6 +725,14 @@ function applyChild(part, value) {
710725
}
711726
const nodes = [startNode, ...frag.childNodes, endNode];
712727
marker.parentNode?.insertBefore(nodesToFrag(nodes), marker);
728+
// Slot parts in this nested template need their one-shot apply just
729+
// like createInstance does for top-level templates. The slot is now
730+
// in the live tree (insertBefore above) so its parent walk can
731+
// reach the host. Without this loop, conditional / nested templates
732+
// with <slot> inside never trigger projection.
733+
for (const p of bound) {
734+
if (p.kind === 'slot') applyPart(p, undefined, undefined, []);
735+
}
713736
part.child = { strings: tr.strings, bound, lastValues, startNode, endNode };
714737
return;
715738
}

packages/core/src/slot.js

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -411,21 +411,35 @@ export function attachSlotObservers(host) {
411411
}
412412
} else if (r.type === 'attributes' && r.attributeName === 'slot') {
413413
const target = /** @type {Element} */ (r.target);
414-
if (target.parentElement === host || hostOwnsAssignedNode(host, target)) {
415-
if (removeFromAssignments(state, target)) {
416-
appendToMap(state.assignedByName, slotNameOf(target), target);
417-
dirty = true;
418-
}
414+
// A child's slot=" " attribute changed. If it's an authored
415+
// child currently in the assignment table (either still a
416+
// direct child of host or already projected into a slot deeper
417+
// down), re-partition by the new name.
418+
if (removeFromAssignments(state, target)) {
419+
appendToMap(state.assignedByName, slotNameOf(target), target);
420+
dirty = true;
421+
} else if (target.parentElement === host) {
422+
// Edge case where state.removeFromAssignments missed it: still
423+
// direct on host and not yet captured.
424+
appendToMap(state.assignedByName, slotNameOf(target), target);
425+
dirty = true;
419426
}
420427
}
421428
}
422429
if (dirty) scheduleProjection(host);
423430
});
431+
// subtree: true so children moved into <slot> deeper in the tree are
432+
// still observed when their slot=" " attribute changes (which would
433+
// otherwise be invisible under subtree: false). The attribute filter
434+
// keeps the observed surface small. childList stays scoped to host's
435+
// direct children: appending a node to host is the user-level entry
436+
// point for authoring; projection's slot.appendChild is on a node
437+
// deeper in the tree and the filter does not fire on those.
424438
state.childObserver.observe(host, {
425439
childList: true,
426440
attributes: true,
427441
attributeFilter: ['slot'],
428-
subtree: false,
442+
subtree: true,
429443
});
430444
}
431445

@@ -686,11 +700,12 @@ function applyFallback(state, slot) {
686700
return false;
687701
}
688702

689-
const prev = state.lastSnapshot.get(slot) || [];
690-
if (prev.length > 0) {
691-
const lastName = slot.getAttribute('name') || null;
692-
appendArrayToMap(state.pendingByName, lastName, prev);
693-
}
703+
// Slot transitioning from actual to fallback. This happens when the
704+
// host's assignment for this slot's name is empty (e.g., user removed
705+
// all matching children). Drop the lastSnapshot record; do NOT push
706+
// children to the pending map (that path is for slot destruction
707+
// during a conditional collapse, handled separately by
708+
// moveSlotChildrenToPending).
694709
state.lastSnapshot.delete(slot);
695710
while (slot.firstChild) slot.removeChild(slot.firstChild);
696711
restoreFallbackInto(slot);

0 commit comments

Comments
 (0)