Keep a late-arriving server-component boundary claimable and its slot range live

.changeset/adopted-slot-range-stays-live.md

@@ -0,0 +1,5 @@
+---
+"@solidjs/web": patch
+---
+
+frames: keep an adopted boundary's slot range reactive. The claim scope wrapped insert's accessor, so the binding's first read ran inside `runWithOwner`'s untracked window — reactive only by accident, via the re-read of whatever accessor that first read returned. A `<Loading>` answering a still-pending streamed fragment returns fallback NODES instead, leaving the effect with no dependency at all and the range permanently inert: the boundary's own resume still claimed the swapped-in server markup, so the region looked right, but nothing downstream ever re-rendered it (in the notes example, every navigation out of a late-settling note changed the URL and nothing else). The claim now wraps the insert CALL, so the first evaluation is the render effect's own compute — still under the producer's hydration keys, but tracked.

.changeset/late-boundary-held-fragment-wait.md

@@ -0,0 +1,5 @@
+---
+"@solidjs/web": patch
+---
+
+frames: keep waiting for a document boundary whose element is still held by a deferred fragment. `_$HY.done` stopped meaning "the page is complete" once post-done swaps became held-until-claimed (#2964) — a boundary rendering in that window mounted a fresh frame, orphaning the markup the replay then delivered, and left the id unclaimed so every later call resolved back to the document placeholder instead of fetching (a server-component region that never updates again). An unresolved `pl-*` placeholder now keeps the answer "not yet"; a reveal that exhausts the page's deferred fragments releases the waiter to mount fresh.

packages/solid-web/frames/src/client.ts

@@ -308,32 +308,40 @@ function slotsFor(props: Record<string, any>) {
           // positions) and return undefined so the frame leaves the interior
           // alone. `existing` seeds insert's tracked array: an accessor that
           // yields the claimed nodes reconciles to a zero-mutation no-op, one
-          // that yields new content swaps it in place. The accessor's FIRST
-          // evaluation re-enters the claim scope — boundary-deferred children
-          // (route content behind <Loading>) create on that read and must see
-          // the producer's hydration keys.
+          // that yields new content swaps it in place.
+          //
+          // The claim scope wraps the insert CALL, not the accessor: the
+          // binding's first evaluation is insert's own render effect computing
+          // synchronously, so it still creates under the producer's hydration
+          // keys — boundary-deferred children (route content behind
+          // <Loading>) create on that read — while the reads it makes belong
+          // to the effect and stay tracked. Claiming inside the accessor
+          // instead put that first read inside runWithOwner's UNTRACKED window
+          // (it clears `tracking` along with the owner). Whenever the value it
+          // returned was not itself an accessor for insert to re-read — a
+          // <Loading> answering a still-pending streamed fragment returns its
+          // fallback NODES — the effect ended up with no dependency at all and
+          // the range went permanently inert: the boundary's own resume still
+          // claimed the swapped-in server markup, so the region looked right,
+          // but nothing downstream (a route change out of it) ever re-rendered
+          // it again.
           if (ctx && ctx.range) {
-            let claim = adopted;
             const source = value;
-            const accessor = () => {
-              if (claim) {
-                claim = false;
-                return claimRender(prefix, ctx.existing, () =>
-                  typeof source === "function" ? source() : source
-                );
-              }
-              return typeof source === "function" ? source() : source;
-            };
             const owner = createOwner();
             bindings.set(key, owner);
             ctx.onCleanup(() => {
               if (bindings.get(key) === owner) bindings.delete(key);
               owner.dispose();
             });
             const end = ctx.range.end;
-            runWithOwner(owner, () =>
-              insert(end.parentNode as any, accessor, end, [...ctx.existing])
-            );
+            const bind = () =>
+              insert(
+                end.parentNode as any,
+                () => (typeof source === "function" ? source() : source),
+                end,
+                [...ctx.existing]
+              );
+            runWithOwner(owner, () => (adopted ? claimRender(prefix, ctx.existing, bind) : bind()));
             return undefined;
           }
           // No range handle (a consumer-constructed frame without markers):
@@ -455,12 +463,31 @@ function findBoundaryElement(id: string): Element | undefined {
 // that carries it. One waiter per id: a second mount while the first is still
 // waiting takes the fresh-frame path, since only one frame may adopt an
 // element.
-const boundaryWaiters = new Map<string, (el: Element) => void>();
+const boundaryWaiters = new Map<string, (el?: Element) => void>();
+
+/** An unresolved deferred fragment (`<template id="pl-*">`) anywhere in the page. */
+function pendingFragmentInDocument() {
+  return typeof document !== "undefined" && !!document.querySelector('template[id^="pl-"]');
+}
 
-/** Whether the document may still deliver boundary elements. */
-function documentStreaming() {
+/**
+ * Whether the document may still deliver boundary elements.
+ *
+ * `_$HY.done` alone stopped being that answer with the held-swap policy
+ * (#2964): a fragment that settles after global hydration completes is HELD —
+ * placeholder, fallback and template all left in place — until its boundary
+ * registers as the claimant, and the replay that follows is what puts this
+ * element in the page. A boundary rendering in that window (a frames slot fill
+ * or lazy route module running after the root pass) would read done as "the
+ * page is complete", mount a fresh frame, and orphan the markup on the way:
+ * the region goes inert, and because the id is never claimed, every later call
+ * for this function resolves back to the document placeholder instead of
+ * fetching. So an unresolved `pl-*` placeholder keeps the answer "not yet".
+ */
+function boundaryMayArrive() {
   const hy = (globalThis as any)._$HY;
-  return !!hy && !hy.done;
+  if (!hy) return false;
+  return !hy.done || pendingFragmentInDocument();
 }
 
 /**
@@ -489,9 +516,15 @@ function installRevealHook() {
     const root = parent || (typeof document !== "undefined" ? document.body : null);
     if (!root) return;
     indexBoundaries(root);
+    if (!boundaryWaiters.size) return;
+    // A waiter the page can no longer answer must not wait forever: once the
+    // document is done and no deferred fragment is left to reveal, nothing
+    // else can deliver this element, so release the waiter to mount fresh
+    // (the client-only shape) instead of holding the fallback on screen.
+    const exhausted = hy.done && !pendingFragmentInDocument();
     for (const [id, notify] of boundaryWaiters) {
       const el = boundaryIndex.get(id);
-      if (!el) continue;
+      if (!el && !exhausted) continue;
       boundaryWaiters.delete(id);
       notify(el);
     }
@@ -502,20 +535,28 @@ function documentBoundary(host: any, id: string, props: Record<string, any>) {
   const claimed = claimedBoundaries.has(id);
   const el = !claimed ? findBoundaryElement(id) : undefined;
   if (el) return adoptBoundary(host, id, el, props);
-  // Not in the page (yet). While the document is still streaming, this is a
-  // boundary whose content settled after the shell flush: its markup is on the
-  // way and will be swapped over the fallback that is on screen right now.
-  // Mounting a fresh frame here is unrecoverable — the markup arrives owned by
-  // nothing (visible but inert) while the stream drives an element outside the
-  // page, so the boundary never updates again. Suspend instead and adopt on
-  // delivery; the enclosing <Loading> goes on showing the server's fallback,
-  // which is exactly what the document is displaying.
-  if (!claimed && !boundaryWaiters.has(id) && documentStreaming()) {
+  // Not in the page (yet). While the page can still deliver it (the document is
+  // streaming, or a deferred fragment is still holding its markup — see
+  // boundaryMayArrive), this is a boundary whose content settled after the
+  // shell flush: its markup is on the way and will be swapped over the
+  // fallback that is on screen right now. Mounting a fresh frame here is
+  // unrecoverable — the markup arrives owned by nothing (visible but inert)
+  // while the stream drives an element outside the page, so the boundary never
+  // updates again. Suspend instead and adopt on delivery; the enclosing
+  // <Loading> goes on showing the server's fallback, which is exactly what the
+  // document is displaying.
+  if (!claimed && !boundaryWaiters.has(id) && boundaryMayArrive()) {
     const owner = getOwner();
-    const arrival = new Promise<Element>(resolve => boundaryWaiters.set(id, resolve));
+    const arrival = new Promise<Element | undefined>(resolve => boundaryWaiters.set(id, resolve));
     onCleanup(() => boundaryWaiters.delete(id));
     return createMemo(() =>
-      arrival.then(node => runWithOwner(owner, () => adoptBoundary(host, id, node, props)))
+      arrival.then(node =>
+        runWithOwner(owner, () =>
+          // No element after all (the page ran out of reveals): mount fresh,
+          // exactly as an unwaited miss would have.
+          node ? adoptBoundary(host, id, node, props) : boundaryComponent(host, id)(props)
+        )
+      )
     ) as unknown as SolidElement;
   }
   // No SSR'd boundary on the page (client-only boot, or already claimed):

packages/solid-web/test/frames-late-boundary-client.spec.tsx

@@ -29,6 +29,8 @@ import {
   createFrameHost,
   createJSONDataTable
 } from "../frames/src/client.js";
+import { createServerReference } from "@dom-expressions/runtime/src/server-functions/client.js";
+import { createChunk } from "@dom-expressions/runtime/src/server-functions/shared.js";
 
 const settle = () => new Promise(r => setTimeout(r));
 
@@ -41,17 +43,38 @@ function makeHost() {
 }
 
 const FID = "late/feed";
+// Distinct ids per test: a boundary is claimable exactly once per page.
+const FID_HELD = "late/held";
+const FID_EXHAUSTED = "late/exhausted";
 
-/** The `$df` swap, reduced to what matters here: put the server's boundary
- *  element in the live document, then announce the reveal. */
+/** The `$df` swap, reduced to what matters here: retire the fragment's
+ *  placeholder, put the server's boundary element in the live document, then
+ *  announce the reveal. */
 function swapIn(parent: HTMLElement, html: string) {
+  document.getElementById("pl-1902")?.remove();
   const tpl = document.createElement("template");
   tpl.innerHTML = html;
   parent.appendChild(tpl.content);
   const hy = (window as any)._$HY;
   hy.fe && hy.fe("1902", parent);
 }
 
+/** A one-shot frame stream, the shape a navigation's response arrives in. */
+function frameResponse(id: string, html: string) {
+  const chunks = [
+    { type: "start", id, version: 1 },
+    { type: "html", id, version: 1, html },
+    { type: "complete", id, version: 1 }
+  ];
+  const body = new ReadableStream({
+    start(controller) {
+      for (const c of chunks) controller.enqueue(createChunk(JSON.stringify(c)));
+      controller.close();
+    }
+  });
+  return new Response(body, { headers: { "X-Frame-Stream": id } });
+}
+
 describe("boundary that arrives after the shell flush", () => {
   afterEach(() => {
     vi.unstubAllGlobals();
@@ -125,4 +148,116 @@ describe("boundary that arrives after the shell flush", () => {
 
     dispose();
   });
+
+  // The same "not in the page yet" moment, one step later in the document's
+  // life: global hydration has already completed. Under the held-swap policy
+  // (#2964) that no longer means the page is finished — a fragment settling
+  // post-done keeps its placeholder, fallback and template in place until its
+  // boundary claims it, and the replay that follows is what delivers this
+  // element. A boundary rendering in that window (a frames slot fill or lazy
+  // route module running after the root pass) that reads `done` as "never"
+  // mounts a fresh frame and orphans the markup: the region goes inert AND —
+  // because the id is never claimed — every later call for this function
+  // resolves back to the document placeholder instead of fetching, so the app
+  // stops responding to navigation entirely.
+  test("waits for a fragment still holding the element after hydration reports done", async () => {
+    document.body.innerHTML =
+      '<div id="app"><template id="pl-1902"></template>fallback<!--pl-1902--></div>';
+    (window as any)._$HY = { r: {}, fe() {}, done: true };
+    vi.stubGlobal("fetch", () => {
+      throw new Error("fetch must not be called while awaiting the swap");
+    });
+    const host = makeHost();
+    installServerComponents(host);
+
+    const Feed = (window as any)._$SC.r(FID_HELD);
+    const appEl = document.getElementById("app") as HTMLElement;
+    let mount!: HTMLDivElement;
+    const dispose = createRoot(d => {
+      <div ref={mount}>
+        <Loading fallback={<span>fallback</span>}>
+          <Feed />
+        </Loading>
+      </div>;
+      appEl.appendChild(mount);
+      return d;
+    });
+    flush();
+    await settle();
+    flush();
+
+    expect(mount.querySelectorAll(`dx-frame[data-fid="${FID_HELD}"]`).length).toBe(0);
+
+    swapIn(
+      mount,
+      `<dx-frame data-fid="${FID_HELD}" style="display:contents"><ul><li>server-item</li></ul></dx-frame>`
+    );
+    flush();
+    await settle();
+    flush();
+
+    const frames = mount.querySelectorAll(`dx-frame[data-fid="${FID_HELD}"]`);
+    expect(frames.length).toBe(1);
+    expect((frames[0] as HTMLElement).textContent).toContain("server-item");
+
+    // Claimed on adoption: the next call for this function is a navigation,
+    // and it has to leave the browser. Resolving it locally with the document
+    // placeholder again is what made every subsequent click a no-op.
+    const feed = createServerReference(FID_HELD);
+    let requests = 0;
+    vi.stubGlobal("fetch", async () => {
+      requests++;
+      return frameResponse(FID_HELD, "<ul><li>navigated-item</li></ul>");
+    });
+    await feed(2);
+    flush();
+    await settle();
+    flush();
+
+    expect(requests).toBe(1);
+
+    dispose();
+  });
+
+  // The mirror case: nothing is left to reveal, so waiting would strand the
+  // region on its fallback forever. A reveal that exhausts the page's deferred
+  // fragments releases the waiter to mount fresh.
+  test("gives up waiting once the page has no deferred fragment left", async () => {
+    document.body.innerHTML =
+      '<div id="app"><template id="pl-1902"></template>fallback<!--pl-1902--></div>';
+    (window as any)._$HY = { r: {}, fe() {}, done: true };
+    const host = makeHost();
+    installServerComponents(host);
+
+    const Feed = (window as any)._$SC.r(FID_EXHAUSTED);
+    const appEl = document.getElementById("app") as HTMLElement;
+    let mount!: HTMLDivElement;
+    const dispose = createRoot(d => {
+      <div ref={mount}>
+        <Loading fallback={<span>fallback</span>}>
+          <Feed />
+        </Loading>
+      </div>;
+      appEl.appendChild(mount);
+      return d;
+    });
+    flush();
+    await settle();
+    flush();
+
+    expect(mount.querySelectorAll(`dx-frame[data-fid="${FID_EXHAUSTED}"]`).length).toBe(0);
+
+    // The fragment reveals — but it carried someone else's content, and it was
+    // the last one the page had.
+    swapIn(mount, "<span>unrelated</span>");
+    flush();
+    await settle();
+    flush();
+
+    // A client-owned frame for the id, ready to take the stream a call fills
+    // it with — rather than a permanently pending boundary.
+    expect(mount.querySelectorAll(`dx-frame[data-fid="${FID_EXHAUSTED}"]`).length).toBe(1);
+
+    dispose();
+  });
 });